Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(threads): delay thread start until init_task() has finished #526

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/riot-rs-embassy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ mod executor_thread {
}

#[cfg(feature = "executor-thread")]
#[riot_rs_macros::thread(autostart, stacksize = executor_thread::STACKSIZE, priority = executor_thread::PRIORITY)]
#[riot_rs_macros::thread(autostart, no_wait, stacksize = executor_thread::STACKSIZE, priority = executor_thread::PRIORITY)]
fn init() {
debug!("riot-rs-embassy::init(): using thread executor");
let p = arch::init();
Expand Down Expand Up @@ -326,4 +326,7 @@ async fn init_task(mut peripherals: arch::OptionalPeripherals) {
let _ = peripherals;

debug!("riot-rs-embassy::init_task() done");

#[cfg(feature = "threading")]
riot_rs_threads::_events::THREAD_START_EVENT.set();
}
54 changes: 39 additions & 15 deletions src/riot-rs-macros/src/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
/// - `autostart`: (*mandatory*) autostart the thread.
/// - `stacksize`: (*optional*) the size of the stack allocated to the thread (in bytes).
/// - `priority`: (*optional*) the thread's priority.
/// - `no_wait`: (*optional*) don't wait for system initialization to be finished
/// before starting the thread.
kaspar030 marked this conversation as resolved.
Show resolved Hide resolved
///
/// # Examples
///
Expand Down Expand Up @@ -38,7 +40,7 @@ pub fn thread(args: TokenStream, item: TokenStream) -> TokenStream {
#[allow(clippy::wildcard_imports)]
use thread::*;

use quote::quote;
use quote::{format_ident, quote};

use crate::utils::find_crate;

Expand All @@ -53,32 +55,45 @@ pub fn thread(args: TokenStream, item: TokenStream) -> TokenStream {

let thread_function = syn::parse_macro_input!(item as syn::ItemFn);

let thread_crate = {
match (find_crate("riot-rs"), find_crate("riot-rs-threads")) {
(Some(riot_rs), _) => quote! { #riot_rs::thread },
(None, Some(riot_rs_threads)) => quote! { #riot_rs_threads },
_ => panic!(r#"neither "riot-rs" nor "riot-rs-threads" found in dependencies!"#),
}
};

let no_mangle_attr = if attrs.no_mangle {
quote! {#[no_mangle]}
} else {
quote! {}
};

let maybe_wait_for_start_event = if attrs.no_wait {
quote! {}
} else {
quote! {#thread_crate::_events::THREAD_START_EVENT.wait();}
};

let fn_name = thread_function.sig.ident.clone();
let trampoline_function_name = format_ident!("__{fn_name}_trampoline");

let Parameters {
stack_size,
priority,
affinity,
} = Parameters::from(attrs);

let thread_crate = {
match (find_crate("riot-rs"), find_crate("riot-rs-threads")) {
(Some(riot_rs), _) => quote! { #riot_rs::thread },
(None, Some(riot_rs_threads)) => quote! { #riot_rs_threads },
_ => panic!(r#"neither "riot-rs" nor "riot-rs-threads" found in dependencies!"#),
}
};

let expanded = quote! {
#no_mangle_attr
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#no_mangle_attr
#no_mangle_attr
#[inline(always)]

Not saying that I know better than the compiler, but maybe in this case it can be useful, so that it gets inlined inside the trampoline function?

#thread_function

#thread_crate::autostart_thread!(#fn_name, stacksize = #stack_size, priority = #priority, affinity = #affinity);
fn #trampoline_function_name() {
#maybe_wait_for_start_event;
#fn_name()
}

#thread_crate::autostart_thread!(#trampoline_function_name, stacksize = #stack_size, priority = #priority, affinity = #affinity);
};

TokenStream::from(expanded)
Expand All @@ -95,9 +110,9 @@ mod thread {
fn default() -> Self {
// TODO: proper values
Self {
stack_size: syn::parse_quote!{ 2048 },
priority: syn::parse_quote!{ 1 },
affinity: syn::parse_quote!{ None }
stack_size: syn::parse_quote! { 2048 },
priority: syn::parse_quote! { 1 },
affinity: syn::parse_quote! { None },
}
}
}
Expand All @@ -108,12 +123,15 @@ mod thread {

let stack_size = attrs.stack_size.unwrap_or(default.stack_size);
let priority = attrs.priority.unwrap_or(default.priority);
let affinity = attrs.affinity.map(|expr| syn::parse_quote!{ Some(#expr) }).unwrap_or(default.affinity);
let affinity = attrs
.affinity
.map(|expr| syn::parse_quote! { Some(#expr) })
.unwrap_or(default.affinity);

Self {
stack_size,
priority,
affinity
affinity,
}
}
}
Expand All @@ -125,6 +143,7 @@ mod thread {
pub priority: Option<syn::Expr>,
pub affinity: Option<syn::Expr>,
pub no_mangle: bool,
pub no_wait: bool,
}

impl Attributes {
Expand Down Expand Up @@ -159,6 +178,11 @@ mod thread {
return Ok(());
}

if meta.path.is_ident("no_wait") {
self.no_wait = true;
return Ok(());
}

Err(meta.error("unsupported parameter"))
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/riot-rs-threads/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ pub mod macro_reexports {
pub use static_cell;
}

#[doc(hidden)]
pub mod _events {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub mod _events {
pub mod events {

_foo means that it's unused, not that it is private or "not to be used by users"

use crate::sync::Event;
// this is set in `riot_rs_embassy::init_task()``
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// this is set in `riot_rs_embassy::init_task()``
// this is set in `riot_rs_embassy::init_task()`

pub static THREAD_START_EVENT: Event = Event::new();
}

pub use riot_rs_runqueue::{RunqueueId, ThreadId};
pub use thread_flags as flags;

Expand Down
Loading