[PATCH v7 2/9] rust: sync: Add basic atomic operation mapping framework

Boqun Feng posted 9 patches 2 months, 3 weeks ago
[PATCH v7 2/9] rust: sync: Add basic atomic operation mapping framework
Posted by Boqun Feng 2 months, 3 weeks ago
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. This trait is sealed,
and currently only `i32` and `i64` impl this.

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 responsiblity on the Rust atomic mod.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
Benno, I actually followed your suggestion and put the safety
requirement inline, and also I realized I don't need to mention about
data race, because no data race is an implied safety requirement. Note
that macro-wise, I forced only #[doc] attributes can be put before
`unsafe fn ..` because this is the only usage, and I don't think it's
likely we want to support other attributes. We can always add them
later.

 MAINTAINERS                    |   4 +-
 rust/kernel/sync.rs            |   1 +
 rust/kernel/sync/atomic.rs     |  19 +++
 rust/kernel/sync/atomic/ops.rs | 265 +++++++++++++++++++++++++++++++++
 4 files changed, 288 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/sync/atomic.rs
 create mode 100644 rust/kernel/sync/atomic/ops.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index 0c1d245bf7b8..5eef524975ca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3894,7 +3894,7 @@ F:	drivers/input/touchscreen/atmel_mxt_ts.c
 ATOMIC INFRASTRUCTURE
 M:	Will Deacon <will@kernel.org>
 M:	Peter Zijlstra <peterz@infradead.org>
-R:	Boqun Feng <boqun.feng@gmail.com>
+M:	Boqun Feng <boqun.feng@gmail.com>
 R:	Mark Rutland <mark.rutland@arm.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
@@ -3903,6 +3903,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@attotech.com>
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 36a719015583..b620027e0641 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -10,6 +10,7 @@
 use pin_init;
 
 mod arc;
+pub mod atomic;
 mod condvar;
 pub mod lock;
 mod locked_by;
diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
new file mode 100644
index 000000000000..c9c7c3617dd5
--- /dev/null
+++ b/rust/kernel/sync/atomic.rs
@@ -0,0 +1,19 @@
+// 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/
+
+pub mod ops;
diff --git a/rust/kernel/sync/atomic/ops.rs b/rust/kernel/sync/atomic/ops.rs
new file mode 100644
index 000000000000..1353dc748ef9
--- /dev/null
+++ b/rust/kernel/sync/atomic/ops.rs
@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Atomic implementations.
+//!
+//! Provides 1:1 mapping of atomic implementations.
+
+use crate::bindings;
+use crate::macros::paste;
+
+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;
+}
+
+// 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])*
+            unsafe 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)? {
+            call($($c_arg:expr),*)
+        }
+    ) => {
+        paste!(
+            #[inline(always)]
+            unsafe fn [< atomic_ $func >]($($arg: $arg_type,)*) $(-> $ret)? {
+                // SAFETY: Per function safety requirement, all pointers are aligned and valid, and
+                // accesses won't cause data race per LKMM.
+                unsafe { bindings::[< $ctype _ $func >]($($c_arg,)*) }
+            }
+        );
+    };
+    (
+        ($ctype:ident) $func:ident[$variant:ident $($rest:ident)*]($($arg_sig:tt)*) $(-> $ret:ty)? {
+            call($($arg:tt)*)
+        }
+    ) => {
+        paste!(
+            impl_atomic_method!(
+                ($ctype) [< $func _ $variant >]($($arg_sig)*) $( -> $ret)? {
+                    call($($arg)*)
+            }
+            );
+        );
+        impl_atomic_method!(
+            ($ctype) $func [$($rest)*]($($arg_sig)*) $( -> $ret)? {
+                call($($arg)*)
+            }
+        );
+    };
+    (
+        ($ctype:ident) $func:ident[]($($arg_sig:tt)*) $( -> $ret:ty)? {
+            call($($arg:tt)*)
+        }
+    ) => {
+        impl_atomic_method!(
+            ($ctype) $func($($arg_sig)*) $(-> $ret)? {
+                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 trait $ops:ident {
+        $(
+            $(#[doc=$doc:expr])*
+            unsafe fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
+                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)? {
+                        call($($arg)*)
+                    }
+                );
+            )*
+        }
+
+        impl $ops for i64 {
+            $(
+                impl_atomic_method!(
+                    (atomic64) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
+                        call($($arg)*)
+                    }
+                );
+            )*
+        }
+    }
+}
+
+declare_and_impl_atomic_methods!(
+    /// Basic atomic operations
+    pub trait AtomicHasBasicOps {
+        /// Atomic read (load).
+        ///
+        /// # Safety
+        /// - `ptr` is aligned to [`align_of::<Self>()`].
+        /// - `ptr` is valid for reads.
+        ///
+        /// [`align_of::<Self>()`]: core::mem::align_of
+        unsafe fn read[acquire](ptr: *mut Self) -> Self {
+            bindings::#call(ptr.cast())
+        }
+
+        /// Atomic set (store).
+        ///
+        /// # Safety
+        /// - `ptr` is aligned to [`align_of::<Self>()`].
+        /// - `ptr` is valid for writes.
+        ///
+        /// [`align_of::<Self>()`]: core::mem::align_of
+        unsafe fn set[release](ptr: *mut Self, v: Self) {
+            bindings::#call(ptr.cast(), v)
+        }
+    }
+);
+
+declare_and_impl_atomic_methods!(
+    /// Exchange and compare-and-exchange atomic operations
+    pub trait AtomicHasXchgOps {
+        /// Atomic exchange.
+        ///
+        /// Atomically updates `*ptr` to `v` and returns the old value.
+        ///
+        /// # Safety
+        /// - `ptr` is aligned to [`align_of::<Self>()`].
+        /// - `ptr` is valid for reads and writes.
+        ///
+        /// [`align_of::<Self>()`]: core::mem::align_of
+        unsafe fn xchg[acquire, release, relaxed](ptr: *mut Self, v: Self) -> Self {
+            bindings::#call(ptr.cast(), v)
+        }
+
+        /// Atomic compare and exchange.
+        ///
+        /// If `*ptr` == `*old`, atomically updates `*ptr` to `new`. Otherwise, `*ptr` is not
+        /// modified, `*old` is updated to the current value of `*ptr`.
+        ///
+        /// Return `true` if the update of `*ptr` occured, `false` otherwise.
+        ///
+        /// # Safety
+        /// - `ptr` is aligned to [`align_of::<Self>()`].
+        /// - `ptr` is valid for reads and writes.
+        /// - `old` is aligned to [`align_of::<Self>()`].
+        /// - `old` is valid for reads and writes.
+        ///
+        /// [`align_of::<Self>()`]: core::mem::align_of
+        unsafe fn try_cmpxchg[acquire, release, relaxed](ptr: *mut Self, old: *mut Self, new: Self) -> bool {
+            bindings::#call(ptr.cast(), old, new)
+        }
+    }
+);
+
+declare_and_impl_atomic_methods!(
+    /// Atomic arithmetic operations
+    pub trait AtomicHasArithmeticOps {
+        /// Atomic add (wrapping).
+        ///
+        /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`.
+        ///
+        /// # Safety
+        /// - `ptr` is aligned to `align_of::<Self>()`.
+        /// - `ptr` is valid for reads and writes.
+        ///
+        /// [`align_of::<Self>()`]: core::mem::align_of
+        unsafe fn add[](ptr: *mut Self, v: Self::Delta) {
+            bindings::#call(v, ptr.cast())
+        }
+
+        /// Atomic fetch and add (wrapping).
+        ///
+        /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`, and returns the value of `*ptr`
+        /// before the update.
+        ///
+        /// # Safety
+        /// - `ptr` is aligned to `align_of::<Self>()`.
+        /// - `ptr` is valid for reads and writes.
+        ///
+        /// [`align_of::<Self>()`]: core::mem::align_of
+        unsafe fn fetch_add[acquire, release, relaxed](ptr: *mut Self, v: Self::Delta) -> Self {
+            bindings::#call(v, ptr.cast())
+        }
+    }
+);
-- 
2.39.5 (Apple Git-154)
Re: [PATCH v7 2/9] rust: sync: Add basic atomic operation mapping framework
Posted by Benno Lossin 2 months, 3 weeks ago
On Mon Jul 14, 2025 at 7:36 AM CEST, 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. This trait is sealed,
> and currently only `i32` and `i64` impl this.
>
> 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 responsiblity on the Rust atomic mod.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Benno Lossin <lossin@kernel.org>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
> Benno, I actually followed your suggestion and put the safety
> requirement inline, and also I realized I don't need to mention about
> data race, because no data race is an implied safety requirement.

Thanks! I think it looks much better :)

