Re: [PATCH v2 1/2] rust: sync: atomic: Add perfromance-optimal Flag type for atomic booleans

["Gary Guo" <gary@garyguo.net>]

On Thu Jan 29, 2026 at 12:26 PM GMT, FUJITA Tomonori wrote:
> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>
> Add AtomicFlag type for boolean flags.
>
> Document when AtomicFlag is generally preferable to Atomic<bool>: in
> particular, when RMW operations such as xchg()/cmpxchg() may be used
> and minimizing memory usage is not the top priority. On some
> architectures without byte-sized RMW instructions, Atomic<bool> can be
> slower for RMW operations.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Hi Fujita,

Thanks for the patch. I think this looks nice, so from design point of view:

Reviewed-by: Gary Guo <gary@garyguo.net>

However, Boqun reported that the codegen of `.bool_field` may involve a bit
masking instruction.

Best,
Gary

> ---
>  rust/kernel/sync/atomic.rs           | 125 +++++++++++++++++++++++++++
>  rust/kernel/sync/atomic/predefine.rs |  17 ++++
>  2 files changed, 142 insertions(+)
>
> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> index 4aebeacb961a..bfc393d98aa9 100644
> --- a/rust/kernel/sync/atomic.rs
> +++ b/rust/kernel/sync/atomic.rs
> @@ -560,3 +560,128 @@ pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering)
>          unsafe { from_repr(ret) }
>      }
>  }
> +
> +#[cfg(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64))]
> +#[repr(C)]
> +#[derive(Clone, Copy)]
> +struct Flag {
> +    bool_field: bool,
> +}
> +
> +/// # Invariants
> +///
> +/// `padding` must be all zeroes.
> +#[cfg(not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64)))]
> +#[repr(C, align(4))]
> +#[derive(Clone, Copy)]
> +struct Flag {
> +    #[cfg(target_endian = "big")]
> +    padding: [u8; 3],
> +    bool_field: bool,
> +    #[cfg(target_endian = "little")]
> +    padding: [u8; 3],
> +}
> +
> +impl Flag {
> +    #[inline(always)]
> +    const fn new(b: bool) -> Self {
> +        // INVARIANT: `padding` is all zeroes.
> +        Self {
> +            bool_field: b,
> +            #[cfg(not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64)))]
> +            padding: [0; 3],
> +        }
> +    }
> +}
> +
> +// SAFETY: `Flag` and `Repr` have the same size and alignment, and `Flag` is round-trip
> +// transmutable to the selected representation (`i8` or `i32`).
> +unsafe impl AtomicType for Flag {
> +    #[cfg(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64))]
> +    type Repr = i8;
> +    #[cfg(not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64)))]
> +    type Repr = i32;
> +}
> +
> +/// An atomic flag type intended to be backed by performance-optimal integer type.
> +///
> +/// The backing integer type is an implementation detail; it may vary by architecture and change
> +/// in the future.
> +///
> +/// [`AtomicFlag`] is generally preferable to [`Atomic<bool>`] when you need read-modify-write
> +/// (RMW) operations (e.g. [`Atomic::xchg()`]/[`Atomic::cmpxchg()`]) or when [`Atomic<bool>`] does
> +/// not save memory due to padding. On some architectures that do not support byte-sized atomic
> +/// RMW operations, RMW operations on [`Atomic<bool>`] are slower.
> +///
> +/// If you only use [`Atomic::load()`]/[`Atomic::store()`], [`Atomic<bool>`] is fine.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::atomic::{AtomicFlag, Relaxed};
> +///
> +/// let flag = AtomicFlag::new(false);
> +/// assert_eq!(false, flag.load(Relaxed));
> +/// flag.store(true, Relaxed);
> +/// assert_eq!(true, flag.load(Relaxed));
> +/// ```
> +pub struct AtomicFlag(Atomic<Flag>);
> +
> +impl AtomicFlag {
> +    /// Creates a new atomic flag.
> +    #[inline(always)]
> +    pub const fn new(b: bool) -> Self {
> +        Self(Atomic::new(Flag::new(b)))
> +    }
> +
> +    /// Returns a mutable reference to the underlying flag as a [`bool`].
> +    ///
> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::sync::atomic::{AtomicFlag, Relaxed};
> +    ///
> +    /// let mut atomic_flag = AtomicFlag::new(false);
> +    /// assert_eq!(false, atomic_flag.load(Relaxed));
> +    /// *atomic_flag.get_mut() = true;
> +    /// assert_eq!(true, atomic_flag.load(Relaxed));
> +    /// ```
> +    #[inline(always)]
> +    pub fn get_mut(&mut self) -> &mut bool {
> +        &mut self.0.get_mut().bool_field
> +    }
> +
> +    /// Loads the value from the atomic flag.
> +    #[inline(always)]
> +    pub fn load<Ordering: ordering::AcquireOrRelaxed>(&self, o: Ordering) -> bool {
> +        self.0.load(o).bool_field
> +    }
> +
> +    /// Stores a value to the atomic flag.
> +    #[inline(always)]
> +    pub fn store<Ordering: ordering::ReleaseOrRelaxed>(&self, v: bool, o: Ordering) {
> +        self.0.store(Flag::new(v), o);
> +    }
> +
> +    /// Stores a value to the atomic flag and returns the previous value.
> +    #[inline(always)]
> +    pub fn xchg<Ordering: ordering::Ordering>(&self, new: bool, o: Ordering) -> bool {
> +        self.0.xchg(Flag::new(new), o).bool_field
> +    }
> +
> +    /// Store a value to the atomic flag if the current value is equal to `old`.
> +    #[inline(always)]
> +    pub fn cmpxchg<Ordering: ordering::Ordering>(
> +        &self,
> +        old: bool,
> +        new: bool,
> +        o: Ordering,
> +    ) -> Result<bool, bool> {
> +        match self.0.cmpxchg(Flag::new(old), Flag::new(new), o) {
> +            Ok(_) => Ok(old),
> +            Err(f) => Err(f.bool_field),
> +        }
> +    }
> +}
> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> index 42067c6a266c..d14e10544dcf 100644
> --- a/rust/kernel/sync/atomic/predefine.rs
> +++ b/rust/kernel/sync/atomic/predefine.rs
> @@ -215,4 +215,21 @@ fn atomic_bool_tests() {
>          assert_eq!(false, x.load(Relaxed));
>          assert_eq!(Ok(false), x.cmpxchg(false, true, Full));
>      }
> +
> +    #[test]
> +    fn atomic_flag_tests() {
> +        let mut flag = AtomicFlag::new(false);
> +
> +        assert_eq!(false, flag.load(Relaxed));
> +
> +        *flag.get_mut() = true;
> +        assert_eq!(true, flag.load(Relaxed));
> +
> +        assert_eq!(true, flag.xchg(false, Relaxed));
> +        assert_eq!(false, flag.load(Relaxed));
> +
> +        *flag.get_mut() = true;
> +        assert_eq!(Ok(true), flag.cmpxchg(true, false, Full));
> +        assert_eq!(false, flag.load(Relaxed));
> +    }
>  }