From 71b5f9438e1beb5fe12b90415d9d6307e79c0cdf Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Mon, 23 Jan 2023 20:57:56 +0100 Subject: Fixed systick monotonic --- rtic-time/src/lib.rs | 336 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 336 insertions(+) create mode 100644 rtic-time/src/lib.rs (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs new file mode 100644 index 0000000..d7faa07 --- /dev/null +++ b/rtic-time/src/lib.rs @@ -0,0 +1,336 @@ +//! Crate + +#![no_std] +#![no_main] +#![deny(missing_docs)] +#![allow(incomplete_features)] +#![feature(async_fn_in_trait)] + +pub mod monotonic; + +use core::future::{poll_fn, Future}; +use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use core::task::{Poll, Waker}; +use futures_util::{ + future::{select, Either}, + pin_mut, +}; +pub use monotonic::Monotonic; + +mod linked_list; + +use linked_list::{Link, LinkedList}; + +/// Holds a waker and at which time instant this waker shall be awoken. +struct WaitingWaker { + waker: Waker, + release_at: Mono::Instant, +} + +impl Clone for WaitingWaker { + fn clone(&self) -> Self { + Self { + waker: self.waker.clone(), + release_at: self.release_at, + } + } +} + +impl PartialEq for WaitingWaker { + fn eq(&self, other: &Self) -> bool { + self.release_at == other.release_at + } +} + +impl PartialOrd for WaitingWaker { + fn partial_cmp(&self, other: &Self) -> Option { + self.release_at.partial_cmp(&other.release_at) + } +} + +/// A generic timer queue for async executors. +/// +/// # Blocking +/// +/// The internal priority queue uses global critical sections to manage access. This means that +/// `await`ing a delay will cause a lock of the entire system for O(n) time. In practice the lock +/// duration is ~10 clock cycles per element in the queue. +/// +/// # Safety +/// +/// This timer queue is based on an intrusive linked list, and by extension the links are strored +/// on the async stacks of callers. The links are deallocated on `drop` or when the wait is +/// complete. +/// +/// Do not call `mem::forget` on an awaited future, or there will be dragons! +pub struct TimerQueue { + queue: LinkedList>, + initialized: AtomicBool, +} + +/// This indicates that there was a timeout. +pub struct TimeoutError; + +impl TimerQueue { + /// Make a new queue. + pub const fn new() -> Self { + Self { + queue: LinkedList::new(), + initialized: AtomicBool::new(false), + } + } + + /// Forwards the `Monotonic::now()` method. + #[inline(always)] + pub fn now(&self) -> Mono::Instant { + Mono::now() + } + + /// Takes the initialized monotonic to initialize the TimerQueue. + pub fn initialize(&self, monotonic: Mono) { + self.initialized.store(true, Ordering::SeqCst); + + // Don't run drop on `Mono` + core::mem::forget(monotonic); + } + + /// Call this in the interrupt handler of the hardware timer supporting the `Monotonic` + /// + /// # Safety + /// + /// It's always safe to call, but it must only be called from the interrupt of the + /// monotonic timer for correct operation. + pub unsafe fn on_monotonic_interrupt(&self) { + Mono::clear_compare_flag(); + Mono::on_interrupt(); + + loop { + let mut release_at = None; + let head = self.queue.pop_if(|head| { + release_at = Some(head.release_at); + + Mono::now() >= head.release_at + }); + + match (head, release_at) { + (Some(link), _) => { + link.waker.wake(); + } + (None, Some(instant)) => { + Mono::enable_timer(); + Mono::set_compare(instant); + + if Mono::now() >= instant { + // The time for the next instant passed while handling it, + // continue dequeueing + continue; + } + + break; + } + (None, None) => { + // Queue is empty + Mono::disable_timer(); + + break; + } + } + } + } + + /// Timeout at a specific time. + pub async fn timeout_at( + &self, + instant: Mono::Instant, + future: F, + ) -> Result { + let delay = self.delay_until(instant); + + pin_mut!(future); + pin_mut!(delay); + + match select(future, delay).await { + Either::Left((r, _)) => Ok(r), + Either::Right(_) => Err(TimeoutError), + } + } + + /// Timeout after a specific duration. + #[inline] + pub async fn timeout_after( + &self, + duration: Mono::Duration, + future: F, + ) -> Result { + self.timeout_at(Mono::now() + duration, future).await + } + + /// Delay for some duration of time. + #[inline] + pub async fn delay(&self, duration: Mono::Duration) { + let now = Mono::now(); + + self.delay_until(now + duration).await; + } + + /// Delay to some specific time instant. + pub async fn delay_until(&self, instant: Mono::Instant) { + if !self.initialized.load(Ordering::Relaxed) { + panic!( + "The timer queue is not initialized with a monotonic, you need to run `initialize`" + ); + } + + let mut first_run = true; + let queue = &self.queue; + let mut link = Link::new(WaitingWaker { + waker: poll_fn(|cx| Poll::Ready(cx.waker().clone())).await, + release_at: instant, + }); + + let marker = &AtomicUsize::new(0); + + let dropper = OnDrop::new(|| { + queue.delete(marker.load(Ordering::Relaxed)); + }); + + poll_fn(|_| { + if Mono::now() >= instant { + return Poll::Ready(()); + } + + if first_run { + first_run = false; + let (was_empty, addr) = queue.insert(&mut link); + marker.store(addr, Ordering::Relaxed); + + if was_empty { + // Pend the monotonic handler if the queue was empty to setup the timer. + Mono::pend_interrupt(); + } + } + + Poll::Pending + }) + .await; + + // Make sure that our link is deleted from the list before we drop this stack + drop(dropper); + } +} + +struct OnDrop { + f: core::mem::MaybeUninit, +} + +impl OnDrop { + pub fn new(f: F) -> Self { + Self { + f: core::mem::MaybeUninit::new(f), + } + } + + #[allow(unused)] + pub fn defuse(self) { + core::mem::forget(self) + } +} + +impl Drop for OnDrop { + fn drop(&mut self) { + unsafe { self.f.as_ptr().read()() } + } +} + +// -------- Test program --------- +// +// +// use systick_monotonic::{Systick, TimerQueue}; +// +// // same panicking *behavior* as `panic-probe` but doesn't print a panic message +// // this prevents the panic message being printed *twice* when `defmt::panic` is invoked +// #[defmt::panic_handler] +// fn panic() -> ! { +// cortex_m::asm::udf() +// } +// +// /// Terminates the application and makes `probe-run` exit with exit-code = 0 +// pub fn exit() -> ! { +// loop { +// cortex_m::asm::bkpt(); +// } +// } +// +// defmt::timestamp!("{=u64:us}", { +// let time_us: fugit::MicrosDurationU32 = MONO.now().duration_since_epoch().convert(); +// +// time_us.ticks() as u64 +// }); +// +// make_systick_timer_queue!(MONO, Systick<1_000>); +// +// #[rtic::app( +// device = nrf52832_hal::pac, +// dispatchers = [SWI0_EGU0, SWI1_EGU1, SWI2_EGU2, SWI3_EGU3, SWI4_EGU4, SWI5_EGU5], +// )] +// mod app { +// use super::{Systick, MONO}; +// use fugit::ExtU32; +// +// #[shared] +// struct Shared {} +// +// #[local] +// struct Local {} +// +// #[init] +// fn init(cx: init::Context) -> (Shared, Local) { +// defmt::println!("init"); +// +// let systick = Systick::start(cx.core.SYST, 64_000_000); +// +// defmt::println!("initializing monotonic"); +// +// MONO.initialize(systick); +// +// async_task::spawn().ok(); +// async_task2::spawn().ok(); +// async_task3::spawn().ok(); +// +// (Shared {}, Local {}) +// } +// +// #[idle] +// fn idle(_: idle::Context) -> ! { +// defmt::println!("idle"); +// +// loop { +// core::hint::spin_loop(); +// } +// } +// +// #[task] +// async fn async_task(_: async_task::Context) { +// loop { +// defmt::println!("async task waiting for 1 second"); +// MONO.delay(1.secs()).await; +// } +// } +// +// #[task] +// async fn async_task2(_: async_task2::Context) { +// loop { +// defmt::println!(" async task 2 waiting for 0.5 second"); +// MONO.delay(500.millis()).await; +// } +// } +// +// #[task] +// async fn async_task3(_: async_task3::Context) { +// loop { +// defmt::println!(" async task 3 waiting for 0.2 second"); +// MONO.delay(200.millis()).await; +// } +// } +// } +// -- cgit v1.2.3 From 143cd136eeeb2856d06a1b83e3ef5682f720c251 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Tue, 24 Jan 2023 11:55:48 +0100 Subject: Optimize linked list popping so delete is not run everytime --- rtic-time/src/lib.rs | 131 ++++++++++----------------------------------------- 1 file changed, 26 insertions(+), 105 deletions(-) (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index d7faa07..850885b 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -6,9 +6,8 @@ #![allow(incomplete_features)] #![feature(async_fn_in_trait)] -pub mod monotonic; - use core::future::{poll_fn, Future}; +use core::mem::MaybeUninit; use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use core::task::{Poll, Waker}; use futures_util::{ @@ -18,20 +17,25 @@ use futures_util::{ pub use monotonic::Monotonic; mod linked_list; +mod monotonic; use linked_list::{Link, LinkedList}; /// Holds a waker and at which time instant this waker shall be awoken. struct WaitingWaker { - waker: Waker, + // This is alway initialized when used, we create this struct on the async stack and then + // initialize the waker field in the `poll_fn` closure (we then know the waker) + waker: MaybeUninit, release_at: Mono::Instant, + was_poped: AtomicBool, } impl Clone for WaitingWaker { fn clone(&self) -> Self { Self { - waker: self.waker.clone(), + waker: MaybeUninit::new(unsafe { self.waker.assume_init_ref() }.clone()), release_at: self.release_at, + was_poped: AtomicBool::new(self.was_poped.load(Ordering::Relaxed)), } } } @@ -109,12 +113,15 @@ impl TimerQueue { let head = self.queue.pop_if(|head| { release_at = Some(head.release_at); - Mono::now() >= head.release_at + let should_pop = Mono::now() >= head.release_at; + head.was_poped.store(should_pop, Ordering::Relaxed); + + should_pop }); match (head, release_at) { (Some(link), _) => { - link.waker.wake(); + link.waker.assume_init().wake(); } (None, Some(instant)) => { Mono::enable_timer(); @@ -181,26 +188,28 @@ impl TimerQueue { ); } - let mut first_run = true; - let queue = &self.queue; let mut link = Link::new(WaitingWaker { - waker: poll_fn(|cx| Poll::Ready(cx.waker().clone())).await, + waker: MaybeUninit::uninit(), release_at: instant, + was_poped: AtomicBool::new(false), }); + let mut first_run = true; + let queue = &self.queue; let marker = &AtomicUsize::new(0); let dropper = OnDrop::new(|| { queue.delete(marker.load(Ordering::Relaxed)); }); - poll_fn(|_| { + poll_fn(|cx| { if Mono::now() >= instant { return Poll::Ready(()); } if first_run { first_run = false; + link.val.waker.write(cx.waker().clone()); let (was_empty, addr) = queue.insert(&mut link); marker.store(addr, Ordering::Relaxed); @@ -214,8 +223,13 @@ impl TimerQueue { }) .await; - // Make sure that our link is deleted from the list before we drop this stack - drop(dropper); + if link.val.was_poped.load(Ordering::Relaxed) { + // If it was poped from the queue there is no need to run delete + dropper.defuse(); + } else { + // Make sure that our link is deleted from the list before we drop this stack + drop(dropper); + } } } @@ -241,96 +255,3 @@ impl Drop for OnDrop { unsafe { self.f.as_ptr().read()() } } } - -// -------- Test program --------- -// -// -// use systick_monotonic::{Systick, TimerQueue}; -// -// // same panicking *behavior* as `panic-probe` but doesn't print a panic message -// // this prevents the panic message being printed *twice* when `defmt::panic` is invoked -// #[defmt::panic_handler] -// fn panic() -> ! { -// cortex_m::asm::udf() -// } -// -// /// Terminates the application and makes `probe-run` exit with exit-code = 0 -// pub fn exit() -> ! { -// loop { -// cortex_m::asm::bkpt(); -// } -// } -// -// defmt::timestamp!("{=u64:us}", { -// let time_us: fugit::MicrosDurationU32 = MONO.now().duration_since_epoch().convert(); -// -// time_us.ticks() as u64 -// }); -// -// make_systick_timer_queue!(MONO, Systick<1_000>); -// -// #[rtic::app( -// device = nrf52832_hal::pac, -// dispatchers = [SWI0_EGU0, SWI1_EGU1, SWI2_EGU2, SWI3_EGU3, SWI4_EGU4, SWI5_EGU5], -// )] -// mod app { -// use super::{Systick, MONO}; -// use fugit::ExtU32; -// -// #[shared] -// struct Shared {} -// -// #[local] -// struct Local {} -// -// #[init] -// fn init(cx: init::Context) -> (Shared, Local) { -// defmt::println!("init"); -// -// let systick = Systick::start(cx.core.SYST, 64_000_000); -// -// defmt::println!("initializing monotonic"); -// -// MONO.initialize(systick); -// -// async_task::spawn().ok(); -// async_task2::spawn().ok(); -// async_task3::spawn().ok(); -// -// (Shared {}, Local {}) -// } -// -// #[idle] -// fn idle(_: idle::Context) -> ! { -// defmt::println!("idle"); -// -// loop { -// core::hint::spin_loop(); -// } -// } -// -// #[task] -// async fn async_task(_: async_task::Context) { -// loop { -// defmt::println!("async task waiting for 1 second"); -// MONO.delay(1.secs()).await; -// } -// } -// -// #[task] -// async fn async_task2(_: async_task2::Context) { -// loop { -// defmt::println!(" async task 2 waiting for 0.5 second"); -// MONO.delay(500.millis()).await; -// } -// } -// -// #[task] -// async fn async_task3(_: async_task3::Context) { -// loop { -// defmt::println!(" async task 3 waiting for 0.2 second"); -// MONO.delay(200.millis()).await; -// } -// } -// } -// -- cgit v1.2.3 From 2e96229c912076829d4c2be83a8b5ce27d81ebaa Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Wed, 25 Jan 2023 15:24:32 +0100 Subject: Remove unnecessary MaybeUninit --- rtic-time/src/lib.rs | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index 850885b..34f9362 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -7,7 +7,6 @@ #![feature(async_fn_in_trait)] use core::future::{poll_fn, Future}; -use core::mem::MaybeUninit; use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use core::task::{Poll, Waker}; use futures_util::{ @@ -23,9 +22,7 @@ use linked_list::{Link, LinkedList}; /// Holds a waker and at which time instant this waker shall be awoken. struct WaitingWaker { - // This is alway initialized when used, we create this struct on the async stack and then - // initialize the waker field in the `poll_fn` closure (we then know the waker) - waker: MaybeUninit, + waker: Waker, release_at: Mono::Instant, was_poped: AtomicBool, } @@ -33,7 +30,7 @@ struct WaitingWaker { impl Clone for WaitingWaker { fn clone(&self) -> Self { Self { - waker: MaybeUninit::new(unsafe { self.waker.assume_init_ref() }.clone()), + waker: self.waker.clone(), release_at: self.release_at, was_poped: AtomicBool::new(self.was_poped.load(Ordering::Relaxed)), } @@ -121,7 +118,7 @@ impl TimerQueue { match (head, release_at) { (Some(link), _) => { - link.waker.assume_init().wake(); + link.waker.wake(); } (None, Some(instant)) => { Mono::enable_timer(); @@ -188,13 +185,8 @@ impl TimerQueue { ); } - let mut link = Link::new(WaitingWaker { - waker: MaybeUninit::uninit(), - release_at: instant, - was_poped: AtomicBool::new(false), - }); + let mut link = None; - let mut first_run = true; let queue = &self.queue; let marker = &AtomicUsize::new(0); @@ -207,10 +199,14 @@ impl TimerQueue { return Poll::Ready(()); } - if first_run { - first_run = false; - link.val.waker.write(cx.waker().clone()); - let (was_empty, addr) = queue.insert(&mut link); + if link.is_none() { + let mut link_ref = link.insert(Link::new(WaitingWaker { + waker: cx.waker().clone(), + release_at: instant, + was_poped: AtomicBool::new(false), + })); + + let (was_empty, addr) = queue.insert(&mut link_ref); marker.store(addr, Ordering::Relaxed); if was_empty { @@ -223,9 +219,11 @@ impl TimerQueue { }) .await; - if link.val.was_poped.load(Ordering::Relaxed) { - // If it was poped from the queue there is no need to run delete - dropper.defuse(); + if let Some(link) = link { + if link.val.was_poped.load(Ordering::Relaxed) { + // If it was poped from the queue there is no need to run delete + dropper.defuse(); + } } else { // Make sure that our link is deleted from the list before we drop this stack drop(dropper); -- cgit v1.2.3 From 51d4eccc726d4dc7950aac6f8d74a34ddf669f67 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Thu, 26 Jan 2023 21:29:52 +0100 Subject: Fixes in MPSC linked list and dropper handling --- rtic-time/src/lib.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index 34f9362..78ece1d 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -1,7 +1,6 @@ //! Crate #![no_std] -#![no_main] #![deny(missing_docs)] #![allow(incomplete_features)] #![feature(async_fn_in_trait)] -- cgit v1.2.3 From 1baa4a4228cae4576e194174618bf35f5c206959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Tj=C3=A4der?= Date: Fri, 27 Jan 2023 13:18:29 +0100 Subject: CI: Don't let warnings get away --- rtic-time/src/lib.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index 78ece1d..eeecd86 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -2,6 +2,7 @@ #![no_std] #![deny(missing_docs)] +//deny_warnings_placeholder_for_ci #![allow(incomplete_features)] #![feature(async_fn_in_trait)] -- cgit v1.2.3 From 3050fc0591f087a4fbe08840c69633e89d3f58a7 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Sat, 28 Jan 2023 13:21:44 +0100 Subject: Use `Pin` in the linked lists --- rtic-time/src/lib.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index eeecd86..6b23f76 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -7,6 +7,7 @@ #![feature(async_fn_in_trait)] use core::future::{poll_fn, Future}; +use core::pin::Pin; use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use core::task::{Poll, Waker}; use futures_util::{ @@ -185,7 +186,10 @@ impl TimerQueue { ); } - let mut link = None; + let mut link_ptr: Option>> = None; + + // Make this future `Drop`-safe, also shadow the original definition so we can't abuse it. + let link_ptr = &mut link_ptr as *mut Option>>; let queue = &self.queue; let marker = &AtomicUsize::new(0); @@ -199,6 +203,9 @@ impl TimerQueue { return Poll::Ready(()); } + // SAFETY: This pointer is only dereferenced here and on drop of the future + // which happens outside this `poll_fn`'s stack frame. + let link = unsafe { &mut *link_ptr }; if link.is_none() { let mut link_ref = link.insert(Link::new(WaitingWaker { waker: cx.waker().clone(), @@ -206,7 +213,9 @@ impl TimerQueue { was_poped: AtomicBool::new(false), })); - let (was_empty, addr) = queue.insert(&mut link_ref); + // SAFETY: The address to the link is stable as it is defined outside this stack + // frame. + let (was_empty, addr) = queue.insert(unsafe { Pin::new_unchecked(&mut link_ref) }); marker.store(addr, Ordering::Relaxed); if was_empty { @@ -219,7 +228,10 @@ impl TimerQueue { }) .await; - if let Some(link) = link { + // SAFETY: We only run this and dereference the pointer if we have + // exited the `poll_fn` below in the `drop(dropper)` call. The other dereference + // of this pointer is in the `poll_fn`. + if let Some(link) = unsafe { &mut *link_ptr } { if link.val.was_poped.load(Ordering::Relaxed) { // If it was poped from the queue there is no need to run delete dropper.defuse(); -- cgit v1.2.3 From 2bd70baeb9362050196d431f2801551066e27e59 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Sat, 28 Jan 2023 21:11:18 +0100 Subject: rtic-time: Make Send happy --- rtic-time/src/lib.rs | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index 6b23f76..44fdbce 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -73,6 +73,26 @@ pub struct TimerQueue { /// This indicates that there was a timeout. pub struct TimeoutError; +/// This is needed to make the async closure in `delay_until` accept that we "share" +/// the link possible between threads. +struct LinkPtr(*mut Option>>); + +impl Clone for LinkPtr { + fn clone(&self) -> Self { + LinkPtr(self.0) + } +} + +impl LinkPtr { + /// This will dereference the pointer stored within and give out an `&mut`. + unsafe fn get(&mut self) -> &mut Option>> { + &mut *self.0 + } +} + +unsafe impl Send for LinkPtr {} +unsafe impl Sync for LinkPtr {} + impl TimerQueue { /// Make a new queue. pub const fn new() -> Self { @@ -189,7 +209,9 @@ impl TimerQueue { let mut link_ptr: Option>> = None; // Make this future `Drop`-safe, also shadow the original definition so we can't abuse it. - let link_ptr = &mut link_ptr as *mut Option>>; + let mut link_ptr = + LinkPtr(&mut link_ptr as *mut Option>>); + let mut link_ptr2 = link_ptr.clone(); let queue = &self.queue; let marker = &AtomicUsize::new(0); @@ -205,7 +227,7 @@ impl TimerQueue { // SAFETY: This pointer is only dereferenced here and on drop of the future // which happens outside this `poll_fn`'s stack frame. - let link = unsafe { &mut *link_ptr }; + let link = unsafe { link_ptr2.get() }; if link.is_none() { let mut link_ref = link.insert(Link::new(WaitingWaker { waker: cx.waker().clone(), @@ -231,7 +253,7 @@ impl TimerQueue { // SAFETY: We only run this and dereference the pointer if we have // exited the `poll_fn` below in the `drop(dropper)` call. The other dereference // of this pointer is in the `poll_fn`. - if let Some(link) = unsafe { &mut *link_ptr } { + if let Some(link) = unsafe { link_ptr.get() } { if link.val.was_poped.load(Ordering::Relaxed) { // If it was poped from the queue there is no need to run delete dropper.defuse(); -- cgit v1.2.3 From d0c51269608c18a105fd010f070bd9af6f443c60 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Tue, 31 Jan 2023 22:05:43 +0100 Subject: Cleanup common code and clippy fixes --- rtic-time/src/lib.rs | 27 ++------------------------- 1 file changed, 2 insertions(+), 25 deletions(-) (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index 44fdbce..5e4457c 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -14,13 +14,13 @@ use futures_util::{ future::{select, Either}, pin_mut, }; +use linked_list::{Link, LinkedList}; pub use monotonic::Monotonic; +use rtic_common::dropper::OnDrop; mod linked_list; mod monotonic; -use linked_list::{Link, LinkedList}; - /// Holds a waker and at which time instant this waker shall be awoken. struct WaitingWaker { waker: Waker, @@ -264,26 +264,3 @@ impl TimerQueue { } } } - -struct OnDrop { - f: core::mem::MaybeUninit, -} - -impl OnDrop { - pub fn new(f: F) -> Self { - Self { - f: core::mem::MaybeUninit::new(f), - } - } - - #[allow(unused)] - pub fn defuse(self) { - core::mem::forget(self) - } -} - -impl Drop for OnDrop { - fn drop(&mut self) { - unsafe { self.f.as_ptr().read()() } - } -} -- cgit v1.2.3 From 1cda61fbda205920517f7b63af90c97c38ff9af6 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Sat, 18 Feb 2023 09:43:06 +0100 Subject: Make some linked list operations unsafe, and document their safety at usage --- rtic-time/src/lib.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index 5e4457c..3126e6b 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -208,7 +208,8 @@ impl TimerQueue { let mut link_ptr: Option>> = None; - // Make this future `Drop`-safe, also shadow the original definition so we can't abuse it. + // Make this future `Drop`-safe + // SAFETY(link_ptr): Shadow the original definition of `link_ptr` so we can't abuse it. let mut link_ptr = LinkPtr(&mut link_ptr as *mut Option>>); let mut link_ptr2 = link_ptr.clone(); @@ -226,18 +227,24 @@ impl TimerQueue { } // SAFETY: This pointer is only dereferenced here and on drop of the future - // which happens outside this `poll_fn`'s stack frame. + // which happens outside this `poll_fn`'s stack frame, so this mutable access cannot + // happen at the same time as `dropper` runs. let link = unsafe { link_ptr2.get() }; if link.is_none() { - let mut link_ref = link.insert(Link::new(WaitingWaker { + let link_ref = link.insert(Link::new(WaitingWaker { waker: cx.waker().clone(), release_at: instant, was_poped: AtomicBool::new(false), })); - // SAFETY: The address to the link is stable as it is defined outside this stack - // frame. - let (was_empty, addr) = queue.insert(unsafe { Pin::new_unchecked(&mut link_ref) }); + // SAFETY(new_unchecked): The address to the link is stable as it is defined + //outside this stack frame. + // SAFETY(insert): `link_ref` lifetime comes from `link_ptr` that is shadowed, and + // we make sure in `dropper` that the link is removed from the queue before + // dropping `link_ptr` AND `dropper` makes sure that the shadowed `link_ptr` lives + // until the end of the stack frame. + let (was_empty, addr) = unsafe { queue.insert(Pin::new_unchecked(&link_ref)) }; + marker.store(addr, Ordering::Relaxed); if was_empty { -- cgit v1.2.3 From ebd35b89a4abe147e11bd7f716788cf642368b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Tj=C3=A4der?= Date: Sat, 4 Mar 2023 20:52:50 +0100 Subject: rtic-time: clippy fixes --- rtic-time/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rtic-time/src/lib.rs') diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index 3126e6b..78b57a4 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -243,7 +243,7 @@ impl TimerQueue { // we make sure in `dropper` that the link is removed from the queue before // dropping `link_ptr` AND `dropper` makes sure that the shadowed `link_ptr` lives // until the end of the stack frame. - let (was_empty, addr) = unsafe { queue.insert(Pin::new_unchecked(&link_ref)) }; + let (was_empty, addr) = unsafe { queue.insert(Pin::new_unchecked(link_ref)) }; marker.store(addr, Ordering::Relaxed); -- cgit v1.2.3