Skip to content

Commit

Permalink
WIP: remove signal_fn in executor.
Browse files Browse the repository at this point in the history
  • Loading branch information
Dirbaio committed Dec 22, 2021
1 parent 79cbad5 commit 50ec141
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 132 deletions.
24 changes: 6 additions & 18 deletions embassy-nrf/src/time_driver.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -61,11 +61,7 @@ mod test {

struct AlarmState {
timestamp: Cell<u64>,

// 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<ExecutorWaker>,
}

unsafe impl Send for AlarmState {}
Expand All @@ -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() }),
}
}
}
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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);
})
}

Expand Down
22 changes: 6 additions & 16 deletions embassy-stm32/src/time_driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -58,10 +58,7 @@ fn calc_now(period: u32, counter: u16) -> u64 {
struct AlarmState {
timestamp: Cell<u64>,

// 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<ExecutorWaker>,
}

unsafe impl Send for AlarmState {}
Expand All @@ -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() }),
}
}
}
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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);
})
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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<I: Interrupt> {
pub struct Executor<I: Interrupt> {
irq: I,
inner: raw::Executor,
not_send: PhantomData<*mut ()>,
}

impl<I: Interrupt> InterruptExecutor<I> {
impl<I: Interrupt> Executor<I> {
/// 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,
}
}
Expand Down
73 changes: 73 additions & 0 deletions embassy/src/executor/arch/arm_wfe.rs
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
2 changes: 1 addition & 1 deletion embassy/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 7 additions & 10 deletions embassy/src/executor/raw/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -217,8 +217,7 @@ unsafe impl<F: Future + 'static> Sync for TaskStorage<F> {}
/// 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,
Expand All @@ -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(),
Expand All @@ -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.
Expand Down
Loading

0 comments on commit 50ec141

Please sign in to comment.