From 54dfe17760b77e199f82ed6a60422c54580104dc Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Sun, 26 Nov 2023 23:40:29 +0000 Subject: [PATCH] Migrate to portable-atomic --- .github/workflows/doctests.yml | 10 +-- .github/workflows/embedded-builds.yml | 2 +- core/Cargo.toml | 7 +- core/src/bbbuffer.rs | 110 +++----------------------- core/src/framed.rs | 1 - core/src/lib.rs | 13 +-- 6 files changed, 16 insertions(+), 127 deletions(-) diff --git a/.github/workflows/doctests.yml b/.github/workflows/doctests.yml index d5f8784..bd553b2 100644 --- a/.github/workflows/doctests.yml +++ b/.github/workflows/doctests.yml @@ -9,18 +9,10 @@ name: Documentation Tests jobs: build: runs-on: ubuntu-latest - strategy: - matrix: - include: - - features: cortex-m - nodefault: "--no-default-features" - - features: "" - nodefault: "" - steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - uses: actions-rs/cargo@v1 with: command: test - args: ${{ matrix.nodefault }} --features=${{ matrix.features }} --manifest-path core/Cargo.toml + args: --manifest-path core/Cargo.toml diff --git a/.github/workflows/embedded-builds.yml b/.github/workflows/embedded-builds.yml index 58b1e28..4459a1d 100644 --- a/.github/workflows/embedded-builds.yml +++ b/.github/workflows/embedded-builds.yml @@ -14,7 +14,7 @@ jobs: include: - features: "" target: thumbv7em-none-eabihf - - feature: cortex-m + - feature: portable-atomic/critical-section target: thumbv6m-none-eabi steps: diff --git a/core/Cargo.toml b/core/Cargo.toml index a49d5ed..af29ea6 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -15,11 +15,8 @@ categories = [ license = "MIT OR Apache-2.0" [dependencies] -cortex-m = { version = "0.6.0", optional = true } - -[dependencies.defmt] -version = "0.3.0" -optional = true +portable-atomic = { version = "1.5.1", default-features = false, features = ["require-cas"] } +defmt = { version = "0.3.0", optional = true } [package.metadata.docs.rs] all-features = true diff --git a/core/src/bbbuffer.rs b/core/src/bbbuffer.rs index 1f9a03b..890983c 100644 --- a/core/src/bbbuffer.rs +++ b/core/src/bbbuffer.rs @@ -11,11 +11,10 @@ use core::{ ptr::NonNull, result::Result as CoreResult, slice::from_raw_parts_mut, - sync::atomic::{ - AtomicBool, AtomicUsize, - Ordering::{AcqRel, Acquire, Release}, - }, + sync::atomic::Ordering::{AcqRel, Acquire, Release}, }; +use portable_atomic::{AtomicBool, AtomicUsize}; + #[derive(Debug)] /// A backing structure for a BBQueue. Can be used to create either /// a BBQueue or a split Producer/Consumer pair @@ -63,9 +62,6 @@ impl<'a, const N: usize> BBBuffer { /// is placed at `static` scope within the `.bss` region, the explicit initialization /// will be elided (as it is already performed as part of memory initialization) /// - /// NOTE: If the `cortex-m` feature is selected, this function takes a short critical section - /// while splitting. - /// /// ```rust /// # // bbqueue test shim! /// # fn bbqtest() { @@ -81,12 +77,11 @@ impl<'a, const N: usize> BBBuffer { /// # } /// # /// # fn main() { - /// # #[cfg(not(feature = "cortex-m"))] /// # bbqtest(); /// # } /// ``` pub fn try_split(&'a self) -> Result<(Producer<'a, N>, Consumer<'a, N>)> { - if atomic::swap(&self.already_split, true, AcqRel) { + if self.already_split.swap(true, AcqRel) { return Err(Error::AlreadySplit); } @@ -122,9 +117,6 @@ impl<'a, const N: usize> BBBuffer { /// of the buffer. This is necessary to prevent undefined behavior. If the buffer /// is placed at `static` scope within the `.bss` region, the explicit initialization /// will be elided (as it is already performed as part of memory initialization) - /// - /// NOTE: If the `cortex-m` feature is selected, this function takes a short critical - /// section while splitting. pub fn try_split_framed(&'a self) -> Result<(FrameProducer<'a, N>, FrameConsumer<'a, N>)> { let (producer, consumer) = self.try_split()?; Ok((FrameProducer { producer }, FrameConsumer { consumer })) @@ -159,7 +151,6 @@ impl<'a, const N: usize> BBBuffer { /// # } /// # /// # fn main() { - /// # #[cfg(not(feature = "cortex-m"))] /// # bbqtest(); /// # } /// ``` @@ -345,14 +336,13 @@ impl<'a, const N: usize> Producer<'a, N> { /// # } /// # /// # fn main() { - /// # #[cfg(not(feature = "cortex-m"))] /// # bbqtest(); /// # } /// ``` pub fn grant_exact(&mut self, sz: usize) -> Result> { let inner = unsafe { &self.bbq.as_ref() }; - if atomic::swap(&inner.write_in_progress, true, AcqRel) { + if inner.write_in_progress.swap(true, AcqRel) { return Err(Error::GrantInProgress); } @@ -443,14 +433,13 @@ impl<'a, const N: usize> Producer<'a, N> { /// # } /// # /// # fn main() { - /// # #[cfg(not(feature = "cortex-m"))] /// # bbqtest(); /// # } /// ``` pub fn grant_max_remaining(&mut self, mut sz: usize) -> Result> { let inner = unsafe { &self.bbq.as_ref() }; - if atomic::swap(&inner.write_in_progress, true, AcqRel) { + if inner.write_in_progress.swap(true, AcqRel) { return Err(Error::GrantInProgress); } @@ -548,14 +537,13 @@ impl<'a, const N: usize> Consumer<'a, N> { /// # } /// # /// # fn main() { - /// # #[cfg(not(feature = "cortex-m"))] /// # bbqtest(); /// # } /// ``` pub fn read(&mut self) -> Result> { let inner = unsafe { &self.bbq.as_ref() }; - if atomic::swap(&inner.read_in_progress, true, AcqRel) { + if inner.read_in_progress.swap(true, AcqRel) { return Err(Error::GrantInProgress); } @@ -607,7 +595,7 @@ impl<'a, const N: usize> Consumer<'a, N> { pub fn split_read(&mut self) -> Result> { let inner = unsafe { &self.bbq.as_ref() }; - if atomic::swap(&inner.read_in_progress, true, AcqRel) { + if inner.read_in_progress.swap(true, AcqRel) { return Err(Error::GrantInProgress); } @@ -674,7 +662,6 @@ impl BBBuffer { /// # } /// # /// # fn main() { - /// # #[cfg(not(feature = "cortex-m"))] /// # bbqtest(); /// # } /// ``` @@ -690,9 +677,6 @@ impl BBBuffer { /// the contents, or by setting a the number of bytes to /// automatically be committed with `to_commit()`, then no bytes /// will be comitted for writing. -/// -/// If the `cortex-m` feature is selected, dropping the grant -/// without committing it takes a short critical section, #[derive(Debug, PartialEq)] pub struct GrantW<'a, const N: usize> { pub(crate) buf: &'a mut [u8], @@ -710,10 +694,6 @@ unsafe impl<'a, const N: usize> Send for GrantW<'a, N> {} /// the contents, or by setting the number of bytes to automatically /// be released with `to_release()`, then no bytes will be released /// as read. -/// -/// -/// If the `cortex-m` feature is selected, dropping the grant -/// without releasing it takes a short critical section, #[derive(Debug, PartialEq)] pub struct GrantR<'a, const N: usize> { pub(crate) buf: &'a mut [u8], @@ -743,9 +723,6 @@ impl<'a, const N: usize> GrantW<'a, N> { /// /// If `used` is larger than the given grant, the maximum amount will /// be commited - /// - /// NOTE: If the `cortex-m` feature is selected, this function takes a short critical - /// section while committing. pub fn commit(mut self, used: usize) { self.commit_inner(used); forget(self); @@ -770,7 +747,6 @@ impl<'a, const N: usize> GrantW<'a, N> { /// # } /// # /// # fn main() { - /// # #[cfg(not(feature = "cortex-m"))] /// # bbqtest(); /// # } /// ``` @@ -814,7 +790,7 @@ impl<'a, const N: usize> GrantW<'a, N> { let used = min(len, used); let write = inner.write.load(Acquire); - atomic::fetch_sub(&inner.reserve, len - used, AcqRel); + inner.reserve.fetch_sub(len - used, AcqRel); let max = N; let last = inner.last.load(Acquire); @@ -860,9 +836,6 @@ impl<'a, const N: usize> GrantR<'a, N> { /// /// If `used` is larger than the given grant, the full grant will /// be released. - /// - /// NOTE: If the `cortex-m` feature is selected, this function takes a short critical - /// section while releasing. pub fn release(mut self, used: usize) { // Saturate the grant release let used = min(self.buf.len(), used); @@ -903,7 +876,6 @@ impl<'a, const N: usize> GrantR<'a, N> { /// # } /// # /// # fn main() { - /// # #[cfg(not(feature = "cortex-m"))] /// # bbqtest(); /// # } /// ``` @@ -951,7 +923,7 @@ impl<'a, const N: usize> GrantR<'a, N> { debug_assert!(used <= self.buf.len()); // This should be fine, purely incrementing - let _ = atomic::fetch_add(&inner.read, used, Release); + let _ = inner.read.fetch_add(used, Release); inner.read_in_progress.store(false, Release); } @@ -968,9 +940,6 @@ impl<'a, const N: usize> SplitGrantR<'a, N> { /// /// If `used` is larger than the given grant, the full grant will /// be released. - /// - /// NOTE: If the `cortex-m` feature is selected, this function takes a short critical - /// section while releasing. pub fn release(mut self, used: usize) { // Saturate the grant release let used = min(self.combined_len(), used); @@ -1004,7 +973,6 @@ impl<'a, const N: usize> SplitGrantR<'a, N> { /// # } /// # /// # fn main() { - /// # #[cfg(not(feature = "cortex-m"))] /// # bbqtest(); /// # } /// ``` @@ -1036,7 +1004,7 @@ impl<'a, const N: usize> SplitGrantR<'a, N> { if used <= self.buf1.len() { // This should be fine, purely incrementing - let _ = atomic::fetch_add(&inner.read, used, Release); + let _ = inner.read.fetch_add(used, Release); } else { // Also release parts of the second buffer inner.read.store(used - self.buf1.len(), Release); @@ -1101,59 +1069,3 @@ impl<'a, const N: usize> DerefMut for GrantR<'a, N> { self.buf } } - -#[cfg(feature = "cortex-m")] -mod atomic { - use core::sync::atomic::{ - AtomicBool, AtomicUsize, - Ordering::{self, Acquire, Release}, - }; - use cortex_m::interrupt::free; - - #[inline(always)] - pub fn fetch_add(atomic: &AtomicUsize, val: usize, _order: Ordering) -> usize { - free(|_| { - let prev = atomic.load(Acquire); - atomic.store(prev.wrapping_add(val), Release); - prev - }) - } - - #[inline(always)] - pub fn fetch_sub(atomic: &AtomicUsize, val: usize, _order: Ordering) -> usize { - free(|_| { - let prev = atomic.load(Acquire); - atomic.store(prev.wrapping_sub(val), Release); - prev - }) - } - - #[inline(always)] - pub fn swap(atomic: &AtomicBool, val: bool, _order: Ordering) -> bool { - free(|_| { - let prev = atomic.load(Acquire); - atomic.store(val, Release); - prev - }) - } -} - -#[cfg(not(feature = "cortex-m"))] -mod atomic { - use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; - - #[inline(always)] - pub fn fetch_add(atomic: &AtomicUsize, val: usize, order: Ordering) -> usize { - atomic.fetch_add(val, order) - } - - #[inline(always)] - pub fn fetch_sub(atomic: &AtomicUsize, val: usize, order: Ordering) -> usize { - atomic.fetch_sub(val, order) - } - - #[inline(always)] - pub fn swap(atomic: &AtomicBool, val: bool, order: Ordering) -> bool { - atomic.swap(val, order) - } -} diff --git a/core/src/framed.rs b/core/src/framed.rs index a14b696..9093b1f 100644 --- a/core/src/framed.rs +++ b/core/src/framed.rs @@ -34,7 +34,6 @@ //! # } //! # //! # fn main() { -//! # #[cfg(not(feature = "cortex-m"))] //! # bbqtest(); //! # } //! ``` diff --git a/core/src/lib.rs b/core/src/lib.rs index 8bf15fb..33536ef 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -83,17 +83,6 @@ //! assert!(BB.try_split().is_err()); //! } //! ``` -//! -//! ## Features -//! -//! By default BBQueue uses atomic operations which are available on most platforms. However on some -//! (mostly embedded) platforms atomic support is limited and with the default features you will get -//! a compiler error about missing atomic methods. -//! -//! This crate contains special support for Cortex-M0(+) targets with the `cortex-m` feature. By -//! enabling the feature, unsupported atomic operations will be replaced with critical sections -//! implemented by disabling interrupts. The critical sections are very short, a few instructions at -//! most, so they should make no difference to most applications. #![cfg_attr(not(feature = "std"), no_std)] #![deny(missing_docs)] @@ -112,7 +101,7 @@ pub type Result = CoreResult; /// Error type used by the `BBQueue` interfaces #[derive(Debug, PartialEq, Eq, Copy, Clone)] -#[cfg_attr(feature = "defmt_0_3", derive(defmt::Format))] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum Error { /// The buffer does not contain sufficient size for the requested action InsufficientSize,