On Sat Jan 17, 2026 at 12:22 PM GMT, Boqun Feng wrote: > Atomic pointer support is an important piece of synchronization > algorithm, e.g. RCU, hence provide the support for that. > > Note that instead of relying on atomic_long or the implementation of > `Atomic<usize>`, a new set of helpers (atomic_ptr_*) is introduced for > atomic pointer specifically, this is because ptr2int casting would > lose the provenance of a pointer and even though in theory there are a > few tricks the provenance can be restored, it'll still be a simpler > implementation if C could provide atomic pointers directly. The side > effects of this approach are: we don't have the arithmetic and logical > operations for pointers yet and the current implementation only works > on ARCH_SUPPORTS_ATOMIC_RMW architectures, but these are implementation > issues and can be added later. > > Signed-off-by: Boqun Feng <[email protected]>
I am happy that this is now using dedicated helpers for pointers, and not going through an intermediate integer which can lose provenance. Some feedbacks below, but in general LGTM. Reviewed-by: Gary Guo <[email protected]> > --- > rust/helpers/atomic_ext.c | 3 +++ > rust/kernel/sync/atomic.rs | 12 +++++++++++- > rust/kernel/sync/atomic/internal.rs | 21 +++++++++++++++------ > rust/kernel/sync/atomic/predefine.rs | 23 +++++++++++++++++++++++ > 4 files changed, 52 insertions(+), 7 deletions(-) > > diff --git a/rust/helpers/atomic_ext.c b/rust/helpers/atomic_ext.c > index 240218e2e708..c267d5190529 100644 > --- a/rust/helpers/atomic_ext.c > +++ b/rust/helpers/atomic_ext.c > @@ -36,6 +36,7 @@ __rust_helper void > rust_helper_atomic_##tname##_set_release(type *ptr, type val) > > GEN_READ_SET_HELPERS(i8, s8) > GEN_READ_SET_HELPERS(i16, s16) > +GEN_READ_SET_HELPERS(ptr, const void *) > > /* > * xchg helpers depend on ARCH_SUPPORTS_ATOMIC_RMW and on the > @@ -59,6 +60,7 @@ rust_helper_atomic_##tname##_xchg##suffix(type *ptr, type > new) \ > > GEN_XCHG_HELPERS(i8, s8) > GEN_XCHG_HELPERS(i16, s16) > +GEN_XCHG_HELPERS(ptr, const void *) > > /* > * try_cmpxchg helpers depend on ARCH_SUPPORTS_ATOMIC_RMW and on the > @@ -82,3 +84,4 @@ rust_helper_atomic_##tname##_try_cmpxchg##suffix(type *ptr, > type *old, type new) > > GEN_TRY_CMPXCHG_HELPERS(i8, s8) > GEN_TRY_CMPXCHG_HELPERS(i16, s16) > +GEN_TRY_CMPXCHG_HELPERS(ptr, const void *) > diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs > index 4aebeacb961a..4d2a5228c2e4 100644 > --- a/rust/kernel/sync/atomic.rs > +++ b/rust/kernel/sync/atomic.rs > @@ -51,6 +51,10 @@ > #[repr(transparent)] > pub struct Atomic<T: AtomicType>(AtomicRepr<T::Repr>); > > +// SAFETY: `Atomic<T>` is safe to transfer between execution contexts > because of the safety > +// requirement of `AtomicType`. > +unsafe impl<T: AtomicType> Send for Atomic<T> {} > + > // SAFETY: `Atomic<T>` is safe to share among execution contexts because all > accesses are atomic. > unsafe impl<T: AtomicType> Sync for Atomic<T> {} > > @@ -68,6 +72,11 @@ unsafe impl<T: AtomicType> Sync for Atomic<T> {} > /// > /// - [`Self`] must have the same size and alignment as [`Self::Repr`]. > /// - [`Self`] must be [round-trip transmutable] to [`Self::Repr`]. > +/// - [`Self`] must be safe to transfer between execution contexts, if it's > [`Send`], this is > +/// automatically satisfied. The exception is pointer types that are even > though marked as > +/// `!Send` (e.g. raw pointers and [`NonNull<T>`]) but requiring `unsafe` > to do anything > +/// meaningful on them. This is because transferring pointer values > between execution contexts is > +/// safe as long as the actual `unsafe` dereferencing is justified. I think the discussion about `Send` on pointers should be moved to the `impl<T> AtomicType for *mut T` side. > /// > /// Note that this is more relaxed than requiring the bi-directional > transmutability (i.e. > /// [`transmute()`] is always sound between `U` and `T`) because of the > support for atomic > @@ -108,7 +117,8 @@ unsafe impl<T: AtomicType> Sync for Atomic<T> {} > /// [`transmute()`]: core::mem::transmute > /// [round-trip transmutable]: AtomicType#round-trip-transmutability > /// [Examples]: AtomicType#examples > -pub unsafe trait AtomicType: Sized + Send + Copy { > +/// [`NonNull<T>`]: core::ptr::NonNull > +pub unsafe trait AtomicType: Sized + Copy { > /// The backing atomic implementation type. > type Repr: AtomicImpl; > } > diff --git a/rust/kernel/sync/atomic/internal.rs > b/rust/kernel/sync/atomic/internal.rs > index 0dac58bca2b3..93f5a7846645 100644 > --- a/rust/kernel/sync/atomic/internal.rs > +++ b/rust/kernel/sync/atomic/internal.rs > @@ -7,6 +7,7 @@ > use crate::bindings; > use crate::macros::paste; > use core::cell::UnsafeCell; > +use ffi::c_void; > > mod private { > /// Sealed trait marker to disable customized impls on atomic > implementation traits. > @@ -14,10 +15,11 @@ pub trait Sealed {} > } > > // The C side supports atomic primitives only for `i32` and `i64` > (`atomic_t` and `atomic64_t`), > -// while the Rust side also layers provides atomic support for `i8` and `i16` > -// on top of lower-level C primitives. > +// while the Rust side also provides atomic support for `i8`, `i16` and > `*const c_void` on top of > +// lower-level C primitives. > impl private::Sealed for i8 {} > impl private::Sealed for i16 {} > +impl private::Sealed for *const c_void {} > impl private::Sealed for i32 {} > impl private::Sealed for i64 {} > > @@ -26,10 +28,10 @@ impl private::Sealed for i64 {} > /// This trait is sealed, and only types that map directly to the C side > atomics > /// or can be implemented with lower-level C primitives are allowed to > implement this: > /// > -/// - `i8` and `i16` are implemented with lower-level C primitives. > +/// - `i8`, `i16` and `*const c_void` are implemented with lower-level C > primitives. > /// - `i32` map to `atomic_t` > /// - `i64` map to `atomic64_t` > -pub trait AtomicImpl: Sized + Send + Copy + private::Sealed { > +pub trait AtomicImpl: Sized + 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 > @@ -51,6 +53,13 @@ impl AtomicImpl for i16 { > type Delta = Self; > } > > +// The current helpers of load/store uses `{WRITE,READ}_ONCE()` hence the > atomicity is only > +// guaranteed against read-modify-write operations if the architecture > supports native atomic RmW. > +#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)] > +impl AtomicImpl for *const c_void { > + type Delta = isize; > +} > + > // `atomic_t` implements atomic operations on `i32`. > impl AtomicImpl for i32 { > type Delta = Self; > @@ -262,7 +271,7 @@ macro_rules! declare_and_impl_atomic_methods { > } > > declare_and_impl_atomic_methods!( > - [ i8 => atomic_i8, i16 => atomic_i16, i32 => atomic, i64 => atomic64 ] > + [ i8 => atomic_i8, i16 => atomic_i16, *const c_void => atomic_ptr, i32 > => atomic, i64 => atomic64 ] > /// Basic atomic operations > pub trait AtomicBasicOps { > /// Atomic read (load). > @@ -280,7 +289,7 @@ fn set[release](a: &AtomicRepr<Self>, v: Self) { > ); > > declare_and_impl_atomic_methods!( > - [ i8 => atomic_i8, i16 => atomic_i16, i32 => atomic, i64 => atomic64 ] > + [ i8 => atomic_i8, i16 => atomic_i16, *const c_void => atomic_ptr, i32 > => atomic, i64 => atomic64 ] > /// Exchange and compare-and-exchange atomic operations > pub trait AtomicExchangeOps { > /// Atomic exchange. > diff --git a/rust/kernel/sync/atomic/predefine.rs > b/rust/kernel/sync/atomic/predefine.rs > index 42067c6a266c..1a4670d225b5 100644 > --- a/rust/kernel/sync/atomic/predefine.rs > +++ b/rust/kernel/sync/atomic/predefine.rs > @@ -4,6 +4,7 @@ > > use crate::static_assert; > use core::mem::{align_of, size_of}; > +use ffi::c_void; > > // Ensure size and alignment requirements are checked. > static_assert!(size_of::<bool>() == size_of::<i8>()); > @@ -28,6 +29,16 @@ unsafe impl super::AtomicType for i16 { > type Repr = i16; > } > > +// SAFETY: > +// > +// - `*mut T` has the same size and alignment with `*const c_void`, and is > round-trip > +// transmutable to `*const c_void`. > +// - `*mut T` is safe to transfer between execution contexts. See the safety > requirement of > +// [`AtomicType`]. > +unsafe impl<T: Sized> super::AtomicType for *mut T { > + type Repr = *const c_void; > +} How about *const T? > + > // SAFETY: `i32` has the same size and alignment with itself, and is > round-trip transmutable to > // itself. > unsafe impl super::AtomicType for i32 { > @@ -215,4 +226,16 @@ fn atomic_bool_tests() { > assert_eq!(false, x.load(Relaxed)); > assert_eq!(Ok(false), x.cmpxchg(false, true, Full)); > } > + > + #[test] > + fn atomic_ptr_tests() { > + let mut v = 42; > + let mut u = 43; > + let x = Atomic::new(&raw mut v); > + > + assert_eq!(x.load(Acquire), &raw mut v); > + assert_eq!(x.cmpxchg(&raw mut u, &raw mut u, Relaxed), Err(&raw mut > v)); > + assert_eq!(x.cmpxchg(&raw mut v, &raw mut u, Relaxed), Ok(&raw mut > v)); > + assert_eq!(x.load(Relaxed), &raw mut u); > + } > }
