From 5a5495aac43d75610735f2ca80fb6c8e8f31ed71 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Tue, 26 Nov 2024 23:54:21 +0100 Subject: Refactor integrated-timers --- embassy-executor/src/arch/avr.rs | 4 - embassy-executor/src/arch/cortex_m.rs | 6 -- embassy-executor/src/arch/riscv32.rs | 4 - embassy-executor/src/arch/spin.rs | 4 - embassy-executor/src/arch/std.rs | 4 - embassy-executor/src/arch/wasm.rs | 4 - embassy-executor/src/raw/mod.rs | 135 ++++++-------------------------- embassy-executor/src/raw/timer_queue.rs | 89 +++++++++++++-------- embassy-executor/src/raw/util.rs | 5 ++ 9 files changed, 88 insertions(+), 167 deletions(-) (limited to 'embassy-executor/src') diff --git a/embassy-executor/src/arch/avr.rs b/embassy-executor/src/arch/avr.rs index 7f9ed4421..70085d04d 100644 --- a/embassy-executor/src/arch/avr.rs +++ b/embassy-executor/src/arch/avr.rs @@ -53,10 +53,6 @@ mod thread { /// /// This function never returns. pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! { - unsafe { - self.inner.initialize(); - } - init(self.inner.spawner()); loop { diff --git a/embassy-executor/src/arch/cortex_m.rs b/embassy-executor/src/arch/cortex_m.rs index 0c2af88a6..5c517e0a2 100644 --- a/embassy-executor/src/arch/cortex_m.rs +++ b/embassy-executor/src/arch/cortex_m.rs @@ -98,9 +98,6 @@ mod thread { /// /// This function never returns. pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! { - unsafe { - self.inner.initialize(); - } init(self.inner.spawner()); loop { @@ -210,9 +207,6 @@ mod interrupt { } let executor = unsafe { (&*self.executor.get()).assume_init_ref() }; - unsafe { - executor.initialize(); - } unsafe { NVIC::unmask(irq) } diff --git a/embassy-executor/src/arch/riscv32.rs b/embassy-executor/src/arch/riscv32.rs index 715e5f3cf..01e63a9fd 100644 --- a/embassy-executor/src/arch/riscv32.rs +++ b/embassy-executor/src/arch/riscv32.rs @@ -54,10 +54,6 @@ mod thread { /// /// This function never returns. pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! { - unsafe { - self.inner.initialize(); - } - init(self.inner.spawner()); loop { diff --git a/embassy-executor/src/arch/spin.rs b/embassy-executor/src/arch/spin.rs index 54c7458b3..340023620 100644 --- a/embassy-executor/src/arch/spin.rs +++ b/embassy-executor/src/arch/spin.rs @@ -48,10 +48,6 @@ mod thread { /// /// This function never returns. pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! { - unsafe { - self.inner.initialize(); - } - init(self.inner.spawner()); loop { diff --git a/embassy-executor/src/arch/std.rs b/embassy-executor/src/arch/std.rs index 948c7711b..b02b15988 100644 --- a/embassy-executor/src/arch/std.rs +++ b/embassy-executor/src/arch/std.rs @@ -55,10 +55,6 @@ mod thread { /// /// This function never returns. pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! { - unsafe { - self.inner.initialize(); - } - init(self.inner.spawner()); loop { diff --git a/embassy-executor/src/arch/wasm.rs b/embassy-executor/src/arch/wasm.rs index 35025f11f..f9d0f935c 100644 --- a/embassy-executor/src/arch/wasm.rs +++ b/embassy-executor/src/arch/wasm.rs @@ -70,10 +70,6 @@ mod thread { /// - a `static mut` (unsafe) /// - a local variable in a function you know never returns (like `fn main() -> !`), upgrading its lifetime with `transmute`. (unsafe) pub fn start(&'static mut self, init: impl FnOnce(Spawner)) { - unsafe { - self.inner.initialize(); - } - unsafe { let executor = &self.inner; let future = Closure::new(move |_| { diff --git a/embassy-executor/src/raw/mod.rs b/embassy-executor/src/raw/mod.rs index 3f93eae6f..80bd49bad 100644 --- a/embassy-executor/src/raw/mod.rs +++ b/embassy-executor/src/raw/mod.rs @@ -17,7 +17,7 @@ mod run_queue; mod state; #[cfg(feature = "integrated-timers")] -mod timer_queue; +pub mod timer_queue; #[cfg(feature = "trace")] mod trace; pub(crate) mod util; @@ -31,9 +31,6 @@ use core::pin::Pin; use core::ptr::NonNull; use core::task::{Context, Poll}; -#[cfg(feature = "integrated-timers")] -use embassy_time_driver::AlarmHandle; - use self::run_queue::{RunQueue, RunQueueItem}; use self::state::State; use self::util::{SyncUnsafeCell, UninitCell}; @@ -47,8 +44,7 @@ pub(crate) struct TaskHeader { pub(crate) executor: SyncUnsafeCell>, poll_fn: SyncUnsafeCell>, - #[cfg(feature = "integrated-timers")] - pub(crate) expires_at: SyncUnsafeCell, + /// Integrated timer queue storage. This field should not be accessed outside of the timer queue. #[cfg(feature = "integrated-timers")] pub(crate) timer_queue_item: timer_queue::TimerQueueItem, } @@ -80,6 +76,12 @@ impl TaskRef { unsafe { self.ptr.as_ref() } } + /// Returns a reference to the executor that the task is currently running on. + #[cfg(feature = "integrated-timers")] + pub unsafe fn executor(self) -> Option<&'static Executor> { + self.header().executor.get().map(|e| Executor::wrap(e)) + } + /// The returned pointer is valid for the entire TaskStorage. pub(crate) fn as_ptr(self) -> *const TaskHeader { self.ptr.as_ptr() @@ -120,8 +122,6 @@ impl TaskStorage { // Note: this is lazily initialized so that a static `TaskStorage` will go in `.bss` poll_fn: SyncUnsafeCell::new(None), - #[cfg(feature = "integrated-timers")] - expires_at: SyncUnsafeCell::new(0), #[cfg(feature = "integrated-timers")] timer_queue_item: timer_queue::TimerQueueItem::new(), }, @@ -160,9 +160,6 @@ impl TaskStorage { Poll::Ready(_) => { this.future.drop_in_place(); this.raw.state.despawn(); - - #[cfg(feature = "integrated-timers")] - this.raw.expires_at.set(u64::MAX); } Poll::Pending => {} } @@ -316,34 +313,16 @@ impl Pender { pub(crate) struct SyncExecutor { run_queue: RunQueue, pender: Pender, - - #[cfg(feature = "integrated-timers")] - pub(crate) timer_queue: timer_queue::TimerQueue, - #[cfg(feature = "integrated-timers")] - alarm: AlarmHandle, } impl SyncExecutor { pub(crate) fn new(pender: Pender) -> Self { - #[cfg(feature = "integrated-timers")] - let alarm = unsafe { unwrap!(embassy_time_driver::allocate_alarm()) }; - Self { run_queue: RunQueue::new(), pender, - - #[cfg(feature = "integrated-timers")] - timer_queue: timer_queue::TimerQueue::new(), - #[cfg(feature = "integrated-timers")] - alarm, } } - pub(crate) unsafe fn initialize(&'static self) { - #[cfg(feature = "integrated-timers")] - embassy_time_driver::set_alarm_callback(self.alarm, Self::alarm_callback, self as *const _ as *mut ()); - } - /// Enqueue a task in the task queue /// /// # Safety @@ -360,12 +339,6 @@ impl SyncExecutor { } } - #[cfg(feature = "integrated-timers")] - fn alarm_callback(ctx: *mut ()) { - let this: &Self = unsafe { &*(ctx as *const Self) }; - this.pender.pend(); - } - pub(super) unsafe fn spawn(&'static self, task: TaskRef) { task.header().executor.set(Some(self)); @@ -379,56 +352,27 @@ impl SyncExecutor { /// /// Same as [`Executor::poll`], plus you must only call this on the thread this executor was created. pub(crate) unsafe fn poll(&'static self) { - #[allow(clippy::never_loop)] - loop { - #[cfg(feature = "integrated-timers")] - self.timer_queue - .dequeue_expired(embassy_time_driver::now(), wake_task_no_pend); - - self.run_queue.dequeue_all(|p| { - let task = p.header(); - - #[cfg(feature = "integrated-timers")] - task.expires_at.set(u64::MAX); - - if !task.state.run_dequeue() { - // If task is not running, ignore it. This can happen in the following scenario: - // - Task gets dequeued, poll starts - // - While task is being polled, it gets woken. It gets placed in the queue. - // - Task poll finishes, returning done=true - // - RUNNING bit is cleared, but the task is already in the queue. - return; - } - - #[cfg(feature = "trace")] - trace::task_exec_begin(self, &p); + self.run_queue.dequeue_all(|p| { + let task = p.header(); + + if !task.state.run_dequeue() { + // If task is not running, ignore it. This can happen in the following scenario: + // - Task gets dequeued, poll starts + // - While task is being polled, it gets woken. It gets placed in the queue. + // - Task poll finishes, returning done=true + // - RUNNING bit is cleared, but the task is already in the queue. + return; + } - // Run the task - task.poll_fn.get().unwrap_unchecked()(p); + #[cfg(feature = "trace")] + trace::task_exec_begin(self, &p); - #[cfg(feature = "trace")] - trace::task_exec_end(self, &p); + // Run the task + task.poll_fn.get().unwrap_unchecked()(p); - // Enqueue or update into timer_queue - #[cfg(feature = "integrated-timers")] - self.timer_queue.update(p); - }); - - #[cfg(feature = "integrated-timers")] - { - // If this is already in the past, set_alarm might return false - // In that case do another poll loop iteration. - let next_expiration = self.timer_queue.next_expiration(); - if embassy_time_driver::set_alarm(self.alarm, next_expiration) { - break; - } - } - - #[cfg(not(feature = "integrated-timers"))] - { - break; - } - } + #[cfg(feature = "trace")] + trace::task_exec_end(self, &p); + }); #[cfg(feature = "trace")] trace::executor_idle(self) @@ -494,15 +438,6 @@ impl Executor { } } - /// Initializes the executor. - /// - /// # Safety - /// - /// This function must be called once before any other method is called. - pub unsafe fn initialize(&'static self) { - self.inner.initialize(); - } - /// Spawn a task in this executor. /// /// # Safety @@ -575,21 +510,3 @@ pub fn wake_task_no_pend(task: TaskRef) { } } } - -#[cfg(feature = "integrated-timers")] -struct TimerQueue; - -#[cfg(feature = "integrated-timers")] -impl embassy_time_queue_driver::TimerQueue for TimerQueue { - fn schedule_wake(&'static self, at: u64, waker: &core::task::Waker) { - let task = waker::task_from_waker(waker); - let task = task.header(); - unsafe { - let expires_at = task.expires_at.get(); - task.expires_at.set(expires_at.min(at)); - } - } -} - -#[cfg(feature = "integrated-timers")] -embassy_time_queue_driver::timer_queue_impl!(static TIMER_QUEUE: TimerQueue = TimerQueue); diff --git a/embassy-executor/src/raw/timer_queue.rs b/embassy-executor/src/raw/timer_queue.rs index 94a5f340b..953bf014f 100644 --- a/embassy-executor/src/raw/timer_queue.rs +++ b/embassy-executor/src/raw/timer_queue.rs @@ -1,75 +1,100 @@ +//! Timer queue operations. use core::cmp::min; +use super::util::SyncUnsafeCell; use super::TaskRef; -use crate::raw::util::SyncUnsafeCell; pub(crate) struct TimerQueueItem { next: SyncUnsafeCell>, + expires_at: SyncUnsafeCell, } impl TimerQueueItem { pub const fn new() -> Self { Self { next: SyncUnsafeCell::new(None), + expires_at: SyncUnsafeCell::new(0), } } } -pub(crate) struct TimerQueue { +/// A timer queue, with items integrated into tasks. +pub struct TimerQueue { head: SyncUnsafeCell>, } impl TimerQueue { + /// Creates a new timer queue. pub const fn new() -> Self { Self { head: SyncUnsafeCell::new(None), } } - pub(crate) unsafe fn update(&self, p: TaskRef) { - let task = p.header(); - if task.expires_at.get() != u64::MAX { + /// Schedules a task to run at a specific time. + /// + /// If this function returns `true`, the called should find the next expiration time and set + /// a new alarm for that time. + pub fn schedule_wake(&mut self, at: u64, p: TaskRef) -> bool { + unsafe { + let task = p.header(); + let item = &task.timer_queue_item; if task.state.timer_enqueue() { - task.timer_queue_item.next.set(self.head.get()); - self.head.set(Some(p)); + // If not in the queue, add it and update. + let prev = self.head.replace(Some(p)); + item.next.set(prev); + } else if at <= item.expires_at.get() { + // If expiration is sooner than previously set, update. + } else { + // Task does not need to be updated. + return false; } + + item.expires_at.set(at); + true } } - pub(crate) unsafe fn next_expiration(&self) -> u64 { - let mut res = u64::MAX; - self.retain(|p| { - let task = p.header(); - let expires = task.expires_at.get(); - res = min(res, expires); - expires != u64::MAX - }); - res - } + /// Dequeues expired timers and returns the next alarm time. + /// + /// The provided callback will be called for each expired task. Tasks that never expire + /// will be removed, but the callback will not be called. + pub fn next_expiration(&mut self, now: u64) -> u64 { + let mut next_expiration = u64::MAX; - pub(crate) unsafe fn dequeue_expired(&self, now: u64, on_task: impl Fn(TaskRef)) { self.retain(|p| { let task = p.header(); - if task.expires_at.get() <= now { - on_task(p); + let item = &task.timer_queue_item; + let expires = unsafe { item.expires_at.get() }; + + if expires <= now { + // Timer expired, process task. + super::wake_task(p); false } else { - true + // Timer didn't yet expire, or never expires. + next_expiration = min(next_expiration, expires); + expires != u64::MAX } }); + + next_expiration } - pub(crate) unsafe fn retain(&self, mut f: impl FnMut(TaskRef) -> bool) { - let mut prev = &self.head; - while let Some(p) = prev.get() { - let task = p.header(); - if f(p) { - // Skip to next - prev = &task.timer_queue_item.next; - } else { - // Remove it - prev.set(task.timer_queue_item.next.get()); - task.state.timer_dequeue(); + fn retain(&self, mut f: impl FnMut(TaskRef) -> bool) { + unsafe { + let mut prev = &self.head; + while let Some(p) = prev.get() { + let task = p.header(); + let item = &task.timer_queue_item; + if f(p) { + // Skip to next + prev = &item.next; + } else { + // Remove it + prev.set(item.next.get()); + task.state.timer_dequeue(); + } } } } diff --git a/embassy-executor/src/raw/util.rs b/embassy-executor/src/raw/util.rs index c46085e45..e2633658a 100644 --- a/embassy-executor/src/raw/util.rs +++ b/embassy-executor/src/raw/util.rs @@ -54,4 +54,9 @@ impl SyncUnsafeCell { { *self.value.get() } + + #[cfg(feature = "integrated-timers")] + pub unsafe fn replace(&self, value: T) -> T { + core::mem::replace(&mut *self.value.get(), value) + } } -- cgit From 12f58fbcfd3f10b43795936127a890c6a0f8f280 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Sun, 8 Dec 2024 23:04:43 +0100 Subject: Remove TIMER_QUEUED state --- embassy-executor/src/raw/state_atomics.rs | 18 ------------------ embassy-executor/src/raw/state_atomics_arm.rs | 19 ++----------------- embassy-executor/src/raw/state_critical_section.rs | 21 --------------------- embassy-executor/src/raw/timer_queue.rs | 4 ++-- 4 files changed, 4 insertions(+), 58 deletions(-) (limited to 'embassy-executor/src') diff --git a/embassy-executor/src/raw/state_atomics.rs b/embassy-executor/src/raw/state_atomics.rs index e1279ac0b..e4127897e 100644 --- a/embassy-executor/src/raw/state_atomics.rs +++ b/embassy-executor/src/raw/state_atomics.rs @@ -4,9 +4,6 @@ use core::sync::atomic::{AtomicU32, Ordering}; pub(crate) const STATE_SPAWNED: u32 = 1 << 0; /// Task is in the executor run queue pub(crate) const STATE_RUN_QUEUED: u32 = 1 << 1; -/// Task is in the executor timer queue -#[cfg(feature = "integrated-timers")] -pub(crate) const STATE_TIMER_QUEUED: u32 = 1 << 2; pub(crate) struct State { state: AtomicU32, @@ -55,19 +52,4 @@ impl State { let state = self.state.fetch_and(!STATE_RUN_QUEUED, Ordering::AcqRel); state & STATE_SPAWNED != 0 } - - /// Mark the task as timer-queued. Return whether it was newly queued (i.e. not queued before) - #[cfg(feature = "integrated-timers")] - #[inline(always)] - pub fn timer_enqueue(&self) -> bool { - let old_state = self.state.fetch_or(STATE_TIMER_QUEUED, Ordering::AcqRel); - old_state & STATE_TIMER_QUEUED == 0 - } - - /// Unmark the task as timer-queued. - #[cfg(feature = "integrated-timers")] - #[inline(always)] - pub fn timer_dequeue(&self) { - self.state.fetch_and(!STATE_TIMER_QUEUED, Ordering::AcqRel); - } } diff --git a/embassy-executor/src/raw/state_atomics_arm.rs b/embassy-executor/src/raw/state_atomics_arm.rs index e4dfe5093..b673c7359 100644 --- a/embassy-executor/src/raw/state_atomics_arm.rs +++ b/embassy-executor/src/raw/state_atomics_arm.rs @@ -11,9 +11,8 @@ pub(crate) struct State { spawned: AtomicBool, /// Task is in the executor run queue run_queued: AtomicBool, - /// Task is in the executor timer queue - timer_queued: AtomicBool, pad: AtomicBool, + pad2: AtomicBool, } impl State { @@ -21,8 +20,8 @@ impl State { Self { spawned: AtomicBool::new(false), run_queued: AtomicBool::new(false), - timer_queued: AtomicBool::new(false), pad: AtomicBool::new(false), + pad2: AtomicBool::new(false), } } @@ -86,18 +85,4 @@ impl State { self.run_queued.store(false, Ordering::Relaxed); r } - - /// Mark the task as timer-queued. Return whether it was newly queued (i.e. not queued before) - #[cfg(feature = "integrated-timers")] - #[inline(always)] - pub fn timer_enqueue(&self) -> bool { - !self.timer_queued.swap(true, Ordering::Relaxed) - } - - /// Unmark the task as timer-queued. - #[cfg(feature = "integrated-timers")] - #[inline(always)] - pub fn timer_dequeue(&self) { - self.timer_queued.store(false, Ordering::Relaxed); - } } diff --git a/embassy-executor/src/raw/state_critical_section.rs b/embassy-executor/src/raw/state_critical_section.rs index c3cc1b0b7..b92eed006 100644 --- a/embassy-executor/src/raw/state_critical_section.rs +++ b/embassy-executor/src/raw/state_critical_section.rs @@ -6,9 +6,6 @@ use critical_section::Mutex; pub(crate) const STATE_SPAWNED: u32 = 1 << 0; /// Task is in the executor run queue pub(crate) const STATE_RUN_QUEUED: u32 = 1 << 1; -/// Task is in the executor timer queue -#[cfg(feature = "integrated-timers")] -pub(crate) const STATE_TIMER_QUEUED: u32 = 1 << 2; pub(crate) struct State { state: Mutex>, @@ -72,22 +69,4 @@ impl State { ok }) } - - /// Mark the task as timer-queued. Return whether it was newly queued (i.e. not queued before) - #[cfg(feature = "integrated-timers")] - #[inline(always)] - pub fn timer_enqueue(&self) -> bool { - self.update(|s| { - let ok = *s & STATE_TIMER_QUEUED == 0; - *s |= STATE_TIMER_QUEUED; - ok - }) - } - - /// Unmark the task as timer-queued. - #[cfg(feature = "integrated-timers")] - #[inline(always)] - pub fn timer_dequeue(&self) { - self.update(|s| *s &= !STATE_TIMER_QUEUED); - } } diff --git a/embassy-executor/src/raw/timer_queue.rs b/embassy-executor/src/raw/timer_queue.rs index 953bf014f..513397090 100644 --- a/embassy-executor/src/raw/timer_queue.rs +++ b/embassy-executor/src/raw/timer_queue.rs @@ -39,7 +39,7 @@ impl TimerQueue { unsafe { let task = p.header(); let item = &task.timer_queue_item; - if task.state.timer_enqueue() { + if item.next.get().is_none() { // If not in the queue, add it and update. let prev = self.head.replace(Some(p)); item.next.set(prev); @@ -93,7 +93,7 @@ impl TimerQueue { } else { // Remove it prev.set(item.next.get()); - task.state.timer_dequeue(); + item.next.set(None); } } } -- cgit From dc18ee29a0f93ce34892731ee0580a3e9e3f2298 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Sun, 8 Dec 2024 23:07:35 +0100 Subject: Do not access task header --- embassy-executor/src/raw/mod.rs | 6 ++++++ embassy-executor/src/raw/timer_queue.rs | 14 ++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) (limited to 'embassy-executor/src') diff --git a/embassy-executor/src/raw/mod.rs b/embassy-executor/src/raw/mod.rs index 80bd49bad..f9c6509f1 100644 --- a/embassy-executor/src/raw/mod.rs +++ b/embassy-executor/src/raw/mod.rs @@ -82,6 +82,12 @@ impl TaskRef { self.header().executor.get().map(|e| Executor::wrap(e)) } + /// Returns a reference to the timer queue item. + #[cfg(feature = "integrated-timers")] + pub fn timer_queue_item(&self) -> &'static timer_queue::TimerQueueItem { + &self.header().timer_queue_item + } + /// The returned pointer is valid for the entire TaskStorage. pub(crate) fn as_ptr(self) -> *const TaskHeader { self.ptr.as_ptr() diff --git a/embassy-executor/src/raw/timer_queue.rs b/embassy-executor/src/raw/timer_queue.rs index 513397090..e0a22f4d4 100644 --- a/embassy-executor/src/raw/timer_queue.rs +++ b/embassy-executor/src/raw/timer_queue.rs @@ -4,13 +4,14 @@ use core::cmp::min; use super::util::SyncUnsafeCell; use super::TaskRef; -pub(crate) struct TimerQueueItem { +/// An item in the timer queue. +pub struct TimerQueueItem { next: SyncUnsafeCell>, expires_at: SyncUnsafeCell, } impl TimerQueueItem { - pub const fn new() -> Self { + pub(crate) const fn new() -> Self { Self { next: SyncUnsafeCell::new(None), expires_at: SyncUnsafeCell::new(0), @@ -37,8 +38,7 @@ impl TimerQueue { /// a new alarm for that time. pub fn schedule_wake(&mut self, at: u64, p: TaskRef) -> bool { unsafe { - let task = p.header(); - let item = &task.timer_queue_item; + let item = p.timer_queue_item(); if item.next.get().is_none() { // If not in the queue, add it and update. let prev = self.head.replace(Some(p)); @@ -63,8 +63,7 @@ impl TimerQueue { let mut next_expiration = u64::MAX; self.retain(|p| { - let task = p.header(); - let item = &task.timer_queue_item; + let item = p.timer_queue_item(); let expires = unsafe { item.expires_at.get() }; if expires <= now { @@ -85,8 +84,7 @@ impl TimerQueue { unsafe { let mut prev = &self.head; while let Some(p) = prev.get() { - let task = p.header(); - let item = &task.timer_queue_item; + let item = p.timer_queue_item(); if f(p) { // Skip to next prev = &item.next; -- cgit From d45ea43892198484b5f6dcea4c351dc11d226cc4 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Sun, 8 Dec 2024 23:21:53 +0100 Subject: Move integrated timer queue into time-queue-driver --- embassy-executor/src/raw/timer_queue.rs | 96 ++++----------------------------- embassy-executor/src/raw/util.rs | 5 -- 2 files changed, 11 insertions(+), 90 deletions(-) (limited to 'embassy-executor/src') diff --git a/embassy-executor/src/raw/timer_queue.rs b/embassy-executor/src/raw/timer_queue.rs index e0a22f4d4..46e346c1b 100644 --- a/embassy-executor/src/raw/timer_queue.rs +++ b/embassy-executor/src/raw/timer_queue.rs @@ -1,99 +1,25 @@ //! Timer queue operations. -use core::cmp::min; -use super::util::SyncUnsafeCell; +use core::cell::Cell; + use super::TaskRef; /// An item in the timer queue. pub struct TimerQueueItem { - next: SyncUnsafeCell>, - expires_at: SyncUnsafeCell, -} + /// The next item in the queue. + pub next: Cell>, -impl TimerQueueItem { - pub(crate) const fn new() -> Self { - Self { - next: SyncUnsafeCell::new(None), - expires_at: SyncUnsafeCell::new(0), - } - } + /// The time at which this item expires. + pub expires_at: Cell, } -/// A timer queue, with items integrated into tasks. -pub struct TimerQueue { - head: SyncUnsafeCell>, -} +unsafe impl Sync for TimerQueueItem {} -impl TimerQueue { - /// Creates a new timer queue. - pub const fn new() -> Self { +impl TimerQueueItem { + pub(crate) const fn new() -> Self { Self { - head: SyncUnsafeCell::new(None), - } - } - - /// Schedules a task to run at a specific time. - /// - /// If this function returns `true`, the called should find the next expiration time and set - /// a new alarm for that time. - pub fn schedule_wake(&mut self, at: u64, p: TaskRef) -> bool { - unsafe { - let item = p.timer_queue_item(); - if item.next.get().is_none() { - // If not in the queue, add it and update. - let prev = self.head.replace(Some(p)); - item.next.set(prev); - } else if at <= item.expires_at.get() { - // If expiration is sooner than previously set, update. - } else { - // Task does not need to be updated. - return false; - } - - item.expires_at.set(at); - true - } - } - - /// Dequeues expired timers and returns the next alarm time. - /// - /// The provided callback will be called for each expired task. Tasks that never expire - /// will be removed, but the callback will not be called. - pub fn next_expiration(&mut self, now: u64) -> u64 { - let mut next_expiration = u64::MAX; - - self.retain(|p| { - let item = p.timer_queue_item(); - let expires = unsafe { item.expires_at.get() }; - - if expires <= now { - // Timer expired, process task. - super::wake_task(p); - false - } else { - // Timer didn't yet expire, or never expires. - next_expiration = min(next_expiration, expires); - expires != u64::MAX - } - }); - - next_expiration - } - - fn retain(&self, mut f: impl FnMut(TaskRef) -> bool) { - unsafe { - let mut prev = &self.head; - while let Some(p) = prev.get() { - let item = p.timer_queue_item(); - if f(p) { - // Skip to next - prev = &item.next; - } else { - // Remove it - prev.set(item.next.get()); - item.next.set(None); - } - } + next: Cell::new(None), + expires_at: Cell::new(0), } } } diff --git a/embassy-executor/src/raw/util.rs b/embassy-executor/src/raw/util.rs index e2633658a..c46085e45 100644 --- a/embassy-executor/src/raw/util.rs +++ b/embassy-executor/src/raw/util.rs @@ -54,9 +54,4 @@ impl SyncUnsafeCell { { *self.value.get() } - - #[cfg(feature = "integrated-timers")] - pub unsafe fn replace(&self, value: T) -> T { - core::mem::replace(&mut *self.value.get(), value) - } } -- cgit From ec96395d084d5edc8be25ddaea8547e2ebd447a6 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Mon, 9 Dec 2024 08:43:57 +0100 Subject: Prevent task from respawning while in the timer queue --- embassy-executor/src/raw/mod.rs | 36 ++++++++++++++++++- embassy-executor/src/raw/state_atomics.rs | 36 +++++++++++++++++++ embassy-executor/src/raw/state_atomics_arm.rs | 40 ++++++++++++++++++++-- embassy-executor/src/raw/state_critical_section.rs | 29 ++++++++++++++++ embassy-executor/src/raw/timer_queue.rs | 15 +++++++- 5 files changed, 152 insertions(+), 4 deletions(-) (limited to 'embassy-executor/src') diff --git a/embassy-executor/src/raw/mod.rs b/embassy-executor/src/raw/mod.rs index f9c6509f1..14d689900 100644 --- a/embassy-executor/src/raw/mod.rs +++ b/embassy-executor/src/raw/mod.rs @@ -50,7 +50,7 @@ pub(crate) struct TaskHeader { } /// This is essentially a `&'static TaskStorage` where the type of the future has been erased. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, PartialEq)] pub struct TaskRef { ptr: NonNull, } @@ -72,6 +72,16 @@ impl TaskRef { } } + /// # Safety + /// + /// The result of this function must only be compared + /// for equality, or stored, but not used. + pub const unsafe fn dangling() -> Self { + Self { + ptr: NonNull::dangling(), + } + } + pub(crate) fn header(self) -> &'static TaskHeader { unsafe { self.ptr.as_ref() } } @@ -88,6 +98,30 @@ impl TaskRef { &self.header().timer_queue_item } + /// Mark the task as timer-queued. Return whether it was newly queued (i.e. not queued before) + /// + /// Entering this state prevents the task from being respawned while in a timer queue. + /// + /// Safety: + /// + /// This functions should only be called by the timer queue implementation, before + /// enqueueing the timer item. + #[cfg(feature = "integrated-timers")] + pub unsafe fn timer_enqueue(&self) -> timer_queue::TimerEnqueueOperation { + self.header().state.timer_enqueue() + } + + /// Unmark the task as timer-queued. + /// + /// Safety: + /// + /// This functions should only be called by the timer queue implementation, after the task has + /// been removed from the timer queue. + #[cfg(feature = "integrated-timers")] + pub unsafe fn timer_dequeue(&self) { + self.header().state.timer_dequeue() + } + /// The returned pointer is valid for the entire TaskStorage. pub(crate) fn as_ptr(self) -> *const TaskHeader { self.ptr.as_ptr() diff --git a/embassy-executor/src/raw/state_atomics.rs b/embassy-executor/src/raw/state_atomics.rs index e4127897e..d03c61ade 100644 --- a/embassy-executor/src/raw/state_atomics.rs +++ b/embassy-executor/src/raw/state_atomics.rs @@ -1,9 +1,15 @@ use core::sync::atomic::{AtomicU32, Ordering}; +#[cfg(feature = "integrated-timers")] +use super::timer_queue::TimerEnqueueOperation; + /// Task is spawned (has a future) pub(crate) const STATE_SPAWNED: u32 = 1 << 0; /// Task is in the executor run queue pub(crate) const STATE_RUN_QUEUED: u32 = 1 << 1; +/// Task is in the executor timer queue +#[cfg(feature = "integrated-timers")] +pub(crate) const STATE_TIMER_QUEUED: u32 = 1 << 2; pub(crate) struct State { state: AtomicU32, @@ -52,4 +58,34 @@ impl State { let state = self.state.fetch_and(!STATE_RUN_QUEUED, Ordering::AcqRel); state & STATE_SPAWNED != 0 } + + /// Mark the task as timer-queued. Return whether it can be enqueued. + #[cfg(feature = "integrated-timers")] + #[inline(always)] + pub fn timer_enqueue(&self) -> TimerEnqueueOperation { + if self + .state + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |state| { + // If not started, ignore it + if state & STATE_SPAWNED == 0 { + None + } else { + // Mark it as enqueued + Some(state | STATE_TIMER_QUEUED) + } + }) + .is_ok() + { + TimerEnqueueOperation::Enqueue + } else { + TimerEnqueueOperation::Ignore + } + } + + /// Unmark the task as timer-queued. + #[cfg(feature = "integrated-timers")] + #[inline(always)] + pub fn timer_dequeue(&self) { + self.state.fetch_and(!STATE_TIMER_QUEUED, Ordering::Relaxed); + } } diff --git a/embassy-executor/src/raw/state_atomics_arm.rs b/embassy-executor/src/raw/state_atomics_arm.rs index b673c7359..f6f2e8f08 100644 --- a/embassy-executor/src/raw/state_atomics_arm.rs +++ b/embassy-executor/src/raw/state_atomics_arm.rs @@ -1,9 +1,14 @@ use core::arch::asm; use core::sync::atomic::{compiler_fence, AtomicBool, AtomicU32, Ordering}; +#[cfg(feature = "integrated-timers")] +use super::timer_queue::TimerEnqueueOperation; + // Must be kept in sync with the layout of `State`! pub(crate) const STATE_SPAWNED: u32 = 1 << 0; pub(crate) const STATE_RUN_QUEUED: u32 = 1 << 8; +#[cfg(feature = "integrated-timers")] +pub(crate) const STATE_TIMER_QUEUED: u32 = 1 << 16; #[repr(C, align(4))] pub(crate) struct State { @@ -11,8 +16,9 @@ pub(crate) struct State { spawned: AtomicBool, /// Task is in the executor run queue run_queued: AtomicBool, + /// Task is in the executor timer queue + timer_queued: AtomicBool, pad: AtomicBool, - pad2: AtomicBool, } impl State { @@ -20,8 +26,8 @@ impl State { Self { spawned: AtomicBool::new(false), run_queued: AtomicBool::new(false), + timer_queued: AtomicBool::new(false), pad: AtomicBool::new(false), - pad2: AtomicBool::new(false), } } @@ -85,4 +91,34 @@ impl State { self.run_queued.store(false, Ordering::Relaxed); r } + + /// Mark the task as timer-queued. Return whether it can be enqueued. + #[cfg(feature = "integrated-timers")] + #[inline(always)] + pub fn timer_enqueue(&self) -> TimerEnqueueOperation { + if self + .as_u32() + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |state| { + // If not started, ignore it + if state & STATE_SPAWNED == 0 { + None + } else { + // Mark it as enqueued + Some(state | STATE_TIMER_QUEUED) + } + }) + .is_ok() + { + TimerEnqueueOperation::Enqueue + } else { + TimerEnqueueOperation::Ignore + } + } + + /// Unmark the task as timer-queued. + #[cfg(feature = "integrated-timers")] + #[inline(always)] + pub fn timer_dequeue(&self) { + self.timer_queued.store(false, Ordering::Relaxed); + } } diff --git a/embassy-executor/src/raw/state_critical_section.rs b/embassy-executor/src/raw/state_critical_section.rs index b92eed006..c0ec2f530 100644 --- a/embassy-executor/src/raw/state_critical_section.rs +++ b/embassy-executor/src/raw/state_critical_section.rs @@ -2,10 +2,16 @@ use core::cell::Cell; use critical_section::Mutex; +#[cfg(feature = "integrated-timers")] +use super::timer_queue::TimerEnqueueOperation; + /// Task is spawned (has a future) pub(crate) const STATE_SPAWNED: u32 = 1 << 0; /// Task is in the executor run queue pub(crate) const STATE_RUN_QUEUED: u32 = 1 << 1; +/// Task is in the executor timer queue +#[cfg(feature = "integrated-timers")] +pub(crate) const STATE_TIMER_QUEUED: u32 = 1 << 2; pub(crate) struct State { state: Mutex>, @@ -69,4 +75,27 @@ impl State { ok }) } + + /// Mark the task as timer-queued. Return whether it can be enqueued. + #[cfg(feature = "integrated-timers")] + #[inline(always)] + pub fn timer_enqueue(&self) -> TimerEnqueueOperation { + self.update(|s| { + // FIXME: we need to split SPAWNED into two phases, to prevent enqueueing a task that is + // just being spawned, because its executor pointer may still be changing. + if *s & STATE_SPAWNED == STATE_SPAWNED { + *s |= STATE_TIMER_QUEUED; + TimerEnqueueOperation::Enqueue + } else { + TimerEnqueueOperation::Ignore + } + }) + } + + /// Unmark the task as timer-queued. + #[cfg(feature = "integrated-timers")] + #[inline(always)] + pub fn timer_dequeue(&self) { + self.update(|s| *s &= !STATE_TIMER_QUEUED); + } } diff --git a/embassy-executor/src/raw/timer_queue.rs b/embassy-executor/src/raw/timer_queue.rs index 46e346c1b..c36708401 100644 --- a/embassy-executor/src/raw/timer_queue.rs +++ b/embassy-executor/src/raw/timer_queue.rs @@ -7,6 +7,9 @@ use super::TaskRef; /// An item in the timer queue. pub struct TimerQueueItem { /// The next item in the queue. + /// + /// If this field contains `Some`, the item is in the queue. The last item in the queue has a + /// value of `Some(dangling_pointer)` pub next: Cell>, /// The time at which this item expires. @@ -19,7 +22,17 @@ impl TimerQueueItem { pub(crate) const fn new() -> Self { Self { next: Cell::new(None), - expires_at: Cell::new(0), + expires_at: Cell::new(u64::MAX), } } } + +/// The operation to perform after `timer_enqueue` is called. +#[derive(Debug, Copy, Clone, PartialEq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum TimerEnqueueOperation { + /// Enqueue the task. + Enqueue, + /// Update the task's expiration time. + Ignore, +} -- cgit From 2f2e2c6031a1abaecdac5ed2febe109e647fe6fd Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 9 Dec 2024 00:28:14 +0100 Subject: Make `integrated-timers` the default, remove Cargo feature. --- embassy-executor/src/raw/mod.rs | 7 ------- embassy-executor/src/raw/state_atomics.rs | 4 ---- embassy-executor/src/raw/state_atomics_arm.rs | 4 ---- embassy-executor/src/raw/state_critical_section.rs | 4 ---- embassy-executor/src/raw/trace.rs | 22 ++++++++-------------- 5 files changed, 8 insertions(+), 33 deletions(-) (limited to 'embassy-executor/src') diff --git a/embassy-executor/src/raw/mod.rs b/embassy-executor/src/raw/mod.rs index 14d689900..2feaab155 100644 --- a/embassy-executor/src/raw/mod.rs +++ b/embassy-executor/src/raw/mod.rs @@ -16,7 +16,6 @@ mod run_queue; #[cfg_attr(not(target_has_atomic = "8"), path = "state_critical_section.rs")] mod state; -#[cfg(feature = "integrated-timers")] pub mod timer_queue; #[cfg(feature = "trace")] mod trace; @@ -45,7 +44,6 @@ pub(crate) struct TaskHeader { poll_fn: SyncUnsafeCell>, /// Integrated timer queue storage. This field should not be accessed outside of the timer queue. - #[cfg(feature = "integrated-timers")] pub(crate) timer_queue_item: timer_queue::TimerQueueItem, } @@ -87,13 +85,11 @@ impl TaskRef { } /// Returns a reference to the executor that the task is currently running on. - #[cfg(feature = "integrated-timers")] pub unsafe fn executor(self) -> Option<&'static Executor> { self.header().executor.get().map(|e| Executor::wrap(e)) } /// Returns a reference to the timer queue item. - #[cfg(feature = "integrated-timers")] pub fn timer_queue_item(&self) -> &'static timer_queue::TimerQueueItem { &self.header().timer_queue_item } @@ -106,7 +102,6 @@ impl TaskRef { /// /// This functions should only be called by the timer queue implementation, before /// enqueueing the timer item. - #[cfg(feature = "integrated-timers")] pub unsafe fn timer_enqueue(&self) -> timer_queue::TimerEnqueueOperation { self.header().state.timer_enqueue() } @@ -117,7 +112,6 @@ impl TaskRef { /// /// This functions should only be called by the timer queue implementation, after the task has /// been removed from the timer queue. - #[cfg(feature = "integrated-timers")] pub unsafe fn timer_dequeue(&self) { self.header().state.timer_dequeue() } @@ -162,7 +156,6 @@ impl TaskStorage { // Note: this is lazily initialized so that a static `TaskStorage` will go in `.bss` poll_fn: SyncUnsafeCell::new(None), - #[cfg(feature = "integrated-timers")] timer_queue_item: timer_queue::TimerQueueItem::new(), }, future: UninitCell::uninit(), diff --git a/embassy-executor/src/raw/state_atomics.rs b/embassy-executor/src/raw/state_atomics.rs index d03c61ade..15eb9a368 100644 --- a/embassy-executor/src/raw/state_atomics.rs +++ b/embassy-executor/src/raw/state_atomics.rs @@ -1,6 +1,5 @@ use core::sync::atomic::{AtomicU32, Ordering}; -#[cfg(feature = "integrated-timers")] use super::timer_queue::TimerEnqueueOperation; /// Task is spawned (has a future) @@ -8,7 +7,6 @@ pub(crate) const STATE_SPAWNED: u32 = 1 << 0; /// Task is in the executor run queue pub(crate) const STATE_RUN_QUEUED: u32 = 1 << 1; /// Task is in the executor timer queue -#[cfg(feature = "integrated-timers")] pub(crate) const STATE_TIMER_QUEUED: u32 = 1 << 2; pub(crate) struct State { @@ -60,7 +58,6 @@ impl State { } /// Mark the task as timer-queued. Return whether it can be enqueued. - #[cfg(feature = "integrated-timers")] #[inline(always)] pub fn timer_enqueue(&self) -> TimerEnqueueOperation { if self @@ -83,7 +80,6 @@ impl State { } /// Unmark the task as timer-queued. - #[cfg(feature = "integrated-timers")] #[inline(always)] pub fn timer_dequeue(&self) { self.state.fetch_and(!STATE_TIMER_QUEUED, Ordering::Relaxed); diff --git a/embassy-executor/src/raw/state_atomics_arm.rs b/embassy-executor/src/raw/state_atomics_arm.rs index f6f2e8f08..7a152e8c0 100644 --- a/embassy-executor/src/raw/state_atomics_arm.rs +++ b/embassy-executor/src/raw/state_atomics_arm.rs @@ -1,13 +1,11 @@ use core::arch::asm; use core::sync::atomic::{compiler_fence, AtomicBool, AtomicU32, Ordering}; -#[cfg(feature = "integrated-timers")] use super::timer_queue::TimerEnqueueOperation; // Must be kept in sync with the layout of `State`! pub(crate) const STATE_SPAWNED: u32 = 1 << 0; pub(crate) const STATE_RUN_QUEUED: u32 = 1 << 8; -#[cfg(feature = "integrated-timers")] pub(crate) const STATE_TIMER_QUEUED: u32 = 1 << 16; #[repr(C, align(4))] @@ -93,7 +91,6 @@ impl State { } /// Mark the task as timer-queued. Return whether it can be enqueued. - #[cfg(feature = "integrated-timers")] #[inline(always)] pub fn timer_enqueue(&self) -> TimerEnqueueOperation { if self @@ -116,7 +113,6 @@ impl State { } /// Unmark the task as timer-queued. - #[cfg(feature = "integrated-timers")] #[inline(always)] pub fn timer_dequeue(&self) { self.timer_queued.store(false, Ordering::Relaxed); diff --git a/embassy-executor/src/raw/state_critical_section.rs b/embassy-executor/src/raw/state_critical_section.rs index c0ec2f530..367162ba2 100644 --- a/embassy-executor/src/raw/state_critical_section.rs +++ b/embassy-executor/src/raw/state_critical_section.rs @@ -2,7 +2,6 @@ use core::cell::Cell; use critical_section::Mutex; -#[cfg(feature = "integrated-timers")] use super::timer_queue::TimerEnqueueOperation; /// Task is spawned (has a future) @@ -10,7 +9,6 @@ pub(crate) const STATE_SPAWNED: u32 = 1 << 0; /// Task is in the executor run queue pub(crate) const STATE_RUN_QUEUED: u32 = 1 << 1; /// Task is in the executor timer queue -#[cfg(feature = "integrated-timers")] pub(crate) const STATE_TIMER_QUEUED: u32 = 1 << 2; pub(crate) struct State { @@ -77,7 +75,6 @@ impl State { } /// Mark the task as timer-queued. Return whether it can be enqueued. - #[cfg(feature = "integrated-timers")] #[inline(always)] pub fn timer_enqueue(&self) -> TimerEnqueueOperation { self.update(|s| { @@ -93,7 +90,6 @@ impl State { } /// Unmark the task as timer-queued. - #[cfg(feature = "integrated-timers")] #[inline(always)] pub fn timer_dequeue(&self) { self.update(|s| *s &= !STATE_TIMER_QUEUED); diff --git a/embassy-executor/src/raw/trace.rs b/embassy-executor/src/raw/trace.rs index c7bcf9c11..b34387b58 100644 --- a/embassy-executor/src/raw/trace.rs +++ b/embassy-executor/src/raw/trace.rs @@ -61,29 +61,23 @@ pub(crate) fn executor_idle(executor: &SyncExecutor) { rtos_trace::trace::system_idle(); } -#[cfg(all(feature = "rtos-trace", feature = "integrated-timers"))] -const fn gcd(a: u64, b: u64) -> u64 { - if b == 0 { - a - } else { - gcd(b, a % b) - } -} - #[cfg(feature = "rtos-trace")] impl rtos_trace::RtosTraceOSCallbacks for crate::raw::SyncExecutor { fn task_list() { // We don't know what tasks exist, so we can't send them. } - #[cfg(feature = "integrated-timers")] fn time() -> u64 { + const fn gcd(a: u64, b: u64) -> u64 { + if b == 0 { + a + } else { + gcd(b, a % b) + } + } + const GCD_1M: u64 = gcd(embassy_time_driver::TICK_HZ, 1_000_000); embassy_time_driver::now() * (1_000_000 / GCD_1M) / (embassy_time_driver::TICK_HZ / GCD_1M) } - #[cfg(not(feature = "integrated-timers"))] - fn time() -> u64 { - 0 - } } #[cfg(feature = "rtos-trace")] -- cgit From 5c4983236c2e68b6ba2ce325ed77ec39466fc3b6 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Fri, 13 Dec 2024 21:45:52 +0100 Subject: Make sure an exited task does not get stuck in a timer queue --- embassy-executor/src/raw/mod.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'embassy-executor/src') diff --git a/embassy-executor/src/raw/mod.rs b/embassy-executor/src/raw/mod.rs index 2feaab155..b825fa6c2 100644 --- a/embassy-executor/src/raw/mod.rs +++ b/embassy-executor/src/raw/mod.rs @@ -192,7 +192,17 @@ impl TaskStorage { match future.poll(&mut cx) { Poll::Ready(_) => { this.future.drop_in_place(); + + // Mark this task to be timer queued, to prevent re-queueing it. + this.raw.state.timer_enqueue(); + + // Now mark the task as not spawned, so that + // - it can be spawned again once it has been removed from the timer queue + // - it can not be timer-queued again this.raw.state.despawn(); + + // Schedule the task by hand in the past, so it runs immediately. + unsafe { _embassy_time_schedule_wake(0, &waker) } } Poll::Pending => {} } @@ -211,6 +221,10 @@ impl TaskStorage { } } +extern "Rust" { + fn _embassy_time_schedule_wake(at: u64, waker: &core::task::Waker); +} + /// An uninitialized [`TaskStorage`]. pub struct AvailableTask { task: &'static TaskStorage, -- cgit From e861344b179b3e955ac47f1985b7f97fdfb93892 Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Sun, 15 Dec 2024 17:44:42 +0100 Subject: Fix comments and tweak task exit --- embassy-executor/src/raw/mod.rs | 21 +++++++++++++++------ embassy-executor/src/raw/timer_queue.rs | 5 +++-- 2 files changed, 18 insertions(+), 8 deletions(-) (limited to 'embassy-executor/src') diff --git a/embassy-executor/src/raw/mod.rs b/embassy-executor/src/raw/mod.rs index b825fa6c2..7da14468d 100644 --- a/embassy-executor/src/raw/mod.rs +++ b/embassy-executor/src/raw/mod.rs @@ -94,13 +94,14 @@ impl TaskRef { &self.header().timer_queue_item } - /// Mark the task as timer-queued. Return whether it was newly queued (i.e. not queued before) + /// Mark the task as timer-queued. Return whether it should be actually enqueued + /// using `_embassy_time_schedule_wake`. /// /// Entering this state prevents the task from being respawned while in a timer queue. /// /// Safety: /// - /// This functions should only be called by the timer queue implementation, before + /// This functions should only be called by the timer queue driver, before /// enqueueing the timer item. pub unsafe fn timer_enqueue(&self) -> timer_queue::TimerEnqueueOperation { self.header().state.timer_enqueue() @@ -193,16 +194,24 @@ impl TaskStorage { Poll::Ready(_) => { this.future.drop_in_place(); - // Mark this task to be timer queued, to prevent re-queueing it. - this.raw.state.timer_enqueue(); + // Mark this task to be timer queued. + // We're splitting the enqueue in two parts, so that we can change task state + // to something that prevent re-queueing. + let op = this.raw.state.timer_enqueue(); // Now mark the task as not spawned, so that // - it can be spawned again once it has been removed from the timer queue // - it can not be timer-queued again + // We must do this before scheduling the wake, to prevent the task from being + // dequeued by the time driver while it's still SPAWNED. this.raw.state.despawn(); - // Schedule the task by hand in the past, so it runs immediately. - unsafe { _embassy_time_schedule_wake(0, &waker) } + // Now let's finish enqueueing. While we shouldn't get an `Ignore` here, it's + // better to be safe. + if op == timer_queue::TimerEnqueueOperation::Enqueue { + // Schedule the task in the past, so it gets dequeued ASAP. + unsafe { _embassy_time_schedule_wake(0, &waker) } + } } Poll::Pending => {} } diff --git a/embassy-executor/src/raw/timer_queue.rs b/embassy-executor/src/raw/timer_queue.rs index c36708401..cd9a73822 100644 --- a/embassy-executor/src/raw/timer_queue.rs +++ b/embassy-executor/src/raw/timer_queue.rs @@ -30,9 +30,10 @@ impl TimerQueueItem { /// The operation to perform after `timer_enqueue` is called. #[derive(Debug, Copy, Clone, PartialEq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[must_use] pub enum TimerEnqueueOperation { - /// Enqueue the task. + /// Enqueue the task (or update its expiration time). Enqueue, - /// Update the task's expiration time. + /// The task must not be enqueued in the timer queue. Ignore, } -- cgit From a10290b28e41922b0f53aafbcc82c49ee3f4e22f Mon Sep 17 00:00:00 2001 From: Dániel Buga Date: Mon, 16 Dec 2024 09:15:15 +0100 Subject: Zero-inizialize expires_at --- embassy-executor/src/raw/timer_queue.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'embassy-executor/src') diff --git a/embassy-executor/src/raw/timer_queue.rs b/embassy-executor/src/raw/timer_queue.rs index cd9a73822..2ba0e00a9 100644 --- a/embassy-executor/src/raw/timer_queue.rs +++ b/embassy-executor/src/raw/timer_queue.rs @@ -22,7 +22,7 @@ impl TimerQueueItem { pub(crate) const fn new() -> Self { Self { next: Cell::new(None), - expires_at: Cell::new(u64::MAX), + expires_at: Cell::new(0), } } } -- cgit