From 50ec1411925a01c0103161a7d2d2ec3968b5a981 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Sat, 16 Oct 2021 03:57:41 +0200 Subject: [PATCH] WIP: remove signal_fn in executor. --- embassy-nrf/src/time_driver.rs | 24 ++---- embassy-stm32/src/time_driver.rs | 22 ++--- .../src/executor/arch/{arm.rs => arm_irq.rs} | 80 +++++-------------- embassy/src/executor/arch/arm_wfe.rs | 73 +++++++++++++++++ embassy/src/executor/mod.rs | 2 +- embassy/src/executor/raw/mod.rs | 17 ++-- embassy/src/time/driver.rs | 21 ++--- examples/nrf/src/bin/multiprio.rs | 40 ++++++---- 8 files changed, 147 insertions(+), 132 deletions(-) rename embassy/src/executor/arch/{arm.rs => arm_irq.rs} (53%) create mode 100644 embassy/src/executor/arch/arm_wfe.rs diff --git a/embassy-nrf/src/time_driver.rs b/embassy-nrf/src/time_driver.rs index 19356c2d23..89715dd67f 100644 --- a/embassy-nrf/src/time_driver.rs +++ b/embassy-nrf/src/time_driver.rs @@ -1,8 +1,8 @@ use core::cell::Cell; use core::sync::atomic::{compiler_fence, AtomicU32, AtomicU8, Ordering}; -use core::{mem, ptr}; use critical_section::CriticalSection; use embassy::blocking_mutex::CriticalSectionMutex as Mutex; +use embassy::executor::ExecutorWaker; use embassy::interrupt::{Interrupt, InterruptExt}; use embassy::time::driver::{AlarmHandle, Driver}; @@ -61,11 +61,7 @@ mod test { struct AlarmState { timestamp: Cell, - - // This is really a Option<(fn(*mut ()), *mut ())> - // but fn pointers aren't allowed in const yet - callback: Cell<*const ()>, - ctx: Cell<*mut ()>, + callback: Cell, } unsafe impl Send for AlarmState {} @@ -74,8 +70,7 @@ impl AlarmState { const fn new() -> Self { Self { timestamp: Cell::new(u64::MAX), - callback: Cell::new(ptr::null()), - ctx: Cell::new(ptr::null_mut()), + callback: Cell::new(unsafe { ExecutorWaker::poison() }), } } } @@ -173,12 +168,7 @@ impl RtcDriver { alarm.timestamp.set(u64::MAX); // Call after clearing alarm, so the callback can set another alarm. - - // safety: - // - we can ignore the possiblity of `f` being unset (null) because of the safety contract of `allocate_alarm`. - // - other than that we only store valid function pointers into alarm.callback - let f: fn(*mut ()) = unsafe { mem::transmute(alarm.callback.get()) }; - f(alarm.ctx.get()); + alarm.callback.get().wake(); } } @@ -208,12 +198,10 @@ impl Driver for RtcDriver { } } - fn set_alarm_callback(&self, alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()) { + fn set_alarm_callback(&self, alarm: AlarmHandle, callback: ExecutorWaker) { critical_section::with(|cs| { let alarm = self.get_alarm(cs, alarm); - - alarm.callback.set(callback as *const ()); - alarm.ctx.set(ctx); + alarm.callback.set(callback); }) } diff --git a/embassy-stm32/src/time_driver.rs b/embassy-stm32/src/time_driver.rs index d20929e17c..549137c96d 100644 --- a/embassy-stm32/src/time_driver.rs +++ b/embassy-stm32/src/time_driver.rs @@ -2,7 +2,7 @@ use atomic_polyfill::{AtomicU32, AtomicU8}; use core::cell::Cell; use core::convert::TryInto; use core::sync::atomic::{compiler_fence, Ordering}; -use core::{mem, ptr}; +use embassy::executor::ExecutorWaker; use embassy::interrupt::InterruptExt; use embassy::time::driver::{AlarmHandle, Driver}; use embassy::time::TICKS_PER_SECOND; @@ -58,10 +58,7 @@ fn calc_now(period: u32, counter: u16) -> u64 { struct AlarmState { timestamp: Cell, - // This is really a Option<(fn(*mut ()), *mut ())> - // but fn pointers aren't allowed in const yet - callback: Cell<*const ()>, - ctx: Cell<*mut ()>, + callback: Cell, } unsafe impl Send for AlarmState {} @@ -70,8 +67,7 @@ impl AlarmState { const fn new() -> Self { Self { timestamp: Cell::new(u64::MAX), - callback: Cell::new(ptr::null()), - ctx: Cell::new(ptr::null_mut()), + callback: Cell::new(unsafe { ExecutorWaker::poison() }), } } } @@ -197,12 +193,7 @@ impl RtcDriver { alarm.timestamp.set(u64::MAX); // Call after clearing alarm, so the callback can set another alarm. - - // safety: - // - we can ignore the possiblity of `f` being unset (null) because of the safety contract of `allocate_alarm`. - // - other than that we only store valid function pointers into alarm.callback - let f: fn(*mut ()) = unsafe { mem::transmute(alarm.callback.get()) }; - f(alarm.ctx.get()); + alarm.callback.get().wake(); } } @@ -234,12 +225,11 @@ impl Driver for RtcDriver { } } - fn set_alarm_callback(&self, alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()) { + fn set_alarm_callback(&self, alarm: AlarmHandle, callback: ExecutorWaker) { critical_section::with(|cs| { let alarm = self.get_alarm(cs, alarm); - alarm.callback.set(callback as *const ()); - alarm.ctx.set(ctx); + alarm.callback.set(callback); }) } diff --git a/embassy/src/executor/arch/arm.rs b/embassy/src/executor/arch/arm_irq.rs similarity index 53% rename from embassy/src/executor/arch/arm.rs rename to embassy/src/executor/arch/arm_irq.rs index d23a595ff0..9a34a2989d 100644 --- a/embassy/src/executor/arch/arm.rs +++ b/embassy/src/executor/arch/arm_irq.rs @@ -1,70 +1,26 @@ use core::marker::PhantomData; -use core::ptr; +use core::mem; use super::{raw, Spawner}; -use crate::interrupt::{Interrupt, InterruptExt}; +use crate::interrupt::{Interrupt, InterruptExt, NrWrap}; -/// Thread mode executor, using WFE/SEV. -/// -/// This is the simplest and most common kind of executor. It runs on -/// thread mode (at the lowest priority level), and uses the `WFE` ARM instruction -/// to sleep when it has no more work to do. When a task is woken, a `SEV` instruction -/// is executed, to make the `WFE` exit from sleep and poll the task. -/// -/// This executor allows for ultra low power consumption for chips where `WFE` -/// triggers low-power sleep without extra steps. If your chip requires extra steps, -/// you may use [`raw::Executor`] directly to program custom behavior. -pub struct Executor { - inner: raw::Executor, - not_send: PhantomData<*mut ()>, +/// TODO +#[derive(Clone, Copy)] +pub struct ExecutorWaker { + irqn: u16, } -impl Executor { - /// Create a new Executor. - pub fn new() -> Self { - Self { - inner: raw::Executor::new(|_| cortex_m::asm::sev(), ptr::null_mut()), - not_send: PhantomData, - } +impl ExecutorWaker { + /// TODO + pub const unsafe fn poison() -> Self { + Self { irqn: 0 } } - /// Run the executor. - /// - /// The `init` closure is called with a [`Spawner`] that spawns tasks on - /// this executor. Use it to spawn the initial task(s). After `init` returns, - /// the executor starts running the tasks. - /// - /// To spawn more tasks later, you may keep copies of the [`Spawner`] (it is `Copy`), - /// for example by passing it as an argument to the initial tasks. - /// - /// This function requires `&'static mut self`. This means you have to store the - /// Executor instance in a place where it'll live forever and grants you mutable - /// access. There's a few ways to do this: - /// - /// - a [Forever](crate::util::Forever) (safe) - /// - a `static mut` (unsafe) - /// - a local variable in a function you know never returns (like `fn main() -> !`), upgrading its lifetime with `transmute`. (unsafe) - /// - /// This function never returns. - pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! { - init(self.inner.spawner()); - - loop { - unsafe { self.inner.poll() }; - cortex_m::asm::wfe(); - } - } -} - -fn pend_by_number(n: u16) { - #[derive(Clone, Copy)] - struct N(u16); - unsafe impl cortex_m::interrupt::InterruptNumber for N { - fn number(self) -> u16 { - self.0 - } + /// TODO + pub fn wake(&self) { + let mut nvic: cortex_m::peripheral::NVIC = unsafe { mem::transmute(()) }; + nvic.request(NrWrap(self.irqn)); } - cortex_m::peripheral::NVIC::pend(N(n)) } /// Interrupt mode executor. @@ -88,19 +44,19 @@ fn pend_by_number(n: u16) { /// /// It is somewhat more complex to use, it's recommended to use the thread-mode /// [`Executor`] instead, if it works for your use case. -pub struct InterruptExecutor { +pub struct Executor { irq: I, inner: raw::Executor, not_send: PhantomData<*mut ()>, } -impl InterruptExecutor { +impl Executor { /// Create a new Executor. pub fn new(irq: I) -> Self { - let ctx = irq.number() as *mut (); + let irqn = irq.number(); Self { irq, - inner: raw::Executor::new(|ctx| pend_by_number(ctx as u16), ctx), + inner: raw::Executor::new(ExecutorWaker { irqn }), not_send: PhantomData, } } diff --git a/embassy/src/executor/arch/arm_wfe.rs b/embassy/src/executor/arch/arm_wfe.rs new file mode 100644 index 0000000000..5a3985234b --- /dev/null +++ b/embassy/src/executor/arch/arm_wfe.rs @@ -0,0 +1,73 @@ +use core::marker::PhantomData; +use core::ptr; + +use super::{raw, Spawner}; +use crate::interrupt::{Interrupt, InterruptExt}; + +/// TODO +#[derive(Clone, Copy)] +pub struct ExecutorWaker {} + +impl ExecutorWaker { + /// TODO + pub const unsafe fn poison() -> Self { + Self {} + } + + /// TODO + pub fn wake(&self) { + cortex_m::asm::sev() + } +} + +/// Thread mode executor, using WFE/SEV. +/// +/// This is the simplest and most common kind of executor. It runs on +/// thread mode (at the lowest priority level), and uses the `WFE` ARM instruction +/// to sleep when it has no more work to do. When a task is woken, a `SEV` instruction +/// is executed, to make the `WFE` exit from sleep and poll the task. +/// +/// This executor allows for ultra low power consumption for chips where `WFE` +/// triggers low-power sleep without extra steps. If your chip requires extra steps, +/// you may use [`raw::Executor`] directly to program custom behavior. +pub struct Executor { + inner: raw::Executor, + not_send: PhantomData<*mut ()>, +} + +impl Executor { + /// Create a new Executor. + pub fn new() -> Self { + Self { + inner: raw::Executor::new(ExecutorWaker {}), + not_send: PhantomData, + } + } + + /// Run the executor. + /// + /// The `init` closure is called with a [`Spawner`] that spawns tasks on + /// this executor. Use it to spawn the initial task(s). After `init` returns, + /// the executor starts running the tasks. + /// + /// To spawn more tasks later, you may keep copies of the [`Spawner`] (it is `Copy`), + /// for example by passing it as an argument to the initial tasks. + /// + /// This function requires `&'static mut self`. This means you have to store the + /// Executor instance in a place where it'll live forever and grants you mutable + /// access. There's a few ways to do this: + /// + /// - a [Forever](crate::util::Forever) (safe) + /// - a `static mut` (unsafe) + /// - a local variable in a function you know never returns (like `fn main() -> !`), upgrading its lifetime with `transmute`. (unsafe) + /// + /// This function never returns. + pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! { + init(self.inner.spawner()); + + loop { + unsafe { self.inner.poll() }; + cortex_m::asm::wfe(); + } + } +} diff --git a/embassy/src/executor/mod.rs b/embassy/src/executor/mod.rs index 3ec24c13c1..52d4bba069 100644 --- a/embassy/src/executor/mod.rs +++ b/embassy/src/executor/mod.rs @@ -4,7 +4,7 @@ #[cfg_attr(feature = "std", path = "arch/std.rs")] #[cfg_attr(feature = "wasm", path = "arch/wasm.rs")] -#[cfg_attr(not(any(feature = "std", feature = "wasm")), path = "arch/arm.rs")] +#[cfg_attr(not(any(feature = "std", feature = "wasm")), path = "arch/arm_irq.rs")] mod arch; pub mod raw; mod spawner; diff --git a/embassy/src/executor/raw/mod.rs b/embassy/src/executor/raw/mod.rs index 5ee38715de..e9b6524f6e 100644 --- a/embassy/src/executor/raw/mod.rs +++ b/embassy/src/executor/raw/mod.rs @@ -24,7 +24,7 @@ use critical_section::CriticalSection; use self::run_queue::{RunQueue, RunQueueItem}; use self::util::UninitCell; -use super::SpawnToken; +use super::{ExecutorWaker, SpawnToken}; #[cfg(feature = "time")] use crate::time::driver::{self, AlarmHandle}; #[cfg(feature = "time")] @@ -217,8 +217,7 @@ unsafe impl Sync for TaskStorage {} /// the requirement for `poll` to not be called reentrantly. pub struct Executor { run_queue: RunQueue, - signal_fn: fn(*mut ()), - signal_ctx: *mut (), + waker: ExecutorWaker, #[cfg(feature = "time")] pub(crate) timer_queue: timer_queue::TimerQueue, @@ -233,16 +232,15 @@ impl Executor { /// `signal_ctx` as argument. /// /// See [`Executor`] docs for details on `signal_fn`. - pub fn new(signal_fn: fn(*mut ()), signal_ctx: *mut ()) -> Self { + pub fn new(waker: ExecutorWaker) -> Self { #[cfg(feature = "time")] let alarm = unsafe { unwrap!(driver::allocate_alarm()) }; #[cfg(feature = "time")] - driver::set_alarm_callback(alarm, signal_fn, signal_ctx); + driver::set_alarm_callback(alarm, waker); Self { run_queue: RunQueue::new(), - signal_fn, - signal_ctx, + waker, #[cfg(feature = "time")] timer_queue: timer_queue::TimerQueue::new(), @@ -259,9 +257,8 @@ impl Executor { /// - `task` must NOT be already enqueued (in this executor or another one). #[inline(always)] unsafe fn enqueue(&self, cs: CriticalSection, task: *mut TaskHeader) { - if self.run_queue.enqueue(cs, task) { - (self.signal_fn)(self.signal_ctx) - } + self.run_queue.enqueue(cs, task); + self.waker.wake(); } /// Spawn a task in this executor. diff --git a/embassy/src/time/driver.rs b/embassy/src/time/driver.rs index a21a29d461..1f51e905bd 100644 --- a/embassy/src/time/driver.rs +++ b/embassy/src/time/driver.rs @@ -46,7 +46,7 @@ //! unsafe fn allocate_alarm(&self) -> Option { //! todo!() //! } -//! fn set_alarm_callback(&self, alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()) { +//! fn set_alarm_callback(&self, alarm: AlarmHandle, signaler: Signaler) { //! todo!() //! } //! fn set_alarm(&self, alarm: AlarmHandle, timestamp: u64) { @@ -55,6 +55,8 @@ //! } //! ``` +use crate::executor::ExecutorWaker; + /// Alarm handle, assigned by the driver. #[derive(Clone, Copy)] pub struct AlarmHandle { @@ -91,9 +93,9 @@ pub trait Driver: Send + Sync + 'static { /// It is UB to make the alarm fire before setting a callback. unsafe fn allocate_alarm(&self) -> Option; - /// Sets the callback function to be called when the alarm triggers. - /// The callback may be called from any context (interrupt or thread mode). - fn set_alarm_callback(&self, alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()); + /// Sets the callback ExecutorWaker to be woken when the alarm triggers. + /// The waker may be called from any context (interrupt or thread mode). + fn set_alarm_callback(&self, alarm: AlarmHandle, callback: ExecutorWaker); /// Sets an alarm at the given timestamp. When the current timestamp reaches the alarm /// timestamp, the provided callback funcion will be called. @@ -110,7 +112,7 @@ pub trait Driver: Send + Sync + 'static { extern "Rust" { fn _embassy_time_now() -> u64; fn _embassy_time_allocate_alarm() -> Option; - fn _embassy_time_set_alarm_callback(alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()); + fn _embassy_time_set_alarm_callback(alarm: AlarmHandle, callback: ExecutorWaker); fn _embassy_time_set_alarm(alarm: AlarmHandle, timestamp: u64); } @@ -121,8 +123,8 @@ pub(crate) fn now() -> u64 { pub(crate) unsafe fn allocate_alarm() -> Option { _embassy_time_allocate_alarm() } -pub(crate) fn set_alarm_callback(alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()) { - unsafe { _embassy_time_set_alarm_callback(alarm, callback, ctx) } +pub(crate) fn set_alarm_callback(alarm: AlarmHandle, callback: ExecutorWaker) { + unsafe { _embassy_time_set_alarm_callback(alarm, callback) } } pub(crate) fn set_alarm(alarm: AlarmHandle, timestamp: u64) { unsafe { _embassy_time_set_alarm(alarm, timestamp) } @@ -149,10 +151,9 @@ macro_rules! time_driver_impl { #[no_mangle] fn _embassy_time_set_alarm_callback( alarm: $crate::time::driver::AlarmHandle, - callback: fn(*mut ()), - ctx: *mut (), + callback: $crate::executor::ExecutorWaker, ) { - <$t as $crate::time::driver::Driver>::set_alarm_callback(&$name, alarm, callback, ctx) + <$t as $crate::time::driver::Driver>::set_alarm_callback(&$name, alarm, callback) } #[no_mangle] diff --git a/examples/nrf/src/bin/multiprio.rs b/examples/nrf/src/bin/multiprio.rs index 85e1269080..6c105fe784 100644 --- a/examples/nrf/src/bin/multiprio.rs +++ b/examples/nrf/src/bin/multiprio.rs @@ -62,7 +62,8 @@ mod example_common; use example_common::*; use cortex_m_rt::entry; -use embassy::executor::{Executor, InterruptExecutor}; +use defmt::panic; +use embassy::executor::Executor; use embassy::interrupt::InterruptExt; use embassy::time::{Duration, Instant, Timer}; use embassy::util::Forever; @@ -110,9 +111,9 @@ async fn run_low() { } } -static EXECUTOR_HIGH: Forever> = Forever::new(); -static EXECUTOR_MED: Forever> = Forever::new(); -static EXECUTOR_LOW: Forever = Forever::new(); +static EXECUTOR_HIGH: Forever> = Forever::new(); +static EXECUTOR_MED: Forever> = Forever::new(); +static EXECUTOR_LOW: Forever> = Forever::new(); #[entry] fn main() -> ! { @@ -120,25 +121,34 @@ fn main() -> ! { let _p = embassy_nrf::init(Default::default()); - // High-priority executor: SWI1_EGU1, priority level 6 - let irq = interrupt::take!(SWI1_EGU1); - irq.set_priority(interrupt::Priority::P6); - let executor = EXECUTOR_HIGH.put(InterruptExecutor::new(irq)); + // High-priority executor: SWI2_EGU2, priority level 5 + let irq = interrupt::take!(SWI2_EGU2); + irq.set_priority(interrupt::Priority::P5); + let executor = EXECUTOR_HIGH.put(Executor::new(irq)); executor.start(|spawner| { unwrap!(spawner.spawn(run_high())); }); - // Medium-priority executor: SWI0_EGU0, priority level 7 - let irq = interrupt::take!(SWI0_EGU0); - irq.set_priority(interrupt::Priority::P7); - let executor = EXECUTOR_MED.put(InterruptExecutor::new(irq)); + // Medium-priority executor: SWI1_EGU1, priority level 6 + let irq = interrupt::take!(SWI1_EGU1); + irq.set_priority(interrupt::Priority::P6); + let executor = EXECUTOR_MED.put(Executor::new(irq)); executor.start(|spawner| { unwrap!(spawner.spawn(run_med())); }); - // Low priority executor: runs in thread mode, using WFE/SEV - let executor = EXECUTOR_LOW.put(Executor::new()); - executor.run(|spawner| { + // Low-priority executor: SWI0_EGU0, priority level 7 + let irq = interrupt::take!(SWI0_EGU0); + irq.set_priority(interrupt::Priority::P7); + let executor = EXECUTOR_LOW.put(Executor::new(irq)); + executor.start(|spawner| { unwrap!(spawner.spawn(run_low())); }); + + let mut scb: cortex_m::peripheral::SCB = unsafe { core::mem::transmute(()) }; + scb.set_sleeponexit(); + + loop { + cortex_m::asm::wfi() + } }