> Note that macro-wise, I forced only #[doc] attributes can be put
> before `unsafe fn ..` because this is the only usage, and I don't
> think it's likely we want to support other attributes. We can always
> add them later.

Sounds good.

> +declare_and_impl_atomic_methods!(
> +    /// Basic atomic operations
> +    pub trait AtomicHasBasicOps {

I think we should drop the `Has` from the names. So this one can just be
`AtomicBasicOps`. Or how about `BasicAtomic`, or `AtomicBase`?

> +        /// Atomic read (load).
> +        ///
> +        /// # Safety
> +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> +        /// - `ptr` is valid for reads.
> +        ///
> +        /// [`align_of::<Self>()`]: core::mem::align_of
> +        unsafe fn read[acquire](ptr: *mut Self) -> Self {
> +            bindings::#call(ptr.cast())
> +        }
> +
> +        /// Atomic set (store).
> +        ///
> +        /// # Safety
> +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> +        /// - `ptr` is valid for writes.
> +        ///
> +        /// [`align_of::<Self>()`]: core::mem::align_of
> +        unsafe fn set[release](ptr: *mut Self, v: Self) {
> +            bindings::#call(ptr.cast(), v)
> +        }
> +    }
> +);
> +
> +declare_and_impl_atomic_methods!(
> +    /// Exchange and compare-and-exchange atomic operations
> +    pub trait AtomicHasXchgOps {

Same here `AtomicXchgOps` or `AtomicExchangeOps` or `AtomicExchange`?
(I would prefer to not abbreviate it to `Xchg`)

> +        /// Atomic exchange.
> +        ///
> +        /// Atomically updates `*ptr` to `v` and returns the old value.
> +        ///
> +        /// # Safety
> +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> +        /// - `ptr` is valid for reads and writes.
> +        ///
> +        /// [`align_of::<Self>()`]: core::mem::align_of
> +        unsafe fn xchg[acquire, release, relaxed](ptr: *mut Self, v: Self) -> Self {
> +            bindings::#call(ptr.cast(), v)
> +        }
> +
> +        /// Atomic compare and exchange.
> +        ///
> +        /// If `*ptr` == `*old`, atomically updates `*ptr` to `new`. Otherwise, `*ptr` is not
> +        /// modified, `*old` is updated to the current value of `*ptr`.
> +        ///
> +        /// Return `true` if the update of `*ptr` occured, `false` otherwise.
> +        ///
> +        /// # Safety
> +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> +        /// - `ptr` is valid for reads and writes.
> +        /// - `old` is aligned to [`align_of::<Self>()`].
> +        /// - `old` is valid for reads and writes.
> +        ///
> +        /// [`align_of::<Self>()`]: core::mem::align_of
> +        unsafe fn try_cmpxchg[acquire, release, relaxed](ptr: *mut Self, old: *mut Self, new: Self) -> bool {
> +            bindings::#call(ptr.cast(), old, new)
> +        )}
> +    }
> +);
> +
> +declare_and_impl_atomic_methods!(
> +    /// Atomic arithmetic operations
> +    pub trait AtomicHasArithmeticOps {

Forgot to rename this one to `Add`? I think `AtomicAdd` sounds best for
this one.

---
Cheers,
Benno

> +        /// Atomic add (wrapping).
> +        ///
> +        /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`.
> +        ///
> +        /// # Safety
> +        /// - `ptr` is aligned to `align_of::<Self>()`.
> +        /// - `ptr` is valid for reads and writes.
> +        ///
> +        /// [`align_of::<Self>()`]: core::mem::align_of
> +        unsafe fn add[](ptr: *mut Self, v: Self::Delta) {
> +            bindings::#call(v, ptr.cast())
> +        }
> +
> +        /// Atomic fetch and add (wrapping).
> +        ///
> +        /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`, and returns the value of `*ptr`
> +        /// before the update.
> +        ///
> +        /// # Safety
> +        /// - `ptr` is aligned to `align_of::<Self>()`.
> +        /// - `ptr` is valid for reads and writes.
> +        ///
> +        /// [`align_of::<Self>()`]: core::mem::align_of
> +        unsafe fn fetch_add[acquire, release, relaxed](ptr: *mut Self, v: Self::Delta) -> Self {
> +            bindings::#call(v, ptr.cast())
> +        }
> +    }
> +);
Re: [PATCH v7 2/9] rust: sync: Add basic atomic operation mapping framework
Posted by Boqun Feng 2 months, 3 weeks ago
On Mon, Jul 14, 2025 at 12:03:11PM +0200, Benno Lossin wrote:
[...]
> > +declare_and_impl_atomic_methods!(
> > +    /// Basic atomic operations
> > +    pub trait AtomicHasBasicOps {
> 
> I think we should drop the `Has` from the names. So this one can just be
> `AtomicBasicOps`. Or how about `BasicAtomic`, or `AtomicBase`?
> 

Actually, given your feedback to `ordering::Any` vs `core::any::Any`,
I think it makes more sense to keep the current names ;-) These are only
trait names to describe what operations do an `AtomicImpl` has, and they
should be used only in atomic mod, so naming them a bit longer is not a
problem. And by doing so, we free the better names `BasicAtomic` or
`AtomicBase` for other future usages.

Best I could do is `AtomicBasicOps` or `HasAtomicBasicOps`.

> > +        /// Atomic read (load).
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> > +        /// - `ptr` is valid for reads.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn read[acquire](ptr: *mut Self) -> Self {
> > +            bindings::#call(ptr.cast())
> > +        }
> > +
> > +        /// Atomic set (store).
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> > +        /// - `ptr` is valid for writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn set[release](ptr: *mut Self, v: Self) {
> > +            bindings::#call(ptr.cast(), v)
> > +        }
> > +    }
> > +);
> > +
> > +declare_and_impl_atomic_methods!(
> > +    /// Exchange and compare-and-exchange atomic operations
> > +    pub trait AtomicHasXchgOps {
> 
> Same here `AtomicXchgOps` or `AtomicExchangeOps` or `AtomicExchange`?
> (I would prefer to not abbreviate it to `Xchg`)
> 

The "Xchg" -> "Exchange" part seems fine to me.

> > +        /// Atomic exchange.
> > +        ///
> > +        /// Atomically updates `*ptr` to `v` and returns the old value.
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> > +        /// - `ptr` is valid for reads and writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn xchg[acquire, release, relaxed](ptr: *mut Self, v: Self) -> Self {
> > +            bindings::#call(ptr.cast(), v)
> > +        }
> > +
> > +        /// Atomic compare and exchange.
> > +        ///
> > +        /// If `*ptr` == `*old`, atomically updates `*ptr` to `new`. Otherwise, `*ptr` is not
> > +        /// modified, `*old` is updated to the current value of `*ptr`.
> > +        ///
> > +        /// Return `true` if the update of `*ptr` occured, `false` otherwise.
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to [`align_of::<Self>()`].
> > +        /// - `ptr` is valid for reads and writes.
> > +        /// - `old` is aligned to [`align_of::<Self>()`].
> > +        /// - `old` is valid for reads and writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn try_cmpxchg[acquire, release, relaxed](ptr: *mut Self, old: *mut Self, new: Self) -> bool {
> > +            bindings::#call(ptr.cast(), old, new)
> > +        )}
> > +    }
> > +);
> > +
> > +declare_and_impl_atomic_methods!(
> > +    /// Atomic arithmetic operations
> > +    pub trait AtomicHasArithmeticOps {
> 
> Forgot to rename this one to `Add`? I think `AtomicAdd` sounds best for

No, because at the `AtomicImpl` level, it's easy to know whether a type
has all the arithmetic operations or none (also the `Delta` type should
be known). But I don't have opinions on renaming it to `AtomicAddOps` or
`HasAtomicAddOps`.

Regards,
Boqun

> this one.
> 
> ---
> Cheers,
> Benno
> 
> > +        /// Atomic add (wrapping).
> > +        ///
> > +        /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`.
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to `align_of::<Self>()`.
> > +        /// - `ptr` is valid for reads and writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn add[](ptr: *mut Self, v: Self::Delta) {
> > +            bindings::#call(v, ptr.cast())
> > +        }
> > +
> > +        /// Atomic fetch and add (wrapping).
> > +        ///
> > +        /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`, and returns the value of `*ptr`
> > +        /// before the update.
> > +        ///
> > +        /// # Safety
> > +        /// - `ptr` is aligned to `align_of::<Self>()`.
> > +        /// - `ptr` is valid for reads and writes.
> > +        ///
> > +        /// [`align_of::<Self>()`]: core::mem::align_of
> > +        unsafe fn fetch_add[acquire, release, relaxed](ptr: *mut Self, v: Self::Delta) -> Self {
> > +            bindings::#call(v, ptr.cast())
> > +        }
> > +    }
> > +);
>
Re: [PATCH v7 2/9] rust: sync: Add basic atomic operation mapping framework
Posted by Benno Lossin 2 months, 3 weeks ago
On Mon Jul 14, 2025 at 3:42 PM CEST, Boqun Feng wrote:
> On Mon, Jul 14, 2025 at 12:03:11PM +0200, Benno Lossin wrote:
> [...]
>> > +declare_and_impl_atomic_methods!(
>> > +    /// Basic atomic operations
>> > +    pub trait AtomicHasBasicOps {
>> 
>> I think we should drop the `Has` from the names. So this one can just be
>> `AtomicBasicOps`. Or how about `BasicAtomic`, or `AtomicBase`?
>> 
>
> Actually, given your feedback to `ordering::Any` vs `core::any::Any`,
> I think it makes more sense to keep the current names ;-) These are only
> trait names to describe what operations do an `AtomicImpl` has, and they
> should be used only in atomic mod, so naming them a bit longer is not a
> problem. And by doing so, we free the better names `BasicAtomic` or
> `AtomicBase` for other future usages.
>
> Best I could do is `AtomicBasicOps` or `HasAtomicBasicOps`.

But you are already in the `ops` namespace, so by your argument you
could just use `ops::BasicAtomic` :)

