Message-ID: <aLu2_V00MUjg0aUV@archiso>
Date: Sat, 6 Sep 2025 04:22:21 +0000
From: Elle Rhumsaa <elle@...thered-steel.dev>
To: Boqun Feng <boqun.feng@...il.com>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
	lkmm@...ts.linux.dev, Will Deacon <will@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Mark Rutland <mark.rutland@....com>, Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"Paul E. McKenney" <paulmck@...nel.org>, stern@...land.harvard.edu,
	Miguel Ojeda <ojeda@...nel.org>, alex.gaynor@...il.com,
	Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <lossin@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
	Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>,
	Andreas Hindborg <a.hindborg@...nel.org>
Subject: Re: [PATCH 02/14] rust: sync: Add basic atomic operation mapping
 framework

On Thu, Sep 04, 2025 at 09:41:29PM -0700, Boqun Feng wrote:
> Preparation for generic atomic implementation. To unify the
> implementation of a generic method over `i32` and `i64`, the C side
> atomic methods need to be grouped so that in a generic method, they can
> be referred as <type>::<method>, otherwise their parameters and return
> value are different between `i32` and `i64`, which would require using
> `transmute()` to unify the type into a `T`.
> 
> Introduce `AtomicImpl` to represent a basic type in Rust that has the
> direct mapping to an atomic implementation from C. Use a sealed trait to
> restrict `AtomicImpl` to only support `i32` and `i64` for now.
> 
> Further, different methods are put into different `*Ops` trait groups,
> and this is for the future when smaller types like `i8`/`i16` are
> supported but only with a limited set of API (e.g. only set(), load(),
> xchg() and cmpxchg(), no add() or sub() etc).
> 
> While the atomic mod is introduced, documentation is also added for
> memory models and data races.
> 
> Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
> my responsibility on the Rust atomic mod.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
> Reviewed-by: Benno Lossin <lossin@...nel.org>
> Signed-off-by: Boqun Feng <boqun.feng@...il.com>
> Link: https://lore.kernel.org/all/20250719030827.61357-3-boqun.feng@gmail.com/
> ---
>  MAINTAINERS                         |   4 +-
>  rust/kernel/sync.rs                 |   1 +
>  rust/kernel/sync/atomic.rs          |  22 +++
>  rust/kernel/sync/atomic/internal.rs | 265 ++++++++++++++++++++++++++++
>  4 files changed, 291 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/sync/atomic.rs
>  create mode 100644 rust/kernel/sync/atomic/internal.rs
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index daf520a13bdf..37a753ec2aca 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3989,7 +3989,7 @@ F:	drivers/input/touchscreen/atmel_mxt_ts.c
>  ATOMIC INFRASTRUCTURE
>  M:	Will Deacon <will@...nel.org>
>  M:	Peter Zijlstra <peterz@...radead.org>
> -R:	Boqun Feng <boqun.feng@...il.com>
> +M:	Boqun Feng <boqun.feng@...il.com>
>  R:	Mark Rutland <mark.rutland@....com>
>  L:	linux-kernel@...r.kernel.org
>  S:	Maintained
> @@ -3998,6 +3998,8 @@ F:	arch/*/include/asm/atomic*.h
>  F:	include/*/atomic*.h
>  F:	include/linux/refcount.h
>  F:	scripts/atomic/
> +F:	rust/kernel/sync/atomic.rs
> +F:	rust/kernel/sync/atomic/
>  
>  ATTO EXPRESSSAS SAS/SATA RAID SCSI DRIVER
>  M:	Bradley Grove <linuxdrivers@...otech.com>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 00f9b558a3ad..7e962e5429d2 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -11,6 +11,7 @@
>  
>  mod arc;
>  pub mod aref;
> +pub mod atomic;
>  pub mod completion;
>  mod condvar;
>  pub mod lock;
> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> new file mode 100644
> index 000000000000..b9f2f4780073
> --- /dev/null
> +++ b/rust/kernel/sync/atomic.rs
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Atomic primitives.
> +//!
> +//! These primitives have the same semantics as their C counterparts: and the precise definitions of
> +//! semantics can be found at [`LKMM`]. Note that Linux Kernel Memory (Consistency) Model is the
> +//! only model for Rust code in kernel, and Rust's own atomics should be avoided.
> +//!
> +//! # Data races
> +//!
> +//! [`LKMM`] atomics have different rules regarding data races:
> +//!
> +//! - A normal write from C side is treated as an atomic write if
> +//!   CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=y.
> +//! - Mixed-size atomic accesses don't cause data races.
> +//!
> +//! [`LKMM`]: srctree/tools/memory-model/
> +
> +#[allow(dead_code, unreachable_pub)]
> +mod internal;
> +
> +pub use internal::AtomicImpl;
> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> new file mode 100644
> index 000000000000..6fdd8e59f45b
> --- /dev/null
> +++ b/rust/kernel/sync/atomic/internal.rs
> @@ -0,0 +1,265 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Atomic internal implementations.
> +//!
> +//! Provides 1:1 mapping to the C atomic operations.
> +
> +use crate::bindings;
> +use crate::macros::paste;
> +use core::cell::UnsafeCell;
> +
> +mod private {
> +    /// Sealed trait marker to disable customized impls on atomic implementation traits.
> +    pub trait Sealed {}
> +}
> +
> +// `i32` and `i64` are only supported atomic implementations.
> +impl private::Sealed for i32 {}
> +impl private::Sealed for i64 {}
> +
> +/// A marker trait for types that implement atomic operations with C side primitives.
> +///
> +/// This trait is sealed, and only types that have directly mapping to the C side atomics should
> +/// impl this:
> +///
> +/// - `i32` maps to `atomic_t`.
> +/// - `i64` maps to `atomic64_t`.
> +pub trait AtomicImpl: Sized + Send + Copy + private::Sealed {
> +    /// The type of the delta in arithmetic or logical operations.
> +    ///
> +    /// For example, in `atomic_add(ptr, v)`, it's the type of `v`. Usually it's the same type of
> +    /// [`Self`], but it may be different for the atomic pointer type.
> +    type Delta;
> +}
> +
> +// `atomic_t` implements atomic operations on `i32`.
> +impl AtomicImpl for i32 {
> +    type Delta = Self;
> +}
> +
> +// `atomic64_t` implements atomic operations on `i64`.
> +impl AtomicImpl for i64 {
> +    type Delta = Self;
> +}
> +
> +/// Atomic representation.
> +#[repr(transparent)]
> +pub struct AtomicRepr<T: AtomicImpl>(UnsafeCell<T>);
> +
> +impl<T: AtomicImpl> AtomicRepr<T> {
> +    /// Creates a new atomic representation `T`.
> +    pub const fn new(v: T) -> Self {
> +        Self(UnsafeCell::new(v))
> +    }
> +
> +    /// Returns a pointer to the underlying `T`.
> +    ///
> +    /// # Guarantees
> +    ///
> +    /// The returned pointer is valid and properly aligned (i.e. aligned to [`align_of::<T>()`]).
> +    pub const fn as_ptr(&self) -> *mut T {
> +        // GUARANTEE: `self.0` is an `UnsafeCell<T>`, therefore the pointer returned by `.get()`
> +        // must be valid and properly aligned.
> +        self.0.get()
> +    }
> +}
> +
> +// This macro generates the function signature with given argument list and return type.
> +macro_rules! declare_atomic_method {
> +    (
> +        $(#[doc=$doc:expr])*
> +        $func:ident($($arg:ident : $arg_type:ty),*) $(-> $ret:ty)?
> +    ) => {
> +        paste!(
> +            $(#[doc = $doc])*
> +            fn [< atomic_ $func >]($($arg: $arg_type,)*) $(-> $ret)?;
> +        );
> +    };
> +    (
> +        $(#[doc=$doc:expr])*
> +        $func:ident [$variant:ident $($rest:ident)*]($($arg_sig:tt)*) $(-> $ret:ty)?
> +    ) => {
> +        paste!(
> +            declare_atomic_method!(
> +                $(#[doc = $doc])*
> +                [< $func _ $variant >]($($arg_sig)*) $(-> $ret)?
> +            );
> +        );
> +
> +        declare_atomic_method!(
> +            $(#[doc = $doc])*
> +            $func [$($rest)*]($($arg_sig)*) $(-> $ret)?
> +        );
> +    };
> +    (
> +        $(#[doc=$doc:expr])*
> +        $func:ident []($($arg_sig:tt)*) $(-> $ret:ty)?
> +    ) => {
> +        declare_atomic_method!(
> +            $(#[doc = $doc])*
> +            $func($($arg_sig)*) $(-> $ret)?
> +        );
> +    }
> +}
> +
> +// This macro generates the function implementation with given argument list and return type, and it
> +// will replace "call(...)" expression with "$ctype _ $func" to call the real C function.
> +macro_rules! impl_atomic_method {
> +    (
> +        ($ctype:ident) $func:ident($($arg:ident: $arg_type:ty),*) $(-> $ret:ty)? {
> +            $unsafe:tt { call($($c_arg:expr),*) }
> +        }
> +    ) => {
> +        paste!(
> +            #[inline(always)]
> +            fn [< atomic_ $func >]($($arg: $arg_type,)*) $(-> $ret)? {
> +                // TODO: Ideally we want to use the SAFETY comments written at the macro invocation
> +                // (e.g. in `declare_and_impl_atomic_methods!()`, however, since SAFETY comments
> +                // are just comments, and they are not passed to macros as tokens, therefore we
> +                // cannot use them here. One potential improvement is that if we support using
> +                // attributes as an alternative for SAFETY comments, then we can use that for macro
> +                // generating code.
> +                //
> +                // SAFETY: specified on macro invocation.
> +                $unsafe { bindings::[< $ctype _ $func >]($($c_arg,)*) }
> +            }
> +        );
> +    };
> +    (
> +        ($ctype:ident) $func:ident[$variant:ident $($rest:ident)*]($($arg_sig:tt)*) $(-> $ret:ty)? {
> +            $unsafe:tt { call($($arg:tt)*) }
> +        }
> +    ) => {
> +        paste!(
> +            impl_atomic_method!(
> +                ($ctype) [< $func _ $variant >]($($arg_sig)*) $( -> $ret)? {
> +                    $unsafe { call($($arg)*) }
> +            }
> +            );
> +        );
> +        impl_atomic_method!(
> +            ($ctype) $func [$($rest)*]($($arg_sig)*) $( -> $ret)? {
> +                $unsafe { call($($arg)*) }
> +            }
> +        );
> +    };
> +    (
> +        ($ctype:ident) $func:ident[]($($arg_sig:tt)*) $( -> $ret:ty)? {
> +            $unsafe:tt { call($($arg:tt)*) }
> +        }
> +    ) => {
> +        impl_atomic_method!(
> +            ($ctype) $func($($arg_sig)*) $(-> $ret)? {
> +                $unsafe { call($($arg)*) }
> +            }
> +        );
> +    }
> +}
> +
> +// Delcares $ops trait with methods and implements the trait for `i32` and `i64`.
> +macro_rules! declare_and_impl_atomic_methods {
> +    ($(#[$attr:meta])* $pub:vis trait $ops:ident {
> +        $(
> +            $(#[doc=$doc:expr])*
> +            fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
> +                $unsafe:tt { bindings::#call($($arg:tt)*) }
> +            }
> +        )*
> +    }) => {
> +        $(#[$attr])*
> +        $pub trait $ops: AtomicImpl {
> +            $(
> +                declare_atomic_method!(
> +                    $(#[doc=$doc])*
> +                    $func[$($variant)*]($($arg_sig)*) $(-> $ret)?
> +                );
> +            )*
> +        }
> +
> +        impl $ops for i32 {
> +            $(
> +                impl_atomic_method!(
> +                    (atomic) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
> +                        $unsafe { call($($arg)*) }
> +                    }
> +                );
> +            )*
> +        }
> +
> +        impl $ops for i64 {
> +            $(
> +                impl_atomic_method!(
> +                    (atomic64) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
> +                        $unsafe { call($($arg)*) }
> +                    }
> +                );
> +            )*
> +        }
> +    }
> +}
> +
> +declare_and_impl_atomic_methods!(
> +    /// Basic atomic operations
> +    pub trait AtomicBasicOps {
> +        /// Atomic read (load).
> +        fn read[acquire](a: &AtomicRepr<Self>) -> Self {
> +            // SAFETY: `a.as_ptr()` is valid and properly aligned.
> +            unsafe { bindings::#call(a.as_ptr().cast()) }
> +        }
> +
> +        /// Atomic set (store).
> +        fn set[release](a: &AtomicRepr<Self>, v: Self) {
> +            // SAFETY: `a.as_ptr()` is valid and properly aligned.
> +            unsafe { bindings::#call(a.as_ptr().cast(), v) }
> +        }
> +    }
> +);
> +
> +declare_and_impl_atomic_methods!(
> +    /// Exchange and compare-and-exchange atomic operations
> +    pub trait AtomicExchangeOps {
> +        /// Atomic exchange.
> +        ///
> +        /// Atomically updates `*a` to `v` and returns the old value.
> +        fn xchg[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self) -> Self {
> +            // SAFETY: `a.as_ptr()` is valid and properly aligned.
> +            unsafe { bindings::#call(a.as_ptr().cast(), v) }
> +        }
> +
> +        /// Atomic compare and exchange.
> +        ///
> +        /// If `*a` == `*old`, atomically updates `*a` to `new`. Otherwise, `*a` is not
> +        /// modified, `*old` is updated to the current value of `*a`.
> +        ///
> +        /// Return `true` if the update of `*a` occurred, `false` otherwise.
> +        fn try_cmpxchg[acquire, release, relaxed](
> +            a: &AtomicRepr<Self>, old: &mut Self, new: Self
> +        ) -> bool {
> +            // SAFETY: `a.as_ptr()` is valid and properly aligned. `core::ptr::from_mut(old)`
> +            // is valid and properly aligned.
> +            unsafe { bindings::#call(a.as_ptr().cast(), core::ptr::from_mut(old), new) }
> +        }
> +    }
> +);
> +
> +declare_and_impl_atomic_methods!(
> +    /// Atomic arithmetic operations
> +    pub trait AtomicArithmeticOps {
> +        /// Atomic add (wrapping).
> +        ///
> +        /// Atomically updates `*a` to `(*a).wrapping_add(v)`.
> +        fn add[](a: &AtomicRepr<Self>, v: Self::Delta) {
> +            // SAFETY: `a.as_ptr()` is valid and properly aligned.
> +            unsafe { bindings::#call(v, a.as_ptr().cast()) }
> +        }
> +
> +        /// Atomic fetch and add (wrapping).
> +        ///
> +        /// Atomically updates `*a` to `(*a).wrapping_add(v)`, and returns the value of `*a`
> +        /// before the update.
> +        fn fetch_add[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self::Delta) -> Self {
> +            // SAFETY: `a.as_ptr()` is valid and properly aligned.
> +            unsafe { bindings::#call(v, a.as_ptr().cast()) }
> +        }
> +    }
> +);
> -- 
> 2.51.0
> 
> 

Reviewed-by: Elle Rhumsaa <elle@...thered-steel.dev>

Powered by blists - more mailing lists