I'm fine with `AtomicBasicOps`.

>> > +declare_and_impl_atomic_methods!(
>> > +    /// Atomic arithmetic operations
>> > +    pub trait AtomicHasArithmeticOps {
>> 
>> Forgot to rename this one to `Add`? I think `AtomicAdd` sounds best for
>
> No, because at the `AtomicImpl` level, it's easy to know whether a type
> has all the arithmetic operations or none (also the `Delta` type should
> be known). But I don't have opinions on renaming it to `AtomicAddOps` or
> `HasAtomicAddOps`.

Ahh right yeah this makes sense. So let's use `AtomicArithmeticOps` if
you insist on the `Ops` part.

---
Cheers,
Benno
Re: [PATCH v7 2/9] rust: sync: Add basic atomic operation mapping framework
Posted by Boqun Feng 2 months, 3 weeks ago
On Mon, Jul 14, 2025 at 05:00:56PM +0200, Benno Lossin wrote:
> On Mon Jul 14, 2025 at 3:42 PM CEST, Boqun Feng wrote:
> > On Mon, Jul 14, 2025 at 12:03:11PM +0200, Benno Lossin wrote:
> > [...]
> >> > +declare_and_impl_atomic_methods!(
> >> > +    /// Basic atomic operations
> >> > +    pub trait AtomicHasBasicOps {
> >> 
> >> I think we should drop the `Has` from the names. So this one can just be
> >> `AtomicBasicOps`. Or how about `BasicAtomic`, or `AtomicBase`?
> >> 
> >
> > Actually, given your feedback to `ordering::Any` vs `core::any::Any`,
> > I think it makes more sense to keep the current names ;-) These are only
> > trait names to describe what operations do an `AtomicImpl` has, and they
> > should be used only in atomic mod, so naming them a bit longer is not a
> > problem. And by doing so, we free the better names `BasicAtomic` or
> > `AtomicBase` for other future usages.
> >
> > Best I could do is `AtomicBasicOps` or `HasAtomicBasicOps`.
> 
> But you are already in the `ops` namespace, so by your argument you
> could just use `ops::BasicAtomic` :)
> 
> I'm fine with `AtomicBasicOps`.
> 
> >> > +declare_and_impl_atomic_methods!(
> >> > +    /// Atomic arithmetic operations
> >> > +    pub trait AtomicHasArithmeticOps {
> >> 
> >> Forgot to rename this one to `Add`? I think `AtomicAdd` sounds best for
> >
> > No, because at the `AtomicImpl` level, it's easy to know whether a type
> > has all the arithmetic operations or none (also the `Delta` type should
> > be known). But I don't have opinions on renaming it to `AtomicAddOps` or
> > `HasAtomicAddOps`.
> 
> Ahh right yeah this makes sense. So let's use `AtomicArithmeticOps` if
> you insist on the `Ops` part.
> 

Done with AtomicBasicOps AtomicExchangeOps and AtomicArithmeticOps ;-)
Thank you!

Regards,
Boqun

> ---
> Cheers,
> Benno