To provide using LKMM atomics for Rust code, a generic `Atomic<T>` is
added, currently `T` needs to be Send + Copy because these are the
straightforward usages and all basic types support this. The trait
`AllowAtomic` should be only implemented inside atomic mod until the
generic atomic framework is mature enough (unless the implementer is a
`#[repr(transparent)]` new type).

`AtomicImpl` types are automatically `AllowAtomic`, and so far only
basic operations load() and store() are introduced.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic.rs         |   2 +
 rust/kernel/sync/atomic/generic.rs | 258 +++++++++++++++++++++++++++++
 2 files changed, 260 insertions(+)
 create mode 100644 rust/kernel/sync/atomic/generic.rs

diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index 9fe5d81fc2a9..a01e44eec380 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -16,7 +16,9 @@
 //!
 //! [`LKMM`]: srctree/tools/memory-mode/
 
+pub mod generic;
 pub mod ops;
 pub mod ordering;
 
+pub use generic::Atomic;
 pub use ordering::{Acquire, Full, Relaxed, Release};
diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
new file mode 100644
index 000000000000..73c26f9cf6b8
--- /dev/null
+++ b/rust/kernel/sync/atomic/generic.rs
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Generic atomic primitives.
+
+use super::ops::*;
+use super::ordering::*;
+use crate::types::Opaque;
+
+/// A generic atomic variable.
+///
+/// `T` must impl [`AllowAtomic`], that is, an [`AtomicImpl`] has to be chosen.
+///
+/// # Invariants
+///
+/// Doing an atomic operation while holding a reference of [`Self`] won't cause a data race, this
+/// is guaranteed by the safety requirement of [`Self::from_ptr`] and the extra safety requirement
+/// of the usage on pointers returned by [`Self::as_ptr`].
+#[repr(transparent)]
+pub struct Atomic<T: AllowAtomic>(Opaque<T>);
+
+// SAFETY: `Atomic<T>` is safe to share among execution contexts because all accesses are atomic.
+unsafe impl<T: AllowAtomic> Sync for Atomic<T> {}
+
+/// Atomics that support basic atomic operations.
+///
+/// TODO: Currently the [`AllowAtomic`] types are restricted within basic integer types (and their
+/// transparent new types). In the future, we could extend the scope to more data types when there
+/// is a clear and meaningful usage, but for now, [`AllowAtomic`] should only be implemented inside
+/// atomic mod for the restricted types mentioned above.
+///
+/// # Safety
+///
+/// [`Self`] must have the same size and alignment as [`Self::Repr`].
+pub unsafe trait AllowAtomic: Sized + Send + Copy {
+    /// The backing atomic implementation type.
+    type Repr: AtomicImpl;
+
+    /// Converts into a [`Self::Repr`].
+    fn into_repr(self) -> Self::Repr;
+
+    /// Converts from a [`Self::Repr`].
+    fn from_repr(repr: Self::Repr) -> Self;
+}
+
+// An `AtomicImpl` is automatically an `AllowAtomic`.
+//
+// SAFETY: `T::Repr` is `Self` (i.e. `T`), so they have the same size and alignment.
+unsafe impl<T: AtomicImpl> AllowAtomic for T {
+    type Repr = Self;
+
+    fn into_repr(self) -> Self::Repr {
+        self
+    }
+
+    fn from_repr(repr: Self::Repr) -> Self {
+        repr
+    }
+}
+
+impl<T: AllowAtomic> Atomic<T> {
+    /// Creates a new atomic.
+    pub const fn new(v: T) -> Self {
+        Self(Opaque::new(v))
+    }
+
+    /// Creates a reference to [`Self`] from a pointer.
+    ///
+    /// # Safety
+    ///
+    /// - `ptr` has to be a valid pointer.
+    /// - `ptr` has to be valid for both reads and writes for the whole lifetime `'a`.
+    /// - For the whole lifetime of '`a`, other accesses to the object cannot cause data races
+    ///   (defined by [`LKMM`]) against atomic operations on the returned reference.
+    ///
+    /// [`LKMM`]: srctree/tools/memory-model
+    ///
+    /// # Examples
+    ///
+    /// Using [`Atomic::from_ptr()`] combined with [`Atomic::load()`] or [`Atomic::store()`] can
+    /// achieve the same functionality as `READ_ONCE()`/`smp_load_acquire()` or
+    /// `WRITE_ONCE()`/`smp_store_release()` in C side:
+    ///
+    /// ```rust
+    /// # use kernel::types::Opaque;
+    /// use kernel::sync::atomic::{Atomic, Relaxed, Release};
+    ///
+    /// // Assume there is a C struct `Foo`.
+    /// mod cbindings {
+    ///     #[repr(C)]
+    ///     pub(crate) struct foo { pub(crate) a: i32, pub(crate) b: i32 }
+    /// }
+    ///
+    /// let tmp = Opaque::new(cbindings::foo { a: 1, b: 2});
+    ///
+    /// // struct foo *foo_ptr = ..;
+    /// let foo_ptr = tmp.get();
+    ///
+    /// // SAFETY: `foo_ptr` is a valid pointer, and `.a` is inbound.
+    /// let foo_a_ptr = unsafe { core::ptr::addr_of_mut!((*foo_ptr).a) };
+    ///
+    /// // a = READ_ONCE(foo_ptr->a);
+    /// //
+    /// // SAFETY: `foo_a_ptr` is a valid pointer for read, and all accesses on it is atomic, so no
+    /// // data race.
+    /// let a = unsafe { Atomic::from_ptr(foo_a_ptr) }.load(Relaxed);
+    /// # assert_eq!(a, 1);
+    ///
+    /// // smp_store_release(&foo_ptr->a, 2);
+    /// //
+    /// // SAFETY: `foo_a_ptr` is a valid pointer for write, and all accesses on it is atomic, so no
+    /// // data race.
+    /// unsafe { Atomic::from_ptr(foo_a_ptr) }.store(2, Release);
+    /// ```
+    ///
+    /// However, this should be only used when communicating with C side or manipulating a C struct.
+    pub unsafe fn from_ptr<'a>(ptr: *mut T) -> &'a Self
+    where
+        T: Sync,
+    {
+        // CAST: `T` is transparent to `Atomic<T>`.
+        // SAFETY: Per function safety requirement, `ptr` is a valid pointer and the object will
+        // live long enough. It's safe to return a `&Atomic<T>` because function safety requirement
+        // guarantees other accesses won't cause data races.
+        unsafe { &*ptr.cast::<Self>() }
+    }
+
+    /// Returns a pointer to the underlying atomic variable.
+    ///
+    /// Extra safety requirement on using the return pointer: the operations done via the pointer
+    /// cannot cause data races defined by [`LKMM`].
+    ///
+    /// [`LKMM`]: srctree/tools/memory-model
+    pub const fn as_ptr(&self) -> *mut T {
+        self.0.get()
+    }
+
+    /// Returns a mutable reference to the underlying atomic variable.
+    ///
+    /// This is safe because the mutable reference of the atomic variable guarantees the exclusive
+    /// access.
+    pub fn get_mut(&mut self) -> &mut T {
+        // SAFETY: `self.as_ptr()` is a valid pointer to `T`, and the object has already been
+        // initialized. `&mut self` guarantees the exclusive access, so it's safe to reborrow
+        // mutably.
+        unsafe { &mut *self.as_ptr() }
+    }
+}
+
+impl<T: AllowAtomic> Atomic<T>
+where
+    T::Repr: AtomicHasBasicOps,
+{
+    /// Loads the value from the atomic variable.
+    ///
+    /// # Examples
+    ///
+    /// Simple usages:
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{Atomic, Relaxed};
+    ///
+    /// let x = Atomic::new(42i32);
+    ///
+    /// assert_eq!(42, x.load(Relaxed));
+    ///
+    /// let x = Atomic::new(42i64);
+    ///
+    /// assert_eq!(42, x.load(Relaxed));
+    /// ```
+    ///
+    /// Customized new types in [`Atomic`]:
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{generic::AllowAtomic, Atomic, Relaxed};
+    ///
+    /// #[derive(Clone, Copy)]
+    /// #[repr(transparent)]
+    /// struct NewType(u32);
+    ///
+    /// // SAFETY: `NewType` is transparent to `u32`, which has the same size and alignment as
+    /// // `i32`.
+    /// unsafe impl AllowAtomic for NewType {
+    ///     type Repr = i32;
+    ///
+    ///     fn into_repr(self) -> Self::Repr {
+    ///         self.0 as i32
+    ///     }
+    ///
+    ///     fn from_repr(repr: Self::Repr) -> Self {
+    ///         NewType(repr as u32)
+    ///     }
+    /// }
+    ///
+    /// let n = Atomic::new(NewType(0));
+    ///
+    /// assert_eq!(0, n.load(Relaxed).0);
+    /// ```
+    #[doc(alias("atomic_read", "atomic64_read"))]
+    #[inline(always)]
+    pub fn load<Ordering: AcquireOrRelaxed>(&self, _: Ordering) -> T {
+        let a = self.as_ptr().cast::<T::Repr>();
+
+        // SAFETY:
+        // - For calling the atomic_read*() function:
+        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
+        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
+        //   - per the type invariants, the following atomic operation won't cause data races.
+        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
+        //   - atomic operations are used here.
+        let v = unsafe {
+            if Ordering::IS_RELAXED {
+                T::Repr::atomic_read(a)
+            } else {
+                T::Repr::atomic_read_acquire(a)
+            }
+        };
+
+        T::from_repr(v)
+    }
+
+    /// Stores a value to the atomic variable.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{Atomic, Relaxed};
+    ///
+    /// let x = Atomic::new(42i32);
+    ///
+    /// assert_eq!(42, x.load(Relaxed));
+    ///
+    /// x.store(43, Relaxed);
+    ///
+    /// assert_eq!(43, x.load(Relaxed));
+    /// ```
+    ///
+    #[doc(alias("atomic_set", "atomic64_set"))]
+    #[inline(always)]
+    pub fn store<Ordering: ReleaseOrRelaxed>(&self, v: T, _: Ordering) {
+        let v = T::into_repr(v);
+        let a = self.as_ptr().cast::<T::Repr>();
+
+        // SAFETY:
+        // - For calling the atomic_set*() function:
+        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
+        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
+        //   - per the type invariants, the following atomic operation won't cause data races.
+        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
+        //   - atomic operations are used here.
+        unsafe {
+            if Ordering::IS_RELAXED {
+                T::Repr::atomic_set(a, v)
+            } else {
+                T::Repr::atomic_set_release(a, v)
+            }
+        };
+    }
+}
-- 
2.39.5 (Apple Git-154)

"Boqun Feng" <boqun.feng@gmail.com> writes:

[...]

> +
> +impl<T: AllowAtomic> Atomic<T> {
> +    /// Creates a new atomic.
> +    pub const fn new(v: T) -> Self {
> +        Self(Opaque::new(v))
> +    }
> +
> +    /// Creates a reference to [`Self`] from a pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` has to be a valid pointer.
> +    /// - `ptr` has to be valid for both reads and writes for the whole lifetime `'a`.
> +    /// - For the whole lifetime of '`a`, other accesses to the object cannot cause data races
> +    ///   (defined by [`LKMM`]) against atomic operations on the returned reference.

I feel the wording is a bit tangled here. How about something along the
lines of

  For the duration of `'a`, all accesses to the object must be atomic.

> +    ///
> +    /// [`LKMM`]: srctree/tools/memory-model
> +    ///
> +    /// # Examples
> +    ///
> +    /// Using [`Atomic::from_ptr()`] combined with [`Atomic::load()`] or [`Atomic::store()`] can
> +    /// achieve the same functionality as `READ_ONCE()`/`smp_load_acquire()` or
> +    /// `WRITE_ONCE()`/`smp_store_release()` in C side:
> +    ///
> +    /// ```rust
> +    /// # use kernel::types::Opaque;
> +    /// use kernel::sync::atomic::{Atomic, Relaxed, Release};
> +    ///
> +    /// // Assume there is a C struct `Foo`.
> +    /// mod cbindings {
> +    ///     #[repr(C)]
> +    ///     pub(crate) struct foo { pub(crate) a: i32, pub(crate) b: i32 }
> +    /// }
> +    ///
> +    /// let tmp = Opaque::new(cbindings::foo { a: 1, b: 2});
> +    ///
> +    /// // struct foo *foo_ptr = ..;
> +    /// let foo_ptr = tmp.get();
> +    ///
> +    /// // SAFETY: `foo_ptr` is a valid pointer, and `.a` is inbound.

Did you mean to say "in bounds"? Or what is "inbound"?

> +    /// let foo_a_ptr = unsafe { core::ptr::addr_of_mut!((*foo_ptr).a) };

This should be `&raw mut` by now, right?

> +    ///
> +    /// // a = READ_ONCE(foo_ptr->a);
> +    /// //
> +    /// // SAFETY: `foo_a_ptr` is a valid pointer for read, and all accesses on it is atomic, so no
> +    /// // data race.
> +    /// let a = unsafe { Atomic::from_ptr(foo_a_ptr) }.load(Relaxed);
> +    /// # assert_eq!(a, 1);
> +    ///
> +    /// // smp_store_release(&foo_ptr->a, 2);
> +    /// //
> +    /// // SAFETY: `foo_a_ptr` is a valid pointer for write, and all accesses on it is atomic, so no
> +    /// // data race.
> +    /// unsafe { Atomic::from_ptr(foo_a_ptr) }.store(2, Release);
> +    /// ```
> +    ///
> +    /// However, this should be only used when communicating with C side or manipulating a C struct.
> +    pub unsafe fn from_ptr<'a>(ptr: *mut T) -> &'a Self
> +    where
> +        T: Sync,
> +    {
> +        // CAST: `T` is transparent to `Atomic<T>`.
> +        // SAFETY: Per function safety requirement, `ptr` is a valid pointer and the object will
> +        // live long enough. It's safe to return a `&Atomic<T>` because function safety requirement
> +        // guarantees other accesses won't cause data races.
> +        unsafe { &*ptr.cast::<Self>() }
> +    }
> +
> +    /// Returns a pointer to the underlying atomic variable.
> +    ///
> +    /// Extra safety requirement on using the return pointer: the operations done via the pointer
> +    /// cannot cause data races defined by [`LKMM`].
> +    ///
> +    /// [`LKMM`]: srctree/tools/memory-model
> +    pub const fn as_ptr(&self) -> *mut T {
> +        self.0.get()
> +    }
> +
> +    /// Returns a mutable reference to the underlying atomic variable.
> +    ///
> +    /// This is safe because the mutable reference of the atomic variable guarantees the exclusive
> +    /// access.
> +    pub fn get_mut(&mut self) -> &mut T {
> +        // SAFETY: `self.as_ptr()` is a valid pointer to `T`, and the object has already been
> +        // initialized. `&mut self` guarantees the exclusive access, so it's safe to reborrow
> +        // mutably.
> +        unsafe { &mut *self.as_ptr() }
> +    }
> +}
> +
> +impl<T: AllowAtomic> Atomic<T>
> +where
> +    T::Repr: AtomicHasBasicOps,
> +{
> +    /// Loads the value from the atomic variable.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Simple usages:
> +    ///
> +    /// ```rust
> +    /// use kernel::sync::atomic::{Atomic, Relaxed};
> +    ///
> +    /// let x = Atomic::new(42i32);
> +    ///
> +    /// assert_eq!(42, x.load(Relaxed));
> +    ///
> +    /// let x = Atomic::new(42i64);
> +    ///
> +    /// assert_eq!(42, x.load(Relaxed));
> +    /// ```
> +    ///
> +    /// Customized new types in [`Atomic`]:
> +    ///
> +    /// ```rust
> +    /// use kernel::sync::atomic::{generic::AllowAtomic, Atomic, Relaxed};
> +    ///
> +    /// #[derive(Clone, Copy)]
> +    /// #[repr(transparent)]
> +    /// struct NewType(u32);
> +    ///
> +    /// // SAFETY: `NewType` is transparent to `u32`, which has the same size and alignment as
> +    /// // `i32`.
> +    /// unsafe impl AllowAtomic for NewType {
> +    ///     type Repr = i32;
> +    ///
> +    ///     fn into_repr(self) -> Self::Repr {
> +    ///         self.0 as i32
> +    ///     }
> +    ///
> +    ///     fn from_repr(repr: Self::Repr) -> Self {
> +    ///         NewType(repr as u32)
> +    ///     }
> +    /// }
> +    ///
> +    /// let n = Atomic::new(NewType(0));
> +    ///
> +    /// assert_eq!(0, n.load(Relaxed).0);
> +    /// ```
> +    #[doc(alias("atomic_read", "atomic64_read"))]
> +    #[inline(always)]
> +    pub fn load<Ordering: AcquireOrRelaxed>(&self, _: Ordering) -> T {
> +        let a = self.as_ptr().cast::<T::Repr>();
> +
> +        // SAFETY:
> +        // - For calling the atomic_read*() function:
> +        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,

Typo `AllocAtomic`.

> +        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
> +        //   - per the type invariants, the following atomic operation won't cause data races.
> +        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
> +        //   - atomic operations are used here.
> +        let v = unsafe {
> +            if Ordering::IS_RELAXED {
> +                T::Repr::atomic_read(a)
> +            } else {
> +                T::Repr::atomic_read_acquire(a)
> +            }
> +        };
> +
> +        T::from_repr(v)
> +    }
> +
> +    /// Stores a value to the atomic variable.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```rust
> +    /// use kernel::sync::atomic::{Atomic, Relaxed};
> +    ///
> +    /// let x = Atomic::new(42i32);
> +    ///
> +    /// assert_eq!(42, x.load(Relaxed));
> +    ///
> +    /// x.store(43, Relaxed);
> +    ///
> +    /// assert_eq!(43, x.load(Relaxed));
> +    /// ```
> +    ///
> +    #[doc(alias("atomic_set", "atomic64_set"))]
> +    #[inline(always)]
> +    pub fn store<Ordering: ReleaseOrRelaxed>(&self, v: T, _: Ordering) {
> +        let v = T::into_repr(v);
> +        let a = self.as_ptr().cast::<T::Repr>();
> +
> +        // SAFETY:
> +        // - For calling the atomic_set*() function:
> +        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,

Typo `AllocAtomic`.


Best regards,
Andreas Hindborg

On Thu, Jun 26, 2025 at 02:15:35PM +0200, Andreas Hindborg wrote:
> "Boqun Feng" <boqun.feng@gmail.com> writes:
> 
> [...]
> 
> > +
> > +impl<T: AllowAtomic> Atomic<T> {
> > +    /// Creates a new atomic.
> > +    pub const fn new(v: T) -> Self {
> > +        Self(Opaque::new(v))
> > +    }
> > +
> > +    /// Creates a reference to [`Self`] from a pointer.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// - `ptr` has to be a valid pointer.
> > +    /// - `ptr` has to be valid for both reads and writes for the whole lifetime `'a`.
> > +    /// - For the whole lifetime of '`a`, other accesses to the object cannot cause data races
> > +    ///   (defined by [`LKMM`]) against atomic operations on the returned reference.
> 
> I feel the wording is a bit tangled here. How about something along the
> lines of
> 
>   For the duration of `'a`, all accesses to the object must be atomic.
> 

Well, a non-atomic read vs an atomic read is not a data race (for both
Rust memory model and LKMM), so your proposal is overly restricted.

I can s/the whole lifetime/the duration.

> > +    ///
> > +    /// [`LKMM`]: srctree/tools/memory-model
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// Using [`Atomic::from_ptr()`] combined with [`Atomic::load()`] or [`Atomic::store()`] can
> > +    /// achieve the same functionality as `READ_ONCE()`/`smp_load_acquire()` or
> > +    /// `WRITE_ONCE()`/`smp_store_release()` in C side:
> > +    ///
> > +    /// ```rust
> > +    /// # use kernel::types::Opaque;
> > +    /// use kernel::sync::atomic::{Atomic, Relaxed, Release};
> > +    ///
> > +    /// // Assume there is a C struct `Foo`.
> > +    /// mod cbindings {
> > +    ///     #[repr(C)]
> > +    ///     pub(crate) struct foo { pub(crate) a: i32, pub(crate) b: i32 }
> > +    /// }
> > +    ///
> > +    /// let tmp = Opaque::new(cbindings::foo { a: 1, b: 2});
> > +    ///
> > +    /// // struct foo *foo_ptr = ..;
> > +    /// let foo_ptr = tmp.get();
> > +    ///
> > +    /// // SAFETY: `foo_ptr` is a valid pointer, and `.a` is inbound.
> 
> Did you mean to say "in bounds"? Or what is "inbound"?
> 

I think I meant to say "inbounds", fixed.

> > +    /// let foo_a_ptr = unsafe { core::ptr::addr_of_mut!((*foo_ptr).a) };
> 
> This should be `&raw mut` by now, right?
> 

Right.

[...]
> 
> Typo `AllocAtomic`.
> 

All fixed! Thanks!

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
>

"Boqun Feng" <boqun.feng@gmail.com> writes:

> On Thu, Jun 26, 2025 at 02:15:35PM +0200, Andreas Hindborg wrote:
>> "Boqun Feng" <boqun.feng@gmail.com> writes:
>>
>> [...]
>>
>> > +
>> > +impl<T: AllowAtomic> Atomic<T> {
>> > +    /// Creates a new atomic.
>> > +    pub const fn new(v: T) -> Self {
>> > +        Self(Opaque::new(v))
>> > +    }
>> > +
>> > +    /// Creates a reference to [`Self`] from a pointer.
>> > +    ///
>> > +    /// # Safety
>> > +    ///
>> > +    /// - `ptr` has to be a valid pointer.
>> > +    /// - `ptr` has to be valid for both reads and writes for the whole lifetime `'a`.
>> > +    /// - For the whole lifetime of '`a`, other accesses to the object cannot cause data races
>> > +    ///   (defined by [`LKMM`]) against atomic operations on the returned reference.
>>
>> I feel the wording is a bit tangled here. How about something along the
>> lines of
>>
>>   For the duration of `'a`, all accesses to the object must be atomic.
>>
>
> Well, a non-atomic read vs an atomic read is not a data race (for both
> Rust memory model and LKMM), so your proposal is overly restricted.

OK, my mistake then. I thought mixing marked and plain accesses would be
considered a race. I got hat from
`tools/memory-model/Documentation/explanation.txt`:

    A "data race"
    occurs when there are two memory accesses such that:

    1.	they access the same location,

    2.	at least one of them is a store,

    3.	at least one of them is plain,

    4.	they occur on different CPUs (or in different threads on the
      same CPU), and

    5.	they execute concurrently.

I did not study all that documentation, so I might be missing a point or
two.


Best regards,
Andreas Hindborg

On Mon, Jun 30, 2025 at 11:52:35AM +0200, Andreas Hindborg wrote:
> "Boqun Feng" <boqun.feng@gmail.com> writes:
> > Well, a non-atomic read vs an atomic read is not a data race (for both
> > Rust memory model and LKMM), so your proposal is overly restricted.
> 
> OK, my mistake then. I thought mixing marked and plain accesses would be
> considered a race. I got hat from
> `tools/memory-model/Documentation/explanation.txt`:
> 
>     A "data race"
>     occurs when there are two memory accesses such that:
> 
>     1.	they access the same location,
> 
>     2.	at least one of them is a store,
> 
>     3.	at least one of them is plain,
> 
>     4.	they occur on different CPUs (or in different threads on the
>       same CPU), and
> 
>     5.	they execute concurrently.
> 
> I did not study all that documentation, so I might be missing a point or
> two.

You missed point 2 above: at least one of the accesses has to be a 
store.  When you're looking at a non-atomic read vs. an atomic read, 
both of them are loads and so it isn't a data race.

Alan

"Alan Stern" <stern@rowland.harvard.edu> writes:

> On Mon, Jun 30, 2025 at 11:52:35AM +0200, Andreas Hindborg wrote:
>> "Boqun Feng" <boqun.feng@gmail.com> writes:
>> > Well, a non-atomic read vs an atomic read is not a data race (for both
>> > Rust memory model and LKMM), so your proposal is overly restricted.
>>
>> OK, my mistake then. I thought mixing marked and plain accesses would be
>> considered a race. I got hat from
>> `tools/memory-model/Documentation/explanation.txt`:
>>
>>     A "data race"
>>     occurs when there are two memory accesses such that:
>>
>>     1.	they access the same location,
>>
>>     2.	at least one of them is a store,
>>
>>     3.	at least one of them is plain,
>>
>>     4.	they occur on different CPUs (or in different threads on the
>>       same CPU), and
>>
>>     5.	they execute concurrently.
>>
>> I did not study all that documentation, so I might be missing a point or
>> two.
>
> You missed point 2 above: at least one of the accesses has to be a
> store.  When you're looking at a non-atomic read vs. an atomic read,
> both of them are loads and so it isn't a data race.

Ah, right. I was missing the entire point made by Boqun. Thanks for
clarifying.

Since what constitutes a race might not be immediately clear to users
(like me), can we include the section above in the safety comment,
rather than deferring to LKMM docs?


Best regards,
Andreas Hindborg

On Tue, Jul 01, 2025 at 10:54:09AM +0200, Andreas Hindborg wrote:
> "Alan Stern" <stern@rowland.harvard.edu> writes:
> 
> > On Mon, Jun 30, 2025 at 11:52:35AM +0200, Andreas Hindborg wrote:
> >> "Boqun Feng" <boqun.feng@gmail.com> writes:
> >> > Well, a non-atomic read vs an atomic read is not a data race (for both
> >> > Rust memory model and LKMM), so your proposal is overly restricted.
> >>
> >> OK, my mistake then. I thought mixing marked and plain accesses would be
> >> considered a race. I got hat from
> >> `tools/memory-model/Documentation/explanation.txt`:
> >>
> >>     A "data race"
> >>     occurs when there are two memory accesses such that:
> >>
> >>     1.	they access the same location,
> >>
> >>     2.	at least one of them is a store,
> >>
> >>     3.	at least one of them is plain,
> >>
> >>     4.	they occur on different CPUs (or in different threads on the
> >>       same CPU), and
> >>
> >>     5.	they execute concurrently.
> >>
> >> I did not study all that documentation, so I might be missing a point or
> >> two.
> >
> > You missed point 2 above: at least one of the accesses has to be a
> > store.  When you're looking at a non-atomic read vs. an atomic read,
> > both of them are loads and so it isn't a data race.
> 
> Ah, right. I was missing the entire point made by Boqun. Thanks for
> clarifying.
> 
> Since what constitutes a race might not be immediately clear to users
> (like me), can we include the section above in the safety comment,
> rather than deferring to LKMM docs?
> 

Still, I don't think it's a good idea. For a few reasons:

1) Maintaining multiple sources of truth is painful and risky, it's
   going to be further confusing if users feel LKMM and the function
   safety requirement conflict with each other.

2) Human language is not the best tool to describe memory model, that's
   why we use herd to describe and use memory model. Trying to describe
   the memory model in comments rather than referring to the formal
   model is a way backwards.

3) I believed the reason we got our discussion here is because you tried
   to improve the comment of `from_ptr()`, and I do appreciate that
   effort. And I think we should continue in that direction instead of
   pulling the whole "what are data race conditions" into picture. So
   how about we clearly call out that it'll be safe if other accesses
   are atomic, which should be the most cases:

   /// - For the duration of `'a`, other accesses to the object cannot cause data races
   ///   (defined by [`LKMM`]) against atomic operations on the returned reference. Note
   ///   that if all other accesses are atomic, then this safety requirement is trivially
   ///   fulfilled.

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
>

"Boqun Feng" <boqun.feng@gmail.com> writes:

> On Tue, Jul 01, 2025 at 10:54:09AM +0200, Andreas Hindborg wrote:
>> "Alan Stern" <stern@rowland.harvard.edu> writes:
>>
>> > On Mon, Jun 30, 2025 at 11:52:35AM +0200, Andreas Hindborg wrote:
>> >> "Boqun Feng" <boqun.feng@gmail.com> writes:
>> >> > Well, a non-atomic read vs an atomic read is not a data race (for both
>> >> > Rust memory model and LKMM), so your proposal is overly restricted.
>> >>
>> >> OK, my mistake then. I thought mixing marked and plain accesses would be
>> >> considered a race. I got hat from
>> >> `tools/memory-model/Documentation/explanation.txt`:
>> >>
>> >>     A "data race"
>> >>     occurs when there are two memory accesses such that:
>> >>
>> >>     1.	they access the same location,
>> >>
>> >>     2.	at least one of them is a store,
>> >>
>> >>     3.	at least one of them is plain,
>> >>
>> >>     4.	they occur on different CPUs (or in different threads on the
>> >>       same CPU), and
>> >>
>> >>     5.	they execute concurrently.
>> >>
>> >> I did not study all that documentation, so I might be missing a point or
>> >> two.
>> >
>> > You missed point 2 above: at least one of the accesses has to be a
>> > store.  When you're looking at a non-atomic read vs. an atomic read,
>> > both of them are loads and so it isn't a data race.
>>
>> Ah, right. I was missing the entire point made by Boqun. Thanks for
>> clarifying.
>>
>> Since what constitutes a race might not be immediately clear to users
>> (like me), can we include the section above in the safety comment,
>> rather than deferring to LKMM docs?
>>
>
> Still, I don't think it's a good idea. For a few reasons:
>
> 1) Maintaining multiple sources of truth is painful and risky, it's
>    going to be further confusing if users feel LKMM and the function
>    safety requirement conflict with each other.

I would agree.

>
> 2) Human language is not the best tool to describe memory model, that's
>    why we use herd to describe and use memory model. Trying to describe
>    the memory model in comments rather than referring to the formal
>    model is a way backwards.

I do not agree with this. I read human language much better than formal logic.

>
> 3) I believed the reason we got our discussion here is because you tried
>    to improve the comment of `from_ptr()`, and I do appreciate that
>    effort. And I think we should continue in that direction instead of
>    pulling the whole "what are data race conditions" into picture.

Yes, absolutely.

> So
>    how about we clearly call out that it'll be safe if other accesses
>    are atomic, which should be the most cases:
>
>    /// - For the duration of `'a`, other accesses to the object cannot cause data races
>    ///   (defined by [`LKMM`]) against atomic operations on the returned reference. Note
>    ///   that if all other accesses are atomic, then this safety requirement is trivially
>    ///   fulfilled.

Sounds good to me, thanks!


Best regards,
Andreas Hindborg

On Wed, 18 Jun 2025 09:49:28 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> To provide using LKMM atomics for Rust code, a generic `Atomic<T>` is
> added, currently `T` needs to be Send + Copy because these are the
> straightforward usages and all basic types support this. The trait
> `AllowAtomic` should be only implemented inside atomic mod until the
> generic atomic framework is mature enough (unless the implementer is a
> `#[repr(transparent)]` new type).
> 
> `AtomicImpl` types are automatically `AllowAtomic`, and so far only
> basic operations load() and store() are introduced.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  rust/kernel/sync/atomic.rs         |   2 +
>  rust/kernel/sync/atomic/generic.rs | 258 +++++++++++++++++++++++++++++
>  2 files changed, 260 insertions(+)
>  create mode 100644 rust/kernel/sync/atomic/generic.rs
> 
> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> index 9fe5d81fc2a9..a01e44eec380 100644
> --- a/rust/kernel/sync/atomic.rs
> +++ b/rust/kernel/sync/atomic.rs
> @@ -16,7 +16,9 @@
>  //!
>  //! [`LKMM`]: srctree/tools/memory-mode/
>  
> +pub mod generic;
>  pub mod ops;
>  pub mod ordering;
>  
> +pub use generic::Atomic;
>  pub use ordering::{Acquire, Full, Relaxed, Release};
> diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
> new file mode 100644
> index 000000000000..73c26f9cf6b8
> --- /dev/null
> +++ b/rust/kernel/sync/atomic/generic.rs
> @@ -0,0 +1,258 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Generic atomic primitives.
> +
> +use super::ops::*;
> +use super::ordering::*;
> +use crate::types::Opaque;
> +
> +/// A generic atomic variable.
> +///
> +/// `T` must impl [`AllowAtomic`], that is, an [`AtomicImpl`] has to be chosen.
> +///
> +/// # Invariants
> +///
> +/// Doing an atomic operation while holding a reference of [`Self`] won't cause a data race, this
> +/// is guaranteed by the safety requirement of [`Self::from_ptr`] and the extra safety requirement
> +/// of the usage on pointers returned by [`Self::as_ptr`].
> +#[repr(transparent)]
> +pub struct Atomic<T: AllowAtomic>(Opaque<T>);

This should store `Opaque<T::Repr>` instead.

The implementation below essentially assumes that this is
`Opaque<T::Repr>`:
* atomic ops cast this to `*mut T::Repr`
* load/store operates on `T::Repr` then converts to `T` with
  `T::from_repr`/`T::into_repr`.

Note tha the transparent new types restriction on `AllowAtomic` is not
sufficient for this, as I can define

#[repr(transparent)]
struct MyWeirdI32(pub i32);

impl AllowAtomic for MyWeirdI32 {
    type Repr = i32;

    fn into_repr(self) -> Self::Repr {
        !self
    }

    fn from_repr(repr: Self::Repr) -> Self {
        !self
    }
}

Then `Atomic<MyWeirdI32>::new(MyWeirdI32(0)).load(Relaxed)` will give me
`MyWeirdI32(-1)`.

Alternatively, we should remove `into_repr`/`from_repr` and always cast
instead. In such case, `AllowAtomic` needs to have the transmutability
as a safety precondition.

> +
> +// SAFETY: `Atomic<T>` is safe to share among execution contexts because all accesses are atomic.
> +unsafe impl<T: AllowAtomic> Sync for Atomic<T> {}
> +
> +/// Atomics that support basic atomic operations.
> +///
> +/// TODO: Currently the [`AllowAtomic`] types are restricted within basic integer types (and their
> +/// transparent new types). In the future, we could extend the scope to more data types when there
> +/// is a clear and meaningful usage, but for now, [`AllowAtomic`] should only be implemented inside
> +/// atomic mod for the restricted types mentioned above.
> +///
> +/// # Safety
> +///
> +/// [`Self`] must have the same size and alignment as [`Self::Repr`].
> +pub unsafe trait AllowAtomic: Sized + Send + Copy {
> +    /// The backing atomic implementation type.
> +    type Repr: AtomicImpl;
> +
> +    /// Converts into a [`Self::Repr`].
> +    fn into_repr(self) -> Self::Repr;
> +
> +    /// Converts from a [`Self::Repr`].
> +    fn from_repr(repr: Self::Repr) -> Self;
> +}
> +
> +// An `AtomicImpl` is automatically an `AllowAtomic`.
> +//
> +// SAFETY: `T::Repr` is `Self` (i.e. `T`), so they have the same size and alignment.
> +unsafe impl<T: AtomicImpl> AllowAtomic for T {
> +    type Repr = Self;
> +
> +    fn into_repr(self) -> Self::Repr {
> +        self
> +    }
> +
> +    fn from_repr(repr: Self::Repr) -> Self {
> +        repr
> +    }
> +}
> +
> +impl<T: AllowAtomic> Atomic<T> {
> +    /// Creates a new atomic.
> +    pub const fn new(v: T) -> Self {
> +        Self(Opaque::new(v))
> +    }
> +
> +    /// Creates a reference to [`Self`] from a pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` has to be a valid pointer.
> +    /// - `ptr` has to be valid for both reads and writes for the whole lifetime `'a`.
> +    /// - For the whole lifetime of '`a`, other accesses to the object cannot cause data races
> +    ///   (defined by [`LKMM`]) against atomic operations on the returned reference.
> +    ///
> +    /// [`LKMM`]: srctree/tools/memory-model
> +    ///
> +    /// # Examples
> +    ///
> +    /// Using [`Atomic::from_ptr()`] combined with [`Atomic::load()`] or [`Atomic::store()`] can
> +    /// achieve the same functionality as `READ_ONCE()`/`smp_load_acquire()` or
> +    /// `WRITE_ONCE()`/`smp_store_release()` in C side:
> +    ///
> +    /// ```rust
> +    /// # use kernel::types::Opaque;
> +    /// use kernel::sync::atomic::{Atomic, Relaxed, Release};
> +    ///
> +    /// // Assume there is a C struct `Foo`.
> +    /// mod cbindings {
> +    ///     #[repr(C)]
> +    ///     pub(crate) struct foo { pub(crate) a: i32, pub(crate) b: i32 }
> +    /// }
> +    ///
> +    /// let tmp = Opaque::new(cbindings::foo { a: 1, b: 2});
> +    ///
> +    /// // struct foo *foo_ptr = ..;
> +    /// let foo_ptr = tmp.get();
> +    ///
> +    /// // SAFETY: `foo_ptr` is a valid pointer, and `.a` is inbound.
> +    /// let foo_a_ptr = unsafe { core::ptr::addr_of_mut!((*foo_ptr).a) };
> +    ///
> +    /// // a = READ_ONCE(foo_ptr->a);
> +    /// //
> +    /// // SAFETY: `foo_a_ptr` is a valid pointer for read, and all accesses on it is atomic, so no
> +    /// // data race.
> +    /// let a = unsafe { Atomic::from_ptr(foo_a_ptr) }.load(Relaxed);
> +    /// # assert_eq!(a, 1);
> +    ///
> +    /// // smp_store_release(&foo_ptr->a, 2);
> +    /// //
> +    /// // SAFETY: `foo_a_ptr` is a valid pointer for write, and all accesses on it is atomic, so no
> +    /// // data race.
> +    /// unsafe { Atomic::from_ptr(foo_a_ptr) }.store(2, Release);
> +    /// ```
> +    ///
> +    /// However, this should be only used when communicating with C side or manipulating a C struct.
> +    pub unsafe fn from_ptr<'a>(ptr: *mut T) -> &'a Self
> +    where
> +        T: Sync,
> +    {
> +        // CAST: `T` is transparent to `Atomic<T>`.
> +        // SAFETY: Per function safety requirement, `ptr` is a valid pointer and the object will
> +        // live long enough. It's safe to return a `&Atomic<T>` because function safety requirement
> +        // guarantees other accesses won't cause data races.
> +        unsafe { &*ptr.cast::<Self>() }
> +    }
> +
> +    /// Returns a pointer to the underlying atomic variable.
> +    ///
> +    /// Extra safety requirement on using the return pointer: the operations done via the pointer
> +    /// cannot cause data races defined by [`LKMM`].
> +    ///
> +    /// [`LKMM`]: srctree/tools/memory-model
> +    pub const fn as_ptr(&self) -> *mut T {
> +        self.0.get()
> +    }
> +
> +    /// Returns a mutable reference to the underlying atomic variable.
> +    ///
> +    /// This is safe because the mutable reference of the atomic variable guarantees the exclusive
> +    /// access.
> +    pub fn get_mut(&mut self) -> &mut T {
> +        // SAFETY: `self.as_ptr()` is a valid pointer to `T`, and the object has already been
> +        // initialized. `&mut self` guarantees the exclusive access, so it's safe to reborrow
> +        // mutably.
> +        unsafe { &mut *self.as_ptr() }
> +    }
> +}
> +
> +impl<T: AllowAtomic> Atomic<T>
> +where
> +    T::Repr: AtomicHasBasicOps,
> +{
> +    /// Loads the value from the atomic variable.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Simple usages:
> +    ///
> +    /// ```rust
> +    /// use kernel::sync::atomic::{Atomic, Relaxed};
> +    ///
> +    /// let x = Atomic::new(42i32);
> +    ///
> +    /// assert_eq!(42, x.load(Relaxed));
> +    ///
> +    /// let x = Atomic::new(42i64);
> +    ///
> +    /// assert_eq!(42, x.load(Relaxed));
> +    /// ```
> +    ///
> +    /// Customized new types in [`Atomic`]:
> +    ///
> +    /// ```rust
> +    /// use kernel::sync::atomic::{generic::AllowAtomic, Atomic, Relaxed};
> +    ///
> +    /// #[derive(Clone, Copy)]
> +    /// #[repr(transparent)]
> +    /// struct NewType(u32);
> +    ///
> +    /// // SAFETY: `NewType` is transparent to `u32`, which has the same size and alignment as
> +    /// // `i32`.
> +    /// unsafe impl AllowAtomic for NewType {
> +    ///     type Repr = i32;
> +    ///
> +    ///     fn into_repr(self) -> Self::Repr {
> +    ///         self.0 as i32
> +    ///     }
> +    ///
> +    ///     fn from_repr(repr: Self::Repr) -> Self {
> +    ///         NewType(repr as u32)
> +    ///     }
> +    /// }
> +    ///
> +    /// let n = Atomic::new(NewType(0));
> +    ///
> +    /// assert_eq!(0, n.load(Relaxed).0);
> +    /// ```
> +    #[doc(alias("atomic_read", "atomic64_read"))]
> +    #[inline(always)]
> +    pub fn load<Ordering: AcquireOrRelaxed>(&self, _: Ordering) -> T {
> +        let a = self.as_ptr().cast::<T::Repr>();
> +
> +        // SAFETY:
> +        // - For calling the atomic_read*() function:
> +        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
> +        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
> +        //   - per the type invariants, the following atomic operation won't cause data races.
> +        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
> +        //   - atomic operations are used here.
> +        let v = unsafe {
> +            if Ordering::IS_RELAXED {
> +                T::Repr::atomic_read(a)
> +            } else {
> +                T::Repr::atomic_read_acquire(a)
> +            }

This can be

match Ordering::TYPE {
    OrderingType::Relaxed => T::Repr::atomic_read(a),
    _ => T::Repr::atomic_read_acquire(a),
}

Or, also add the match arm for acquire and add a match
arm for `_ => build_error!()`.

> +        };
> +
> +        T::from_repr(v)
> +    }
> +
> +    /// Stores a value to the atomic variable.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```rust
> +    /// use kernel::sync::atomic::{Atomic, Relaxed};
> +    ///
> +    /// let x = Atomic::new(42i32);
> +    ///
> +    /// assert_eq!(42, x.load(Relaxed));
> +    ///
> +    /// x.store(43, Relaxed);
> +    ///
> +    /// assert_eq!(43, x.load(Relaxed));
> +    /// ```
> +    ///
> +    #[doc(alias("atomic_set", "atomic64_set"))]
> +    #[inline(always)]
> +    pub fn store<Ordering: ReleaseOrRelaxed>(&self, v: T, _: Ordering) {
> +        let v = T::into_repr(v);
> +        let a = self.as_ptr().cast::<T::Repr>();
> +
> +        // SAFETY:
> +        // - For calling the atomic_set*() function:
> +        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
> +        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
> +        //   - per the type invariants, the following atomic operation won't cause data races.
> +        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
> +        //   - atomic operations are used here.
> +        unsafe {
> +            if Ordering::IS_RELAXED {
> +                T::Repr::atomic_set(a, v)
> +            } else {
> +                T::Repr::atomic_set_release(a, v)
> +            }
> +        };
> +    }
> +}

On Sat, Jun 21, 2025 at 12:32:12PM +0100, Gary Guo wrote:
[...]
> > +#[repr(transparent)]
> > +pub struct Atomic<T: AllowAtomic>(Opaque<T>);
> 
> This should store `Opaque<T::Repr>` instead.
> 

"should" is a strong word ;-) If we still use `into_repr`/`from_repr`
it's a bit impossible, because Atomic::new() wants to be a const
function, so it requires const_trait_impl I believe.

If we require transmutability as a safety requirement for `AllowAtomic`,
then either `T` or `T::Repr` is fine.

> The implementation below essentially assumes that this is
> `Opaque<T::Repr>`:
> * atomic ops cast this to `*mut T::Repr`
> * load/store operates on `T::Repr` then converts to `T` with
>   `T::from_repr`/`T::into_repr`.
> 

Note that we only require one direction of strong transmutability, that
is: for every `T`, it must be able to safe transmute to a `T::Repr`, for
`T::Repr` -> `T` transmutation, only if it's a result of a `transmute<T,
T::Repr>()`. This is mostly due to potential support for unit-only enum.
E.g. using an atomic variable to represent a finite state.

> Note tha the transparent new types restriction on `AllowAtomic` is not
> sufficient for this, as I can define
> 

Nice catch! I do agree we should disallow `MyWeirdI32`, and I also agree
that we should put transmutability as safety requirement for
`AllowAtomic`. However, I would suggest we still keep
`into_repr`/`from_repr`, and require the implementation to make them
provide the same results as transmute(), as a correctness precondition
(instead of a safety precondition), in other words, you can still write
a `MyWeirdI32`, and it won't cause safety issues, but it'll be
incorrect.

The reason why I think we should keep `into_repr`/`from_repr` but add
a correctness precondition is that they are easily to implement as safe
code for basic types, so it'll be better than a transmute() call. Also
considering `Atomic<*mut T>`, would transmuting between integers and
pointers act the same as expose_provenance() and
from_exposed_provenance()?


So something like this for `AllowAtomic`, implementation-wise, no need
to change.

    /// Atomics that support basic atomic operations.
    ///
    /// Implementers must guarantee that `into_repr()` and `from_repr()` provide the same results as
    /// transmute between [`Self`] and [`Self::Repr`].
    ///
    /// TODO: Currently the [`AllowAtomic`] types are restricted within basic integer types (and their
    /// transparent new types). In the future, we could extend the scope to more data types when there
    /// is a clear and meaningful usage, but for now, [`AllowAtomic`] should only be implemented inside
    /// atomic mod for the restricted types mentioned above.
    ///
    /// # Safety
    ///
    /// - [`Self`] must have the same size and alignment as [`Self::Repr`].
    /// - Any value of [`Self`] must be safe to [`transmute()`] to a [`Self::Repr`], this also means
    ///   that a pointer to [`Self`] can be treated as a pointer to [`Self::Repr`].
    /// - If a value of [`Self::Repr`] is a result a [`transmute()`] from a [`Self`], it must be safe
    ///   to [`transmute()`] the value back to a [`Self`].
    ///   that a pointer to [`Self`] can be treated as a pointer to [`Self::Repr`].
    /// - The implementer must guarantee it's safe to transfer ownership from one execution context to
    ///   another, this means it has to be a [`Send`], but because `*mut T` is not [`Send`] and that's
    ///   the basic type needs to support atomic operations, so this safety requirement is added to
    ///   [`AllowAtomic`] trait. This safety requirement is automatically satisfied if the type is a
    ///   [`Send`].
    ///
    /// [`transmute()`]: core::mem::transmute
    pub unsafe trait AllowAtomic: Sized + Copy {

Thoughts?

Regards,
Boqun

> #[repr(transparent)]
> struct MyWeirdI32(pub i32);
> 
> impl AllowAtomic for MyWeirdI32 {
>     type Repr = i32;
> 
>     fn into_repr(self) -> Self::Repr {
>         !self
>     }
> 
>     fn from_repr(repr: Self::Repr) -> Self {
>         !self
>     }
> }
> 
> Then `Atomic<MyWeirdI32>::new(MyWeirdI32(0)).load(Relaxed)` will give me
> `MyWeirdI32(-1)`.
> 
> Alternatively, we should remove `into_repr`/`from_repr` and always cast
> instead. In such case, `AllowAtomic` needs to have the transmutability
> as a safety precondition.
> 
[...]

On Sun, 22 Jun 2025 22:19:44 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Sat, Jun 21, 2025 at 12:32:12PM +0100, Gary Guo wrote:
> [...]
> > > +#[repr(transparent)]
> > > +pub struct Atomic<T: AllowAtomic>(Opaque<T>);  
> > 
> > This should store `Opaque<T::Repr>` instead.
> >   
> 
> "should" is a strong word ;-) If we still use `into_repr`/`from_repr`
> it's a bit impossible, because Atomic::new() wants to be a const
> function, so it requires const_trait_impl I believe.
> 
> If we require transmutability as a safety requirement for `AllowAtomic`,
> then either `T` or `T::Repr` is fine.
> 
> > The implementation below essentially assumes that this is
> > `Opaque<T::Repr>`:
> > * atomic ops cast this to `*mut T::Repr`
> > * load/store operates on `T::Repr` then converts to `T` with
> >   `T::from_repr`/`T::into_repr`.
> >   
> 
> Note that we only require one direction of strong transmutability, that
> is: for every `T`, it must be able to safe transmute to a `T::Repr`, for
> `T::Repr` -> `T` transmutation, only if it's a result of a `transmute<T,
> T::Repr>()`. This is mostly due to potential support for unit-only enum.  
> E.g. using an atomic variable to represent a finite state.
> 
> > Note tha the transparent new types restriction on `AllowAtomic` is not
> > sufficient for this, as I can define
> >   
> 
> Nice catch! I do agree we should disallow `MyWeirdI32`, and I also agree
> that we should put transmutability as safety requirement for
> `AllowAtomic`. However, I would suggest we still keep
> `into_repr`/`from_repr`, and require the implementation to make them
> provide the same results as transmute(), as a correctness precondition
> (instead of a safety precondition), in other words, you can still write
> a `MyWeirdI32`, and it won't cause safety issues, but it'll be
> incorrect.
> 
> The reason why I think we should keep `into_repr`/`from_repr` but add
> a correctness precondition is that they are easily to implement as safe
> code for basic types, so it'll be better than a transmute() call. Also
> considering `Atomic<*mut T>`, would transmuting between integers and
> pointers act the same as expose_provenance() and
> from_exposed_provenance()?

Okay, this is more problematic than I thought then. For pointers, you
cannot just transmute between from pointers to usize (which is its
Repr):
* Transmuting from pointer to usize discards provenance
* Transmuting from usize to pointer gives invalid provenance

We want neither behaviour, so we must store `usize` directly and
always call into repr functions.

To make things cost I guess you would need an extra trait to indicate
that transmuting is fine.

Best,
Gary

On Mon, Jun 23, 2025 at 07:30:19PM +0100, Gary Guo wrote:
> On Sun, 22 Jun 2025 22:19:44 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> > On Sat, Jun 21, 2025 at 12:32:12PM +0100, Gary Guo wrote:
> > [...]
> > > > +#[repr(transparent)]
> > > > +pub struct Atomic<T: AllowAtomic>(Opaque<T>);  
> > > 
> > > This should store `Opaque<T::Repr>` instead.
> > >   
> > 
> > "should" is a strong word ;-) If we still use `into_repr`/`from_repr`
> > it's a bit impossible, because Atomic::new() wants to be a const
> > function, so it requires const_trait_impl I believe.
> > 
> > If we require transmutability as a safety requirement for `AllowAtomic`,
> > then either `T` or `T::Repr` is fine.
> > 
> > > The implementation below essentially assumes that this is
> > > `Opaque<T::Repr>`:
> > > * atomic ops cast this to `*mut T::Repr`
> > > * load/store operates on `T::Repr` then converts to `T` with
> > >   `T::from_repr`/`T::into_repr`.
> > >   
> > 
> > Note that we only require one direction of strong transmutability, that
> > is: for every `T`, it must be able to safe transmute to a `T::Repr`, for
> > `T::Repr` -> `T` transmutation, only if it's a result of a `transmute<T,
> > T::Repr>()`. This is mostly due to potential support for unit-only enum.  
> > E.g. using an atomic variable to represent a finite state.
> > 
> > > Note tha the transparent new types restriction on `AllowAtomic` is not
> > > sufficient for this, as I can define
> > >   
> > 
> > Nice catch! I do agree we should disallow `MyWeirdI32`, and I also agree
> > that we should put transmutability as safety requirement for
> > `AllowAtomic`. However, I would suggest we still keep
> > `into_repr`/`from_repr`, and require the implementation to make them
> > provide the same results as transmute(), as a correctness precondition
> > (instead of a safety precondition), in other words, you can still write
> > a `MyWeirdI32`, and it won't cause safety issues, but it'll be
> > incorrect.
> > 
> > The reason why I think we should keep `into_repr`/`from_repr` but add
> > a correctness precondition is that they are easily to implement as safe
> > code for basic types, so it'll be better than a transmute() call. Also
> > considering `Atomic<*mut T>`, would transmuting between integers and
> > pointers act the same as expose_provenance() and
> > from_exposed_provenance()?
> 
> Okay, this is more problematic than I thought then. For pointers, you

Welcome to my nightmare ;-)

> cannot just transmute between from pointers to usize (which is its
> Repr):
> * Transmuting from pointer to usize discards provenance
> * Transmuting from usize to pointer gives invalid provenance
> 
> We want neither behaviour, so we must store `usize` directly and
> always call into repr functions.
> 

If we store `usize`, how can we support the `get_mut()` then? E.g.

    static V: i32 = 32;

    let mut x = Atomic::new(&V as *const i32 as *mut i32);
    // ^ assume we expose_provenance() in new().

    let ptr: &mut *mut i32 = x.get_mut(); // which is `&mut self.0.get()`.

    let ptr_val = *ptr; // Does `ptr_val` have the proper provenance?

> To make things cost I guess you would need an extra trait to indicate
> that transmuting is fine.

Could you maybe provide an example?

Regards,
Boqun

> 
> Best,
> Gary

On Mon, Jun 23, 2025 at 12:09:21PM -0700, Boqun Feng wrote:
> On Mon, Jun 23, 2025 at 07:30:19PM +0100, Gary Guo wrote:
> > On Sun, 22 Jun 2025 22:19:44 -0700
> > Boqun Feng <boqun.feng@gmail.com> wrote:
> > 
> > > On Sat, Jun 21, 2025 at 12:32:12PM +0100, Gary Guo wrote:
> > > [...]
> > > > > +#[repr(transparent)]
> > > > > +pub struct Atomic<T: AllowAtomic>(Opaque<T>);  
> > > > 
> > > > This should store `Opaque<T::Repr>` instead.
> > > >   
> > > 
> > > "should" is a strong word ;-) If we still use `into_repr`/`from_repr`
> > > it's a bit impossible, because Atomic::new() wants to be a const
> > > function, so it requires const_trait_impl I believe.
> > > 
> > > If we require transmutability as a safety requirement for `AllowAtomic`,
> > > then either `T` or `T::Repr` is fine.
> > > 
> > > > The implementation below essentially assumes that this is
> > > > `Opaque<T::Repr>`:
> > > > * atomic ops cast this to `*mut T::Repr`
> > > > * load/store operates on `T::Repr` then converts to `T` with
> > > >   `T::from_repr`/`T::into_repr`.
> > > >   
> > > 
> > > Note that we only require one direction of strong transmutability, that
> > > is: for every `T`, it must be able to safe transmute to a `T::Repr`, for
> > > `T::Repr` -> `T` transmutation, only if it's a result of a `transmute<T,
> > > T::Repr>()`. This is mostly due to potential support for unit-only enum.  
> > > E.g. using an atomic variable to represent a finite state.
> > > 
> > > > Note tha the transparent new types restriction on `AllowAtomic` is not
> > > > sufficient for this, as I can define
> > > >   
> > > 
> > > Nice catch! I do agree we should disallow `MyWeirdI32`, and I also agree
> > > that we should put transmutability as safety requirement for
> > > `AllowAtomic`. However, I would suggest we still keep
> > > `into_repr`/`from_repr`, and require the implementation to make them
> > > provide the same results as transmute(), as a correctness precondition
> > > (instead of a safety precondition), in other words, you can still write
> > > a `MyWeirdI32`, and it won't cause safety issues, but it'll be
> > > incorrect.
> > > 
> > > The reason why I think we should keep `into_repr`/`from_repr` but add
> > > a correctness precondition is that they are easily to implement as safe
> > > code for basic types, so it'll be better than a transmute() call. Also
> > > considering `Atomic<*mut T>`, would transmuting between integers and
> > > pointers act the same as expose_provenance() and
> > > from_exposed_provenance()?
> > 
> > Okay, this is more problematic than I thought then. For pointers, you
> 
> Welcome to my nightmare ;-)
> 
> > cannot just transmute between from pointers to usize (which is its
> > Repr):
> > * Transmuting from pointer to usize discards provenance
> > * Transmuting from usize to pointer gives invalid provenance
> > 
> > We want neither behaviour, so we must store `usize` directly and
> > always call into repr functions.
> > 
> 
> If we store `usize`, how can we support the `get_mut()` then? E.g.
> 
>     static V: i32 = 32;
> 
>     let mut x = Atomic::new(&V as *const i32 as *mut i32);
>     // ^ assume we expose_provenance() in new().
> 
>     let ptr: &mut *mut i32 = x.get_mut(); // which is `&mut self.0.get()`.
> 
>     let ptr_val = *ptr; // Does `ptr_val` have the proper provenance?
> 

There are a few off-list discussions, and I've been trying some
experiment myself, here are a few points/concepts that will help future
discussion or documentation, so I put it down here:

* Round-trip transmutability (thank Benno for the name!).

  We realize this should be a safety requirement of `AllowAtomic` type
  (i.e. the type that can be put in a Atomic<T>). What it means is:

  - If `T: AllowAtomic`, transmute() from `T` to `T::Repr` is always
    safe and
  - if a value of `T::Repr` is a result of transmute() from `T` to
    `T::Repr`, then `transmute()` for that value to `T` is also safe.

  This essentially means a valid bit pattern of `T: AllowAtomic` has to
  be a valid bit pattern of `T::Repr`.

  This is needed because the atomic framework operates on `T::Repr` to
  implement atomic operations on `T`.

  Note that this is more relaxed than bi-direct transmutability (i.e.
  transmute() between `T` and `T::Repr`) because we want to support
  atomic type over unit-only enums:

    #[repr(i32)]
    pub enum State {
        Init = 0,
	Working = 1,
	Done = 2,
    }

  This should be really helpful to support atomics as states, for
  example:

    https://lore.kernel.org/rust-for-linux/20250702-module-params-v3-v14-1-5b1cc32311af@kernel.org/

* transmute()-equivalent from_repr() and into_repr().

  (This is not a safety requirement)

  from_repr() and into_repr(), if exist, should behave like transmute()
  on the bit pattern of the results, in other words, bit patterns of `T`
  or `T::Repr` should stay the same before and after these operations.

  Of course if we remove them and replace with transmute(), same result.

  This reflects the fact that customized atomic types should store
  unmodified bit patterns into atomic variables, and this make atomic
  operations don't have weird behavior [1] when combined with new(),
  from_ptr() and get_mut().

* Provenance preservation.

  (This is not a safety requirement for Atomic itself)

  For a `Atomic<*mut T>`, it should preserve the provenance of the
  pointer that has been stored into it, i.e. the load result from a
  `Atomic<*mut T>` should have the same provenance.

  Technically, without this, `Atomic<*mut T>` still work without any
  safety issue itself, but the user of it must maintain the provenance
  themselves before store or after load.

  And it turns out it's not very hard to prove the current
  implementation achieve this:

  - For a non-atomic operation done on the atomic variable, they are
    already using pointer operation, so the provenance has been
    preserved.
  - For an atomic operation, since they are done via inline asm code, in
    Rust's abstract machine, they can be treated as pointer read and
    write:

    a) A load of the atomic can be treated as a pointer read and then
       exposing the provenance.
    b) A store of the atomic can be treated as a pointer write with a
       value created with the exposed provenance.

    And our implementation, thanks to no arbitrary type coercion,
    already guarantee that for each a) there is a from_repr() after and
    for each b) there is a into_repr() before. And from_repr() acts as
    a with_exposed_provenance() and into_repr() acts as a
    expose_provenance(). Hence the provenance is preserved.

  Note this is a global property and it has to proven at `Atomic<T>`
  level.

Regards,
Boqun

On Fri Jul 4, 2025 at 10:25 PM CEST, Boqun Feng wrote:
> There are a few off-list discussions, and I've been trying some
> experiment myself, here are a few points/concepts that will help future
> discussion or documentation, so I put it down here:
>
> * Round-trip transmutability (thank Benno for the name!).
>
>   We realize this should be a safety requirement of `AllowAtomic` type
>   (i.e. the type that can be put in a Atomic<T>). What it means is:
>
>   - If `T: AllowAtomic`, transmute() from `T` to `T::Repr` is always
>     safe and

s/safe/sound/

>   - if a value of `T::Repr` is a result of transmute() from `T` to
>     `T::Repr`, then `transmute()` for that value to `T` is also safe.

s/safe/sound/

:)

>
>   This essentially means a valid bit pattern of `T: AllowAtomic` has to
>   be a valid bit pattern of `T::Repr`.
>
>   This is needed because the atomic framework operates on `T::Repr` to
>   implement atomic operations on `T`.
>
>   Note that this is more relaxed than bi-direct transmutability (i.e.
>   transmute() between `T` and `T::Repr`) because we want to support
>   atomic type over unit-only enums:
>
>     #[repr(i32)]
>     pub enum State {
>         Init = 0,
> 	Working = 1,
> 	Done = 2,
>     }
>
>   This should be really helpful to support atomics as states, for
>   example:
>
>     https://lore.kernel.org/rust-for-linux/20250702-module-params-v3-v14-1-5b1cc32311af@kernel.org/
>
> * transmute()-equivalent from_repr() and into_repr().

Hmm I don't think this name fits the description below, how about
"bit-equivalency of from_repr() and into_repr()"? We don't need to
transmute, we only want to ensure that `{from,into}_repr` are just
transmutes.

>   (This is not a safety requirement)
>
>   from_repr() and into_repr(), if exist, should behave like transmute()
>   on the bit pattern of the results, in other words, bit patterns of `T`
>   or `T::Repr` should stay the same before and after these operations.
>
>   Of course if we remove them and replace with transmute(), same result.
>
>   This reflects the fact that customized atomic types should store
>   unmodified bit patterns into atomic variables, and this make atomic
>   operations don't have weird behavior [1] when combined with new(),
>   from_ptr() and get_mut().

I remember that this was required to support types like `(u8, u16)`? If
yes, then it would be good to include a paragraph like the one above for
enums :)

> * Provenance preservation.
>
>   (This is not a safety requirement for Atomic itself)
>
>   For a `Atomic<*mut T>`, it should preserve the provenance of the
>   pointer that has been stored into it, i.e. the load result from a
>   `Atomic<*mut T>` should have the same provenance.
>
>   Technically, without this, `Atomic<*mut T>` still work without any
>   safety issue itself, but the user of it must maintain the provenance
>   themselves before store or after load.
>
>   And it turns out it's not very hard to prove the current
>   implementation achieve this:
>
>   - For a non-atomic operation done on the atomic variable, they are
>     already using pointer operation, so the provenance has been
>     preserved.
>   - For an atomic operation, since they are done via inline asm code, in
>     Rust's abstract machine, they can be treated as pointer read and
>     write:
>
>     a) A load of the atomic can be treated as a pointer read and then
>        exposing the provenance.
>     b) A store of the atomic can be treated as a pointer write with a
>        value created with the exposed provenance.
>
>     And our implementation, thanks to no arbitrary type coercion,
>     already guarantee that for each a) there is a from_repr() after and
>     for each b) there is a into_repr() before. And from_repr() acts as
>     a with_exposed_provenance() and into_repr() acts as a
>     expose_provenance(). Hence the provenance is preserved.

I'm not sure this point is correct, but I'm an atomics noob, so maybe
Gary should take a look at this :)

>   Note this is a global property and it has to proven at `Atomic<T>`
>   level.

Thanks for he awesome writeup, do you want to put this in some comment
or at least the commit log?

---
Cheers,
Benno

On Fri, Jul 04, 2025 at 10:45:48PM +0200, Benno Lossin wrote:
> On Fri Jul 4, 2025 at 10:25 PM CEST, Boqun Feng wrote:
> > There are a few off-list discussions, and I've been trying some
> > experiment myself, here are a few points/concepts that will help future
> > discussion or documentation, so I put it down here:
> >
> > * Round-trip transmutability (thank Benno for the name!).
> >
> >   We realize this should be a safety requirement of `AllowAtomic` type
> >   (i.e. the type that can be put in a Atomic<T>). What it means is:
> >
> >   - If `T: AllowAtomic`, transmute() from `T` to `T::Repr` is always
> >     safe and
> 
> s/safe/sound/
> 
> >   - if a value of `T::Repr` is a result of transmute() from `T` to
> >     `T::Repr`, then `transmute()` for that value to `T` is also safe.
> 
> s/safe/sound/
> 

Make sense.

> :)
> 
> >
> >   This essentially means a valid bit pattern of `T: AllowAtomic` has to
> >   be a valid bit pattern of `T::Repr`.
> >
> >   This is needed because the atomic framework operates on `T::Repr` to
> >   implement atomic operations on `T`.
> >
> >   Note that this is more relaxed than bi-direct transmutability (i.e.
> >   transmute() between `T` and `T::Repr`) because we want to support
> >   atomic type over unit-only enums:
> >
> >     #[repr(i32)]
> >     pub enum State {
> >         Init = 0,
> > 	Working = 1,
> > 	Done = 2,
> >     }
> >
> >   This should be really helpful to support atomics as states, for
> >   example:
> >
> >     https://lore.kernel.org/rust-for-linux/20250702-module-params-v3-v14-1-5b1cc32311af@kernel.org/
> >
> > * transmute()-equivalent from_repr() and into_repr().
> 
> Hmm I don't think this name fits the description below, how about
> "bit-equivalency of from_repr() and into_repr()"? We don't need to
> transmute, we only want to ensure that `{from,into}_repr` are just
> transmutes.
> 

Good point!

Btw, do you offer naming service, I will pay! ;-)

> >   (This is not a safety requirement)
> >
> >   from_repr() and into_repr(), if exist, should behave like transmute()
> >   on the bit pattern of the results, in other words, bit patterns of `T`
> >   or `T::Repr` should stay the same before and after these operations.
> >
> >   Of course if we remove them and replace with transmute(), same result.
> >
> >   This reflects the fact that customized atomic types should store
> >   unmodified bit patterns into atomic variables, and this make atomic
> >   operations don't have weird behavior [1] when combined with new(),
> >   from_ptr() and get_mut().
> 
> I remember that this was required to support types like `(u8, u16)`? If

My bad, I forgot to put the link to [1]...

[1]: https://lore.kernel.org/rust-for-linux/20250621123212.66fb016b.gary@garyguo.net/

Basically, without requiring from_repr() and into_repr() to act as a
transmute(), you can have weird types in Atomic<T>.

`(u8, u16)` (in case it's not clear to other audience, it's tuple with a
`u8` and a `u16` in it, so there is a 8-bit hole) is not going to
support until we have something like a `Atomic<MaybeUninit<i32>>`.

> yes, then it would be good to include a paragraph like the one above for
> enums :)
> 
> > * Provenance preservation.
> >
> >   (This is not a safety requirement for Atomic itself)
> >
> >   For a `Atomic<*mut T>`, it should preserve the provenance of the
> >   pointer that has been stored into it, i.e. the load result from a
> >   `Atomic<*mut T>` should have the same provenance.
> >
> >   Technically, without this, `Atomic<*mut T>` still work without any
> >   safety issue itself, but the user of it must maintain the provenance
> >   themselves before store or after load.
> >
> >   And it turns out it's not very hard to prove the current
> >   implementation achieve this:
> >
> >   - For a non-atomic operation done on the atomic variable, they are
> >     already using pointer operation, so the provenance has been
> >     preserved.
> >   - For an atomic operation, since they are done via inline asm code, in
> >     Rust's abstract machine, they can be treated as pointer read and
> >     write:
> >
> >     a) A load of the atomic can be treated as a pointer read and then
> >        exposing the provenance.
> >     b) A store of the atomic can be treated as a pointer write with a
> >        value created with the exposed provenance.
> >
> >     And our implementation, thanks to no arbitrary type coercion,
> >     already guarantee that for each a) there is a from_repr() after and
> >     for each b) there is a into_repr() before. And from_repr() acts as
> >     a with_exposed_provenance() and into_repr() acts as a
> >     expose_provenance(). Hence the provenance is preserved.
> 
> I'm not sure this point is correct, but I'm an atomics noob, so maybe
> Gary should take a look at this :)
> 

Basically, what I'm trying to prove is that we can have a provenance-
preserved Atomic<*mut T> implementation based on the C atomics. Either
that is true, or we should write our own atomic pointer implementation.

> >   Note this is a global property and it has to proven at `Atomic<T>`
> >   level.
> 
> Thanks for he awesome writeup, do you want to put this in some comment
> or at least the commit log?
> 

Yes, so the round-trip transmutability will be in the safe requirement
of `AllowAtomic`. And if we still keep `from_repr()` and `into_repr()`
(we can give them default implementation using trasnmute()), I will put
the "bit-equivalency of from_repr() and into_repr()" in the requirement
of `AllowAtomic` as well.

For the "Provenance preservation", I will put it before `impl
AllowAtomic for *mut T`. (Remember we recently discover that doc comment
works for impl block as well? [2])

[2]: https://lore.kernel.org/rust-for-linux/aD4NW2vDc9rKBDPy@tardis.local/

Regards,
Boqun

> ---
> Cheers,
> Benno

On Fri Jul 4, 2025 at 11:17 PM CEST, Boqun Feng wrote:
> On Fri, Jul 04, 2025 at 10:45:48PM +0200, Benno Lossin wrote:
>> On Fri Jul 4, 2025 at 10:25 PM CEST, Boqun Feng wrote:
>> > * transmute()-equivalent from_repr() and into_repr().
>> 
>> Hmm I don't think this name fits the description below, how about
>> "bit-equivalency of from_repr() and into_repr()"? We don't need to
>> transmute, we only want to ensure that `{from,into}_repr` are just
>> transmutes.
>> 
>
> Good point!
>
> Btw, do you offer naming service, I will pay! ;-)

:) :)

>> >   (This is not a safety requirement)
>> >
>> >   from_repr() and into_repr(), if exist, should behave like transmute()
>> >   on the bit pattern of the results, in other words, bit patterns of `T`
>> >   or `T::Repr` should stay the same before and after these operations.
>> >
>> >   Of course if we remove them and replace with transmute(), same result.
>> >
>> >   This reflects the fact that customized atomic types should store
>> >   unmodified bit patterns into atomic variables, and this make atomic
>> >   operations don't have weird behavior [1] when combined with new(),
>> >   from_ptr() and get_mut().
>> 
>> I remember that this was required to support types like `(u8, u16)`? If
>
> My bad, I forgot to put the link to [1]...
>
> [1]: https://lore.kernel.org/rust-for-linux/20250621123212.66fb016b.gary@garyguo.net/
>
> Basically, without requiring from_repr() and into_repr() to act as a
> transmute(), you can have weird types in Atomic<T>.

Ah right, I forgot some context... Is this really a problem? I mean it's
weird sure, but if someone needs this, then it's fine?

> `(u8, u16)` (in case it's not clear to other audience, it's tuple with a
> `u8` and a `u16` in it, so there is a 8-bit hole) is not going to
> support until we have something like a `Atomic<MaybeUninit<i32>>`.

Ahh right we also had this issue, could you also include that in your
writeup? :)

>> yes, then it would be good to include a paragraph like the one above for
>> enums :)
>> 
>> > * Provenance preservation.
>> >
>> >   (This is not a safety requirement for Atomic itself)
>> >
>> >   For a `Atomic<*mut T>`, it should preserve the provenance of the
>> >   pointer that has been stored into it, i.e. the load result from a
>> >   `Atomic<*mut T>` should have the same provenance.
>> >
>> >   Technically, without this, `Atomic<*mut T>` still work without any
>> >   safety issue itself, but the user of it must maintain the provenance
>> >   themselves before store or after load.
>> >
>> >   And it turns out it's not very hard to prove the current
>> >   implementation achieve this:
>> >
>> >   - For a non-atomic operation done on the atomic variable, they are
>> >     already using pointer operation, so the provenance has been
>> >     preserved.
>> >   - For an atomic operation, since they are done via inline asm code, in
>> >     Rust's abstract machine, they can be treated as pointer read and
>> >     write:
>> >
>> >     a) A load of the atomic can be treated as a pointer read and then
>> >        exposing the provenance.
>> >     b) A store of the atomic can be treated as a pointer write with a
>> >        value created with the exposed provenance.
>> >
>> >     And our implementation, thanks to no arbitrary type coercion,
>> >     already guarantee that for each a) there is a from_repr() after and
>> >     for each b) there is a into_repr() before. And from_repr() acts as
>> >     a with_exposed_provenance() and into_repr() acts as a
>> >     expose_provenance(). Hence the provenance is preserved.
>> 
>> I'm not sure this point is correct, but I'm an atomics noob, so maybe
>> Gary should take a look at this :)
>> 
>
> Basically, what I'm trying to prove is that we can have a provenance-
> preserved Atomic<*mut T> implementation based on the C atomics. Either
> that is true, or we should write our own atomic pointer implementation.

That much I remembered :) But since you were going into the specifics
above, I think we should try to be correct. But maybe natural language
is the wrong medium for that, just write the rust code and we'll see...

>> >   Note this is a global property and it has to proven at `Atomic<T>`
>> >   level.
>> 
>> Thanks for he awesome writeup, do you want to put this in some comment
>> or at least the commit log?
>> 
>
> Yes, so the round-trip transmutability will be in the safe requirement
> of `AllowAtomic`. And if we still keep `from_repr()` and `into_repr()`
> (we can give them default implementation using trasnmute()), I will put
> the "bit-equivalency of from_repr() and into_repr()" in the requirement
> of `AllowAtomic` as well.
>
> For the "Provenance preservation", I will put it before `impl
> AllowAtomic for *mut T`. (Remember we recently discover that doc comment
> works for impl block as well? [2])

Yeah that sounds good!

---
Cheers,
Benno

> [2]: https://lore.kernel.org/rust-for-linux/aD4NW2vDc9rKBDPy@tardis.local/

On Sat, Jul 05, 2025 at 12:38:05AM +0200, Benno Lossin wrote:
[..]
> >> >   (This is not a safety requirement)
> >> >
> >> >   from_repr() and into_repr(), if exist, should behave like transmute()
> >> >   on the bit pattern of the results, in other words, bit patterns of `T`
> >> >   or `T::Repr` should stay the same before and after these operations.
> >> >
> >> >   Of course if we remove them and replace with transmute(), same result.
> >> >
> >> >   This reflects the fact that customized atomic types should store
> >> >   unmodified bit patterns into atomic variables, and this make atomic
> >> >   operations don't have weird behavior [1] when combined with new(),
> >> >   from_ptr() and get_mut().
> >> 
> >> I remember that this was required to support types like `(u8, u16)`? If
> >
> > My bad, I forgot to put the link to [1]...
> >
> > [1]: https://lore.kernel.org/rust-for-linux/20250621123212.66fb016b.gary@garyguo.net/
> >
> > Basically, without requiring from_repr() and into_repr() to act as a
> > transmute(), you can have weird types in Atomic<T>.
> 
> Ah right, I forgot some context... Is this really a problem? I mean it's

It's not a problem for safety, so it's not a safety requirement. But I
really don't see a reason why we want to support this. Not supporting
this makes the atomic implementation reasoning easier.

> weird sure, but if someone needs this, then it's fine?
> 

They can always play the !value game outside atomic, i.e. !value before
store and !value after load, so I don't think it's reasonable request.

> > `(u8, u16)` (in case it's not clear to other audience, it's tuple with a
> > `u8` and a `u16` in it, so there is a 8-bit hole) is not going to
> > support until we have something like a `Atomic<MaybeUninit<i32>>`.
> 
> Ahh right we also had this issue, could you also include that in your
> writeup? :)
> 

Sure, I will put it in a limitation section maybe.

> >> yes, then it would be good to include a paragraph like the one above for
> >> enums :)
> >> 
> >> > * Provenance preservation.
> >> >
> >> >   (This is not a safety requirement for Atomic itself)
> >> >
> >> >   For a `Atomic<*mut T>`, it should preserve the provenance of the
> >> >   pointer that has been stored into it, i.e. the load result from a
> >> >   `Atomic<*mut T>` should have the same provenance.
> >> >
> >> >   Technically, without this, `Atomic<*mut T>` still work without any
> >> >   safety issue itself, but the user of it must maintain the provenance
> >> >   themselves before store or after load.
> >> >
> >> >   And it turns out it's not very hard to prove the current
> >> >   implementation achieve this:
> >> >
> >> >   - For a non-atomic operation done on the atomic variable, they are
> >> >     already using pointer operation, so the provenance has been
> >> >     preserved.
> >> >   - For an atomic operation, since they are done via inline asm code, in
> >> >     Rust's abstract machine, they can be treated as pointer read and
> >> >     write:
> >> >
> >> >     a) A load of the atomic can be treated as a pointer read and then
> >> >        exposing the provenance.
> >> >     b) A store of the atomic can be treated as a pointer write with a
> >> >        value created with the exposed provenance.
> >> >
> >> >     And our implementation, thanks to no arbitrary type coercion,
> >> >     already guarantee that for each a) there is a from_repr() after and
> >> >     for each b) there is a into_repr() before. And from_repr() acts as
> >> >     a with_exposed_provenance() and into_repr() acts as a
> >> >     expose_provenance(). Hence the provenance is preserved.
> >> 
> >> I'm not sure this point is correct, but I'm an atomics noob, so maybe
> >> Gary should take a look at this :)
> >> 
> >
> > Basically, what I'm trying to prove is that we can have a provenance-
> > preserved Atomic<*mut T> implementation based on the C atomics. Either
> > that is true, or we should write our own atomic pointer implementation.
> 
> That much I remembered :) But since you were going into the specifics
> above, I think we should try to be correct. But maybe natural language
> is the wrong medium for that, just write the rust code and we'll see...
> 

I don't thinking writing rust code can help us here other than duplicate
my reasoning above, so like:

    ipml *mut() {
        pub fn xchg(ptr: *mut *mut (), new: *mut ()) -> *mut () {
	    // SAFTEY: ..
	    // `atomic_long_xchg()` is implemented as asm(), so it can
	    // be treated as a normal pointer swap() hence preserve the
	    // provenance.
	    unsafe { atomic_long_xchg(ptr.cast::<atomic_long_t>(), new as ffi:c_long) }
	}

        pub fn cmpxchg(ptr: *mut *mut (), old: *mut (), new: *mut ()) -> *mut () {
	    // SAFTEY: ..
	    // `atomic_long_xchg()` is implemented as asm(), so it can
	    // be treated as a normal pointer compare_exchange() hence preserve the
	    // provenance.
	    unsafe { atomic_long_cmpxchg(ptr.cast::<atomic_long_t>(), old as ffi::c_long, new as ffi:c_long) }
	}

	<do it for a lot of functions>
    }

So I don't think that approach is worth doing. Again the provenance
preserving is a global property, either we have it as whole or we don't
have it, and adding precise comment of each function call won't change
the result. I don't see much difference between reasoning about a set of
functions vs. reasoning one function by one function with the same
reasoning.

If we have a reason to believe that C atomic doesn't support this we
just need to move to our own implementation. I know you (and probably
Gary) may feel the reasoning about provenance preserving a bit handwavy,
but this is probably the best we can get, and it's technically better
than using Rust native atomics, because that's just UB and no one would
help you.

(I made a copy-pasta on purpose above, just to make another point why
writing each function out is not worth)

Regards,
Boqun


> >> >   Note this is a global property and it has to proven at `Atomic<T>`
> >> >   level.
> >> 
> >> Thanks for he awesome writeup, do you want to put this in some comment
> >> or at least the commit log?
> >> 
> >
> > Yes, so the round-trip transmutability will be in the safe requirement
> > of `AllowAtomic`. And if we still keep `from_repr()` and `into_repr()`
> > (we can give them default implementation using trasnmute()), I will put
> > the "bit-equivalency of from_repr() and into_repr()" in the requirement
> > of `AllowAtomic` as well.
> >
> > For the "Provenance preservation", I will put it before `impl
> > AllowAtomic for *mut T`. (Remember we recently discover that doc comment
> > works for impl block as well? [2])
> 
> Yeah that sounds good!
> 
> ---
> Cheers,
> Benno
> 
> > [2]: https://lore.kernel.org/rust-for-linux/aD4NW2vDc9rKBDPy@tardis.local/

On Sat Jul 5, 2025 at 1:21 AM CEST, Boqun Feng wrote:
> On Sat, Jul 05, 2025 at 12:38:05AM +0200, Benno Lossin wrote:
> [..]
>> >> >   (This is not a safety requirement)
>> >> >
>> >> >   from_repr() and into_repr(), if exist, should behave like transmute()
>> >> >   on the bit pattern of the results, in other words, bit patterns of `T`
>> >> >   or `T::Repr` should stay the same before and after these operations.
>> >> >
>> >> >   Of course if we remove them and replace with transmute(), same result.
>> >> >
>> >> >   This reflects the fact that customized atomic types should store
>> >> >   unmodified bit patterns into atomic variables, and this make atomic
>> >> >   operations don't have weird behavior [1] when combined with new(),
>> >> >   from_ptr() and get_mut().
>> >> 
>> >> I remember that this was required to support types like `(u8, u16)`? If
>> >
>> > My bad, I forgot to put the link to [1]...
>> >
>> > [1]: https://lore.kernel.org/rust-for-linux/20250621123212.66fb016b.gary@garyguo.net/
>> >
>> > Basically, without requiring from_repr() and into_repr() to act as a
>> > transmute(), you can have weird types in Atomic<T>.
>> 
>> Ah right, I forgot some context... Is this really a problem? I mean it's
>
> It's not a problem for safety, so it's not a safety requirement. But I
> really don't see a reason why we want to support this. Not supporting
> this makes the atomic implementation reasoning easier.

Yeah.

>> weird sure, but if someone needs this, then it's fine?
>> 
>
> They can always play the !value game outside atomic, i.e. !value before
> store and !value after load, so I don't think it's reasonable request.

That's true, yeah let's forbid this :)

>> > `(u8, u16)` (in case it's not clear to other audience, it's tuple with a
>> > `u8` and a `u16` in it, so there is a 8-bit hole) is not going to
>> > support until we have something like a `Atomic<MaybeUninit<i32>>`.
>> 
>> Ahh right we also had this issue, could you also include that in your
>> writeup? :)
>> 
>
> Sure, I will put it in a limitation section maybe.
>
>> >> yes, then it would be good to include a paragraph like the one above for
>> >> enums :)
>> >> 
>> >> > * Provenance preservation.
>> >> >
>> >> >   (This is not a safety requirement for Atomic itself)
>> >> >
>> >> >   For a `Atomic<*mut T>`, it should preserve the provenance of the
>> >> >   pointer that has been stored into it, i.e. the load result from a
>> >> >   `Atomic<*mut T>` should have the same provenance.
>> >> >
>> >> >   Technically, without this, `Atomic<*mut T>` still work without any
>> >> >   safety issue itself, but the user of it must maintain the provenance
>> >> >   themselves before store or after load.
>> >> >
>> >> >   And it turns out it's not very hard to prove the current
>> >> >   implementation achieve this:
>> >> >
>> >> >   - For a non-atomic operation done on the atomic variable, they are
>> >> >     already using pointer operation, so the provenance has been
>> >> >     preserved.
>> >> >   - For an atomic operation, since they are done via inline asm code, in
>> >> >     Rust's abstract machine, they can be treated as pointer read and
>> >> >     write:
>> >> >
>> >> >     a) A load of the atomic can be treated as a pointer read and then
>> >> >        exposing the provenance.
>> >> >     b) A store of the atomic can be treated as a pointer write with a
>> >> >        value created with the exposed provenance.
>> >> >
>> >> >     And our implementation, thanks to no arbitrary type coercion,
>> >> >     already guarantee that for each a) there is a from_repr() after and
>> >> >     for each b) there is a into_repr() before. And from_repr() acts as
>> >> >     a with_exposed_provenance() and into_repr() acts as a
>> >> >     expose_provenance(). Hence the provenance is preserved.
>> >> 
>> >> I'm not sure this point is correct, but I'm an atomics noob, so maybe
>> >> Gary should take a look at this :)
>> >> 
>> >
>> > Basically, what I'm trying to prove is that we can have a provenance-
>> > preserved Atomic<*mut T> implementation based on the C atomics. Either
>> > that is true, or we should write our own atomic pointer implementation.
>> 
>> That much I remembered :) But since you were going into the specifics
>> above, I think we should try to be correct. But maybe natural language
>> is the wrong medium for that, just write the rust code and we'll see...
>> 
>
> I don't thinking writing rust code can help us here other than duplicate
> my reasoning above, so like:
>
>     ipml *mut() {
>         pub fn xchg(ptr: *mut *mut (), new: *mut ()) -> *mut () {
> 	    // SAFTEY: ..
> 	    // `atomic_long_xchg()` is implemented as asm(), so it can
> 	    // be treated as a normal pointer swap() hence preserve the
> 	    // provenance.

Oh I think Gary was talking specifically about Rust's `asm!`. I don't
know if C asm is going to play the same way... (inside LLVM they
probably are the same thing, but in the abstract machine?)

> 	    unsafe { atomic_long_xchg(ptr.cast::<atomic_long_t>(), new as ffi:c_long) }
> 	}
>
>         pub fn cmpxchg(ptr: *mut *mut (), old: *mut (), new: *mut ()) -> *mut () {
> 	    // SAFTEY: ..
> 	    // `atomic_long_xchg()` is implemented as asm(), so it can
> 	    // be treated as a normal pointer compare_exchange() hence preserve the
> 	    // provenance.
> 	    unsafe { atomic_long_cmpxchg(ptr.cast::<atomic_long_t>(), old as ffi::c_long, new as ffi:c_long) }
> 	}
>
> 	<do it for a lot of functions>
>     }
>
> So I don't think that approach is worth doing. Again the provenance
> preserving is a global property, either we have it as whole or we don't
> have it, and adding precise comment of each function call won't change
> the result. I don't see much difference between reasoning about a set of
> functions vs. reasoning one function by one function with the same
> reasoning.
>
> If we have a reason to believe that C atomic doesn't support this we
> just need to move to our own implementation. I know you (and probably
> Gary) may feel the reasoning about provenance preserving a bit handwavy,

YES :)

> but this is probably the best we can get, and it's technically better

I think we can at improve the safety docs situation.

> than using Rust native atomics, because that's just UB and no one would
> help you.

I'm not arguing using those :)

> (I made a copy-pasta on purpose above, just to make another point why
> writing each function out is not worth)

Yeah that's true, but at the moment that safety comment is on the `impl`
block? I don't think that's the right place...

---
Cheers,
Benno

On Sat, Jul 05, 2025 at 10:04:04AM +0200, Benno Lossin wrote:
[...]
> >> >
> >> > Basically, what I'm trying to prove is that we can have a provenance-
> >> > preserved Atomic<*mut T> implementation based on the C atomics. Either
> >> > that is true, or we should write our own atomic pointer implementation.
> >> 
> >> That much I remembered :) But since you were going into the specifics
> >> above, I think we should try to be correct. But maybe natural language
> >> is the wrong medium for that, just write the rust code and we'll see...
> >> 
> >
> > I don't thinking writing rust code can help us here other than duplicate
> > my reasoning above, so like:
> >
> >     ipml *mut() {
> >         pub fn xchg(ptr: *mut *mut (), new: *mut ()) -> *mut () {
> > 	    // SAFTEY: ..

Note: provenance preserving is not about the safety of Atomic<*mut T>
implementation, even if we don't preserve the provenance, calling
`Atomic<*mut T>` function won't cause UB, it's just that any pointer you
get from `Atomic<*mut T>` is a pointer without provenance.

So what I meant in this example is all the safey comment is above and 
the rest is not a safe comment.

Hope it's clear.

> > 	    // `atomic_long_xchg()` is implemented as asm(), so it can
> > 	    // be treated as a normal pointer swap() hence preserve the
> > 	    // provenance.
> 
> Oh I think Gary was talking specifically about Rust's `asm!`. I don't
> know if C asm is going to play the same way... (inside LLVM they
> probably are the same thing, but in the abstract machine?)
> 

You need to understand why Rust abstract machine model `asm!()` in
that way: Rust abstract machine cannot see through `asm!()`, so it has
to assume that `asm!() block can do anything that some equivalent Rust
code does. Further more, this "can do anything that some equivalent Rust
code does" is only one way to reason, the core part about this is Rust
will be very conservative when using the `asm!()` result for
optimization.

It should apply to C asm!() as well because LLVM cannot know see through
the asm block either. And based on the spirit, it might apply to any C
code as well, because it's outside Rust abstract machine. But if you
don't agree the reasoning, then we just cannot implement Atomic<*mut T>
with the existing C API.

> > 	    unsafe { atomic_long_xchg(ptr.cast::<atomic_long_t>(), new as ffi:c_long) }
> > 	}
> >
> >         pub fn cmpxchg(ptr: *mut *mut (), old: *mut (), new: *mut ()) -> *mut () {
> > 	    // SAFTEY: ..
> > 	    // `atomic_long_xchg()` is implemented as asm(), so it can
> > 	    // be treated as a normal pointer compare_exchange() hence preserve the
> > 	    // provenance.
> > 	    unsafe { atomic_long_cmpxchg(ptr.cast::<atomic_long_t>(), old as ffi::c_long, new as ffi:c_long) }
> > 	}
> >
> > 	<do it for a lot of functions>
> >     }
> >
> > So I don't think that approach is worth doing. Again the provenance
> > preserving is a global property, either we have it as whole or we don't
> > have it, and adding precise comment of each function call won't change
> > the result. I don't see much difference between reasoning about a set of
> > functions vs. reasoning one function by one function with the same
> > reasoning.
> >
> > If we have a reason to believe that C atomic doesn't support this we
> > just need to move to our own implementation. I know you (and probably
> > Gary) may feel the reasoning about provenance preserving a bit handwavy,
> 
> YES :)
> 
> > but this is probably the best we can get, and it's technically better
> 
> I think we can at improve the safety docs situation.
> 

Once again, it's not about the safety of Atomic<*mut T> implementation.

> > than using Rust native atomics, because that's just UB and no one would
> > help you.
> 
> I'm not arguing using those :)
> 
> > (I made a copy-pasta on purpose above, just to make another point why
> > writing each function out is not worth)
> 
> Yeah that's true, but at the moment that safety comment is on the `impl`
> block? I don't think that's the right place...
> 

Feel free to send any patch that improves this in your opinion ;-)

Regards,
Boqun

> ---
> Cheers,
> Benno

On Sat Jul 5, 2025 at 5:38 PM CEST, Boqun Feng wrote:
> On Sat, Jul 05, 2025 at 10:04:04AM +0200, Benno Lossin wrote:
> [...]
>> >> >
>> >> > Basically, what I'm trying to prove is that we can have a provenance-
>> >> > preserved Atomic<*mut T> implementation based on the C atomics. Either
>> >> > that is true, or we should write our own atomic pointer implementation.
>> >> 
>> >> That much I remembered :) But since you were going into the specifics
>> >> above, I think we should try to be correct. But maybe natural language
>> >> is the wrong medium for that, just write the rust code and we'll see...
>> >> 
>> >
>> > I don't thinking writing rust code can help us here other than duplicate
>> > my reasoning above, so like:
>> >
>> >     ipml *mut() {
>> >         pub fn xchg(ptr: *mut *mut (), new: *mut ()) -> *mut () {
>> > 	    // SAFTEY: ..
>
> Note: provenance preserving is not about the safety of Atomic<*mut T>
> implementation, even if we don't preserve the provenance, calling
> `Atomic<*mut T>` function won't cause UB, it's just that any pointer you
> get from `Atomic<*mut T>` is a pointer without provenance.
>
> So what I meant in this example is all the safey comment is above and 
> the rest is not a safe comment.

Yeah it's not a safety requirement, but a guarantee.

> Hope it's clear.
>
>> > 	    // `atomic_long_xchg()` is implemented as asm(), so it can
>> > 	    // be treated as a normal pointer swap() hence preserve the
>> > 	    // provenance.
>> 
>> Oh I think Gary was talking specifically about Rust's `asm!`. I don't
>> know if C asm is going to play the same way... (inside LLVM they
>> probably are the same thing, but in the abstract machine?)
>> 
>
> You need to understand why Rust abstract machine model `asm!()` in
> that way: Rust abstract machine cannot see through `asm!()`, so it has
> to assume that `asm!() block can do anything that some equivalent Rust
> code does. Further more, this "can do anything that some equivalent Rust
> code does" is only one way to reason, the core part about this is Rust
> will be very conservative when using the `asm!()` result for
> optimization.

Yes that makes sense.

> It should apply to C asm!() as well because LLVM cannot know see through
> the asm block either. And based on the spirit, it might apply to any C
> code as well, because it's outside Rust abstract machine. But if you
> don't agree the reasoning, then we just cannot implement Atomic<*mut T>
> with the existing C API.

We probably should run this by t-opsem on the Rust zulip or ask about
this in the next Meeting with the Rust folks.

>> > 	    unsafe { atomic_long_xchg(ptr.cast::<atomic_long_t>(), new as ffi:c_long) }
>> > 	}
>> >
>> >         pub fn cmpxchg(ptr: *mut *mut (), old: *mut (), new: *mut ()) -> *mut () {
>> > 	    // SAFTEY: ..
>> > 	    // `atomic_long_xchg()` is implemented as asm(), so it can
>> > 	    // be treated as a normal pointer compare_exchange() hence preserve the
>> > 	    // provenance.
>> > 	    unsafe { atomic_long_cmpxchg(ptr.cast::<atomic_long_t>(), old as ffi::c_long, new as ffi:c_long) }
>> > 	}
>> >
>> > 	<do it for a lot of functions>
>> >     }
>> >
>> > So I don't think that approach is worth doing. Again the provenance
>> > preserving is a global property, either we have it as whole or we don't
>> > have it, and adding precise comment of each function call won't change
>> > the result. I don't see much difference between reasoning about a set of
>> > functions vs. reasoning one function by one function with the same
>> > reasoning.
>> >
>> > If we have a reason to believe that C atomic doesn't support this we
>> > just need to move to our own implementation. I know you (and probably
>> > Gary) may feel the reasoning about provenance preserving a bit handwavy,
>> 
>> YES :)
>> 
>> > but this is probably the best we can get, and it's technically better
>> 
>> I think we can at improve the safety docs situation.
>> 
>
> Once again, it's not about the safety of Atomic<*mut T> implementation.

"Safety docs" to me means all of these:
* `SAFETY` comments & `# Safety` sections,
* `INVARIANT` comments & `# Invariants` sections,
* `GUARANTEE` comments & `# Guarantees` sections.

Maybe there is a better name...

>> > than using Rust native atomics, because that's just UB and no one would
>> > help you.
>> 
>> I'm not arguing using those :)
>> 
>> > (I made a copy-pasta on purpose above, just to make another point why
>> > writing each function out is not worth)
>> 
>> Yeah that's true, but at the moment that safety comment is on the `impl`
>> block? I don't think that's the right place...
>> 
>
> Feel free to send any patch that improves this in your opinion ;-)

I'd prefer we do it right away. But we should just have one big comment
explaining it on the impl and then in the functions refer to it from a
`GUARANTEE` comment?

---
Cheers,
Benno

On Mon Jun 23, 2025 at 9:09 PM CEST, Boqun Feng wrote:
> On Mon, Jun 23, 2025 at 07:30:19PM +0100, Gary Guo wrote:
>> cannot just transmute between from pointers to usize (which is its
>> Repr):
>> * Transmuting from pointer to usize discards provenance
>> * Transmuting from usize to pointer gives invalid provenance
>> 
>> We want neither behaviour, so we must store `usize` directly and
>> always call into repr functions.
>> 
>
> If we store `usize`, how can we support the `get_mut()` then? E.g.
>
>     static V: i32 = 32;
>
>     let mut x = Atomic::new(&V as *const i32 as *mut i32);
>     // ^ assume we expose_provenance() in new().
>
>     let ptr: &mut *mut i32 = x.get_mut(); // which is `&mut self.0.get()`.
>
>     let ptr_val = *ptr; // Does `ptr_val` have the proper provenance?

If `get_mut` transmutes the integer into a pointer, then it will have
the wrong provenance (it will just have plain invalid provenance).

---
Cheers,
Benno

On Tue, Jun 24, 2025 at 01:27:38AM +0200, Benno Lossin wrote:
> On Mon Jun 23, 2025 at 9:09 PM CEST, Boqun Feng wrote:
> > On Mon, Jun 23, 2025 at 07:30:19PM +0100, Gary Guo wrote:
> >> cannot just transmute between from pointers to usize (which is its
> >> Repr):
> >> * Transmuting from pointer to usize discards provenance
> >> * Transmuting from usize to pointer gives invalid provenance
> >> 
> >> We want neither behaviour, so we must store `usize` directly and
> >> always call into repr functions.
> >> 
> >
> > If we store `usize`, how can we support the `get_mut()` then? E.g.
> >
> >     static V: i32 = 32;
> >
> >     let mut x = Atomic::new(&V as *const i32 as *mut i32);
> >     // ^ assume we expose_provenance() in new().
> >
> >     let ptr: &mut *mut i32 = x.get_mut(); // which is `&mut self.0.get()`.
> >
> >     let ptr_val = *ptr; // Does `ptr_val` have the proper provenance?
> 
> If `get_mut` transmutes the integer into a pointer, then it will have
> the wrong provenance (it will just have plain invalid provenance).
> 

The key topic Gary and I have been discussing is whether we should
define Atomic<T> as:

(my current implementation)

    pub struct Atomic<T: AllowAtomic>(Opaque<T>);

or

(Gary's suggestion)

    pub struct Atomic<T: AllowAtomic>(Opaque<T::Repr>);

`T::Repr` is guaranteed to be the same size and alignment of `T`, and
per our discussion, it makes sense to further require that `transmute<T,
T::Repr>()` should also be safe (as the safety requirement of
`AllowAtomic`), or we can say `T` bit validity can be preserved by
`T::Repr`: a valid bit combination `T` can be transumated to `T::Repr`,
and if transumated back, it's the same bit combination.

Now as I pointed out, if we use `Opaque<T::Repr>`, then `.get_mut()`
would be unsound for `Atomic<*mut T>`. And Gary's concern is that in
the current implementation, we directly cast a `*mut T` (from
`Opaque::get()`) into a `*mut T::Repr`, and pass it directly into C/asm
atomic primitives. However, I think with the additional safety
requirement above, this shouldn't be a problem: because the C/asm atomic
primitives would just pass the address to an asm block, and that'll be
out of Rust abstract machine, and as long as the C/primitives atomic
primitives are implemented correctly, the bit representation of `T`
remains valid after asm blocks.

So I think the current implementation still works and is better.

Regards,
Boqun

> ---
> Cheers,
> Benno

On Tue Jun 24, 2025 at 6:35 PM CEST, Boqun Feng wrote:
> On Tue, Jun 24, 2025 at 01:27:38AM +0200, Benno Lossin wrote:
>> On Mon Jun 23, 2025 at 9:09 PM CEST, Boqun Feng wrote:
>> > On Mon, Jun 23, 2025 at 07:30:19PM +0100, Gary Guo wrote:
>> >> cannot just transmute between from pointers to usize (which is its
>> >> Repr):
>> >> * Transmuting from pointer to usize discards provenance
>> >> * Transmuting from usize to pointer gives invalid provenance
>> >> 
>> >> We want neither behaviour, so we must store `usize` directly and
>> >> always call into repr functions.
>> >> 
>> >
>> > If we store `usize`, how can we support the `get_mut()` then? E.g.
>> >
>> >     static V: i32 = 32;
>> >
>> >     let mut x = Atomic::new(&V as *const i32 as *mut i32);
>> >     // ^ assume we expose_provenance() in new().
>> >
>> >     let ptr: &mut *mut i32 = x.get_mut(); // which is `&mut self.0.get()`.
>> >
>> >     let ptr_val = *ptr; // Does `ptr_val` have the proper provenance?
>> 
>> If `get_mut` transmutes the integer into a pointer, then it will have
>> the wrong provenance (it will just have plain invalid provenance).
>> 
>
> The key topic Gary and I have been discussing is whether we should
> define Atomic<T> as:
>
> (my current implementation)
>
>     pub struct Atomic<T: AllowAtomic>(Opaque<T>);
>
> or
>
> (Gary's suggestion)
>
>     pub struct Atomic<T: AllowAtomic>(Opaque<T::Repr>);
>
> `T::Repr` is guaranteed to be the same size and alignment of `T`, and
> per our discussion, it makes sense to further require that `transmute<T,
> T::Repr>()` should also be safe (as the safety requirement of
> `AllowAtomic`), or we can say `T` bit validity can be preserved by
> `T::Repr`: a valid bit combination `T` can be transumated to `T::Repr`,
> and if transumated back, it's the same bit combination.
>
> Now as I pointed out, if we use `Opaque<T::Repr>`, then `.get_mut()`
> would be unsound for `Atomic<*mut T>`. And Gary's concern is that in
> the current implementation, we directly cast a `*mut T` (from
> `Opaque::get()`) into a `*mut T::Repr`, and pass it directly into C/asm
> atomic primitives. However, I think with the additional safety
> requirement above, this shouldn't be a problem: because the C/asm atomic
> primitives would just pass the address to an asm block, and that'll be
> out of Rust abstract machine, and as long as the C/primitives atomic
> primitives are implemented correctly, the bit representation of `T`
> remains valid after asm blocks.
>
> So I think the current implementation still works and is better.

I don't think there is a big difference between `Opaque<T>` and
`Opaque<T::Repr>` if we have the transmute equivalence between the two.
From a safety perspective, you don't gain or lose anything by using the
first over the second one. They both require the invariant that they are
valid (as `Opaque` removes that... we should really be using
`UnsafeCell` here instead... why aren't we doing that?).

Where their differences do play a role is in the implementation of the
various operations on the atomic. If you need to pass `*mut T::Repr` to
the C side, it's better if you store `Opaque<T::Repr>` and if you want
to give `&mut T` back to the user, then it's better to
store `Opaque<T>`.

I would choose the one that results in overall less code. It's probably
going to be `Opaque<T::Repr>`, since we will have more operations that
need `*mut T::Repr` than `*mut T`.

Now I don't understand why you value `Opaque<T>` over `Opaque<T::Repr>`,
they are (up to transmute-equivalence) the same.

I think that you said at one point that `Opaque<T>` makes more sense
from a conceptual view, since we're building `Atomic<T>`. I think that
doesn't really matter, since it's implementation detail. The same
argument could be made about casing `u64` to `i64` for implementing the
atomics: just implement atomics in C also for `u64` and then use that
instead...

---
Cheers,
Benno

On Thu, Jun 26, 2025 at 03:54:24PM +0200, Benno Lossin wrote:
> On Tue Jun 24, 2025 at 6:35 PM CEST, Boqun Feng wrote:
> > On Tue, Jun 24, 2025 at 01:27:38AM +0200, Benno Lossin wrote:
> >> On Mon Jun 23, 2025 at 9:09 PM CEST, Boqun Feng wrote:
> >> > On Mon, Jun 23, 2025 at 07:30:19PM +0100, Gary Guo wrote:
> >> >> cannot just transmute between from pointers to usize (which is its
> >> >> Repr):
> >> >> * Transmuting from pointer to usize discards provenance
> >> >> * Transmuting from usize to pointer gives invalid provenance
> >> >> 
> >> >> We want neither behaviour, so we must store `usize` directly and
> >> >> always call into repr functions.
> >> >> 
> >> >
> >> > If we store `usize`, how can we support the `get_mut()` then? E.g.
> >> >
> >> >     static V: i32 = 32;
> >> >
> >> >     let mut x = Atomic::new(&V as *const i32 as *mut i32);
> >> >     // ^ assume we expose_provenance() in new().
> >> >
> >> >     let ptr: &mut *mut i32 = x.get_mut(); // which is `&mut self.0.get()`.
> >> >
> >> >     let ptr_val = *ptr; // Does `ptr_val` have the proper provenance?
> >> 
> >> If `get_mut` transmutes the integer into a pointer, then it will have
> >> the wrong provenance (it will just have plain invalid provenance).
> >> 
> >
> > The key topic Gary and I have been discussing is whether we should
> > define Atomic<T> as:
> >
> > (my current implementation)
> >
> >     pub struct Atomic<T: AllowAtomic>(Opaque<T>);
> >
> > or
> >
> > (Gary's suggestion)
> >
> >     pub struct Atomic<T: AllowAtomic>(Opaque<T::Repr>);
> >
> > `T::Repr` is guaranteed to be the same size and alignment of `T`, and
> > per our discussion, it makes sense to further require that `transmute<T,
> > T::Repr>()` should also be safe (as the safety requirement of
> > `AllowAtomic`), or we can say `T` bit validity can be preserved by
> > `T::Repr`: a valid bit combination `T` can be transumated to `T::Repr`,
> > and if transumated back, it's the same bit combination.
> >
> > Now as I pointed out, if we use `Opaque<T::Repr>`, then `.get_mut()`
> > would be unsound for `Atomic<*mut T>`. And Gary's concern is that in
> > the current implementation, we directly cast a `*mut T` (from
> > `Opaque::get()`) into a `*mut T::Repr`, and pass it directly into C/asm
> > atomic primitives. However, I think with the additional safety
> > requirement above, this shouldn't be a problem: because the C/asm atomic
> > primitives would just pass the address to an asm block, and that'll be
> > out of Rust abstract machine, and as long as the C/primitives atomic
> > primitives are implemented correctly, the bit representation of `T`
> > remains valid after asm blocks.
> >
> > So I think the current implementation still works and is better.
> 
> I don't think there is a big difference between `Opaque<T>` and
> `Opaque<T::Repr>` if we have the transmute equivalence between the two.
> From a safety perspective, you don't gain or lose anything by using the
> first over the second one. They both require the invariant that they are
> valid (as `Opaque` removes that... we should really be using
> `UnsafeCell` here instead... why aren't we doing that?).
> 

I need the `UnsafePinned`-like behavior of `Atomic<*mut T>` to support
Rcu<T>, and I will replace it with `UnsafePinned`, once that's is
available.

Maybe that also means `UnsafePinned<T>` make more sense? Because if `T`
is a pointer, it's easy to prove the provenance is there. (Note a
`&Atomic<*mut T>` may come from a `*mut *mut T`, may be a field in C
struct)

Regards,
Boqun

On Fri Jul 4, 2025 at 11:22 PM CEST, Boqun Feng wrote:
> On Thu, Jun 26, 2025 at 03:54:24PM +0200, Benno Lossin wrote:
>> On Tue Jun 24, 2025 at 6:35 PM CEST, Boqun Feng wrote:
>> > On Tue, Jun 24, 2025 at 01:27:38AM +0200, Benno Lossin wrote:
>> >> On Mon Jun 23, 2025 at 9:09 PM CEST, Boqun Feng wrote:
>> >> > On Mon, Jun 23, 2025 at 07:30:19PM +0100, Gary Guo wrote:
>> >> >> cannot just transmute between from pointers to usize (which is its
>> >> >> Repr):
>> >> >> * Transmuting from pointer to usize discards provenance
>> >> >> * Transmuting from usize to pointer gives invalid provenance
>> >> >> 
>> >> >> We want neither behaviour, so we must store `usize` directly and
>> >> >> always call into repr functions.
>> >> >> 
>> >> >
>> >> > If we store `usize`, how can we support the `get_mut()` then? E.g.
>> >> >
>> >> >     static V: i32 = 32;
>> >> >
>> >> >     let mut x = Atomic::new(&V as *const i32 as *mut i32);
>> >> >     // ^ assume we expose_provenance() in new().
>> >> >
>> >> >     let ptr: &mut *mut i32 = x.get_mut(); // which is `&mut self.0.get()`.
>> >> >
>> >> >     let ptr_val = *ptr; // Does `ptr_val` have the proper provenance?
>> >> 
>> >> If `get_mut` transmutes the integer into a pointer, then it will have
>> >> the wrong provenance (it will just have plain invalid provenance).
>> >> 
>> >
>> > The key topic Gary and I have been discussing is whether we should
>> > define Atomic<T> as:
>> >
>> > (my current implementation)
>> >
>> >     pub struct Atomic<T: AllowAtomic>(Opaque<T>);
>> >
>> > or
>> >
>> > (Gary's suggestion)
>> >
>> >     pub struct Atomic<T: AllowAtomic>(Opaque<T::Repr>);
>> >
>> > `T::Repr` is guaranteed to be the same size and alignment of `T`, and
>> > per our discussion, it makes sense to further require that `transmute<T,
>> > T::Repr>()` should also be safe (as the safety requirement of
>> > `AllowAtomic`), or we can say `T` bit validity can be preserved by
>> > `T::Repr`: a valid bit combination `T` can be transumated to `T::Repr`,
>> > and if transumated back, it's the same bit combination.
>> >
>> > Now as I pointed out, if we use `Opaque<T::Repr>`, then `.get_mut()`
>> > would be unsound for `Atomic<*mut T>`. And Gary's concern is that in
>> > the current implementation, we directly cast a `*mut T` (from
>> > `Opaque::get()`) into a `*mut T::Repr`, and pass it directly into C/asm
>> > atomic primitives. However, I think with the additional safety
>> > requirement above, this shouldn't be a problem: because the C/asm atomic
>> > primitives would just pass the address to an asm block, and that'll be
>> > out of Rust abstract machine, and as long as the C/primitives atomic
>> > primitives are implemented correctly, the bit representation of `T`
>> > remains valid after asm blocks.
>> >
>> > So I think the current implementation still works and is better.
>> 
>> I don't think there is a big difference between `Opaque<T>` and
>> `Opaque<T::Repr>` if we have the transmute equivalence between the two.
>> From a safety perspective, you don't gain or lose anything by using the
>> first over the second one. They both require the invariant that they are
>> valid (as `Opaque` removes that... we should really be using
>> `UnsafeCell` here instead... why aren't we doing that?).
>> 
>
> I need the `UnsafePinned`-like behavior of `Atomic<*mut T>` to support
> Rcu<T>, and I will replace it with `UnsafePinned`, once that's is
> available.

Can you expand on this? What do you mean by "`UnsafePinned`-like
behavior"? And what does `Rcu<T>` have to do with atomics?

> Maybe that also means `UnsafePinned<T>` make more sense? Because if `T`
> is a pointer, it's easy to prove the provenance is there. (Note a
> `&Atomic<*mut T>` may come from a `*mut *mut T`, may be a field in C
> struct)

Also don't understand this.

---
Cheers,
Benno

On Sat, Jul 05, 2025 at 12:05:48AM +0200, Benno Lossin wrote:
[..]
> >> 
> >> I don't think there is a big difference between `Opaque<T>` and
> >> `Opaque<T::Repr>` if we have the transmute equivalence between the two.
> >> From a safety perspective, you don't gain or lose anything by using the
> >> first over the second one. They both require the invariant that they are
> >> valid (as `Opaque` removes that... we should really be using
> >> `UnsafeCell` here instead... why aren't we doing that?).
> >> 
> >
> > I need the `UnsafePinned`-like behavior of `Atomic<*mut T>` to support
> > Rcu<T>, and I will replace it with `UnsafePinned`, once that's is
> > available.
> 
> Can you expand on this? What do you mean by "`UnsafePinned`-like
> behavior"? And what does `Rcu<T>` have to do with atomics?
> 

`Rcu<T>` is an RCU protected (atomic) pointer, the its definition is

    pub struct Rcu<T>(Atomic<*mut T>);

I need Pin<&mut Rcu<T>> and &Rcu<T> able to co-exist: an updater will
have the access to Pin<&mut Rcu<T>>, and all the readers will have the
access to &Rcu<T>, for that I need `Atomic<*mut T>` to be
`UnsafePinned`, because `Pin<&mut Rcu<T>>` cannot imply noalias.

> > Maybe that also means `UnsafePinned<T>` make more sense? Because if `T`
> > is a pointer, it's easy to prove the provenance is there. (Note a
> > `&Atomic<*mut T>` may come from a `*mut *mut T`, may be a field in C
> > struct)
> 
> Also don't understand this.
> 

One of the usage of the atomic is being able to communicate with C side,
for example, if we have a struct foo:

    struct foo {
        struct bar *b;
    }

and writer can do this at C side:

   struct foo *f = ...;
   struct bar *b = kcalloc(*b, ...);

   // init b;

   smp_store_release(&f->b, b);

and a reader at Rust side can do:

    #[repr(transparent)]
    struct Bar(binding::bar);
    struct Foo(Opaque<bindings::foo>);

    fn get_bar(foo: &Foo) {
        let foo_ptr = foo.0.get();

	let b: *mut *mut Bar = unsafe { &raw mut (*foo_ptr).b }.cast();
	// SAFETY: C side accessing this pointer with atomics.
        let b = unsafe { Atomic::<*mut Bar>::from_ptr(b) };

        // Acquire pairs with the Release from C side;
	let bar_ptr = b.load(Acquire);

	// accessing bar.
    }

This is the case we must support if we want to write any non-trivial
synchronization code communicate with C side.

And in this case, it's generally easier to reason why we can convert a
*mut *mut Bar to &UnsafePinned<*mut Bar>.

Regards,
Boqun

> ---
> Cheers,
> Benno

On Sat Jul 5, 2025 at 12:30 AM CEST, Boqun Feng wrote:
> On Sat, Jul 05, 2025 at 12:05:48AM +0200, Benno Lossin wrote:
> [..]
>> >> 
>> >> I don't think there is a big difference between `Opaque<T>` and
>> >> `Opaque<T::Repr>` if we have the transmute equivalence between the two.
>> >> From a safety perspective, you don't gain or lose anything by using the
>> >> first over the second one. They both require the invariant that they are
>> >> valid (as `Opaque` removes that... we should really be using
>> >> `UnsafeCell` here instead... why aren't we doing that?).
>> >> 
>> >
>> > I need the `UnsafePinned`-like behavior of `Atomic<*mut T>` to support
>> > Rcu<T>, and I will replace it with `UnsafePinned`, once that's is
>> > available.
>> 
>> Can you expand on this? What do you mean by "`UnsafePinned`-like
>> behavior"? And what does `Rcu<T>` have to do with atomics?
>> 
>
> `Rcu<T>` is an RCU protected (atomic) pointer, the its definition is
>
>     pub struct Rcu<T>(Atomic<*mut T>);
>
> I need Pin<&mut Rcu<T>> and &Rcu<T> able to co-exist: an updater will
> have the access to Pin<&mut Rcu<T>>, and all the readers will have the
> access to &Rcu<T>, for that I need `Atomic<*mut T>` to be
> `UnsafePinned`, because `Pin<&mut Rcu<T>>` cannot imply noalias.

Then `Rcu` should be
    
    pub struct Rcu<T>(UnsafePinned<Atomic<*mut T>>);

And `Atomic` shouldn't wrap `UnsafePinned<T>`. Because that prevents
`&mut Atomic<i32>` to be tagged with `noalias` and that should be fine.
You should only pay for what you need :)

>> > Maybe that also means `UnsafePinned<T>` make more sense? Because if `T`
>> > is a pointer, it's easy to prove the provenance is there. (Note a
>> > `&Atomic<*mut T>` may come from a `*mut *mut T`, may be a field in C
>> > struct)
>> 
>> Also don't understand this.
>> 
>
> One of the usage of the atomic is being able to communicate with C side,
> for example, if we have a struct foo:
>
>     struct foo {
>         struct bar *b;
>     }
>
> and writer can do this at C side:
>
>    struct foo *f = ...;
>    struct bar *b = kcalloc(*b, ...);
>
>    // init b;
>
>    smp_store_release(&f->b, b);
>
> and a reader at Rust side can do:
>
>     #[repr(transparent)]
>     struct Bar(binding::bar);
>     struct Foo(Opaque<bindings::foo>);
>
>     fn get_bar(foo: &Foo) {
>         let foo_ptr = foo.0.get();
>
>         let b: *mut *mut Bar = unsafe { &raw mut (*foo_ptr).b }.cast();
>         // SAFETY: C side accessing this pointer with atomics.
>         let b = unsafe { Atomic::<*mut Bar>::from_ptr(b) };
>
>         // Acquire pairs with the Release from C side;
>         let bar_ptr = b.load(Acquire);
>
>         // accessing bar.
>     }

This is a nice example, might be a good idea to put this on
`Atomic::from_ptr`.

> This is the case we must support if we want to write any non-trivial
> synchronization code communicate with C side.
>
> And in this case, it's generally easier to reason why we can convert a
> *mut *mut Bar to &UnsafePinned<*mut Bar>.

What does that have to do with `UnsafePinned`? `UnsafeCell` should
suffice.

Also where does the provenance interact with `UnsafePinned`?

---
Cheers,
Benno

On Sat, Jul 05, 2025 at 12:49:09AM +0200, Benno Lossin wrote:
> On Sat Jul 5, 2025 at 12:30 AM CEST, Boqun Feng wrote:
> > On Sat, Jul 05, 2025 at 12:05:48AM +0200, Benno Lossin wrote:
> > [..]
> >> >> 
> >> >> I don't think there is a big difference between `Opaque<T>` and
> >> >> `Opaque<T::Repr>` if we have the transmute equivalence between the two.
> >> >> From a safety perspective, you don't gain or lose anything by using the
> >> >> first over the second one. They both require the invariant that they are
> >> >> valid (as `Opaque` removes that... we should really be using
> >> >> `UnsafeCell` here instead... why aren't we doing that?).
> >> >> 
> >> >
> >> > I need the `UnsafePinned`-like behavior of `Atomic<*mut T>` to support
> >> > Rcu<T>, and I will replace it with `UnsafePinned`, once that's is
> >> > available.
> >> 
> >> Can you expand on this? What do you mean by "`UnsafePinned`-like
> >> behavior"? And what does `Rcu<T>` have to do with atomics?
> >> 
> >
> > `Rcu<T>` is an RCU protected (atomic) pointer, the its definition is
> >
> >     pub struct Rcu<T>(Atomic<*mut T>);
> >
> > I need Pin<&mut Rcu<T>> and &Rcu<T> able to co-exist: an updater will
> > have the access to Pin<&mut Rcu<T>>, and all the readers will have the
> > access to &Rcu<T>, for that I need `Atomic<*mut T>` to be
> > `UnsafePinned`, because `Pin<&mut Rcu<T>>` cannot imply noalias.
> 
> Then `Rcu` should be
>     
>     pub struct Rcu<T>(UnsafePinned<Atomic<*mut T>>);
> 
> And `Atomic` shouldn't wrap `UnsafePinned<T>`. Because that prevents
> `&mut Atomic<i32>` to be tagged with `noalias` and that should be fine.
> You should only pay for what you need :)
> 

Fair enough. Changing it to UnsafeCell then.

> >> > Maybe that also means `UnsafePinned<T>` make more sense? Because if `T`
> >> > is a pointer, it's easy to prove the provenance is there. (Note a
> >> > `&Atomic<*mut T>` may come from a `*mut *mut T`, may be a field in C
> >> > struct)
> >> 
> >> Also don't understand this.
> >> 
> >
> > One of the usage of the atomic is being able to communicate with C side,
> > for example, if we have a struct foo:
> >
> >     struct foo {
> >         struct bar *b;
> >     }
> >
> > and writer can do this at C side:
> >
> >    struct foo *f = ...;
> >    struct bar *b = kcalloc(*b, ...);
> >
> >    // init b;
> >
> >    smp_store_release(&f->b, b);
> >
> > and a reader at Rust side can do:
> >
> >     #[repr(transparent)]
> >     struct Bar(binding::bar);
> >     struct Foo(Opaque<bindings::foo>);
> >
> >     fn get_bar(foo: &Foo) {
> >         let foo_ptr = foo.0.get();
> >
> >         let b: *mut *mut Bar = unsafe { &raw mut (*foo_ptr).b }.cast();
> >         // SAFETY: C side accessing this pointer with atomics.
> >         let b = unsafe { Atomic::<*mut Bar>::from_ptr(b) };
> >
> >         // Acquire pairs with the Release from C side;
> >         let bar_ptr = b.load(Acquire);
> >
> >         // accessing bar.
> >     }
> 
> This is a nice example, might be a good idea to put this on
> `Atomic::from_ptr`.
> 

I have something similar in the doc comment of `Atomic::from_ptr()`,
just not an `Atomic<*mut T>`.

> > This is the case we must support if we want to write any non-trivial
> > synchronization code communicate with C side.
> >
> > And in this case, it's generally easier to reason why we can convert a
> > *mut *mut Bar to &UnsafePinned<*mut Bar>.
> 
> What does that have to do with `UnsafePinned`? `UnsafeCell` should
> suffice.
> 

I was talking about things like UnsafeCell<*mut T> vs UnsafeCell<isize>
not comparing between UnsafePinned and UnsafeCell.

Regards,
Boqun

> Also where does the provenance interact with `UnsafePinned`?
> 
> ---
> Cheers,
> Benno

On Mon Jun 23, 2025 at 7:19 AM CEST, Boqun Feng wrote:
> On Sat, Jun 21, 2025 at 12:32:12PM +0100, Gary Guo wrote:
>> Note tha the transparent new types restriction on `AllowAtomic` is not
>> sufficient for this, as I can define
>> 
>
> Nice catch! I do agree we should disallow `MyWeirdI32`, and I also agree
> that we should put transmutability as safety requirement for
> `AllowAtomic`. However, I would suggest we still keep
> `into_repr`/`from_repr`, and require the implementation to make them
> provide the same results as transmute(), as a correctness precondition
> (instead of a safety precondition), in other words, you can still write
> a `MyWeirdI32`, and it won't cause safety issues, but it'll be
> incorrect.

Hmm I don't like keeping the function when we add the transmute
requirement.

> The reason why I think we should keep `into_repr`/`from_repr` but add
> a correctness precondition is that they are easily to implement as safe
> code for basic types, so it'll be better than a transmute() call. Also
> considering `Atomic<*mut T>`, would transmuting between integers and
> pointers act the same as expose_provenance() and
> from_exposed_provenance()?

Hmmm, this is indeed a problem for pointers. I guess we do need the
functions...

But this also prevents us from adding the transmute requirement, as it
doesn't hold for pointers. Maybe we need to add the requirement that
`into_repr`/`from_repr` preserve the binary representation?

---
Cheers,
Benno

On Mon, Jun 23, 2025 at 01:54:38PM +0200, Benno Lossin wrote:
> On Mon Jun 23, 2025 at 7:19 AM CEST, Boqun Feng wrote:
> > On Sat, Jun 21, 2025 at 12:32:12PM +0100, Gary Guo wrote:
> >> Note tha the transparent new types restriction on `AllowAtomic` is not
> >> sufficient for this, as I can define
> >> 
> >
> > Nice catch! I do agree we should disallow `MyWeirdI32`, and I also agree
> > that we should put transmutability as safety requirement for
> > `AllowAtomic`. However, I would suggest we still keep
> > `into_repr`/`from_repr`, and require the implementation to make them
> > provide the same results as transmute(), as a correctness precondition
> > (instead of a safety precondition), in other words, you can still write
> > a `MyWeirdI32`, and it won't cause safety issues, but it'll be
> > incorrect.
> 
> Hmm I don't like keeping the function when we add the transmute
> requirement.
> 
> > The reason why I think we should keep `into_repr`/`from_repr` but add
> > a correctness precondition is that they are easily to implement as safe
> > code for basic types, so it'll be better than a transmute() call. Also
> > considering `Atomic<*mut T>`, would transmuting between integers and
> > pointers act the same as expose_provenance() and
> > from_exposed_provenance()?
> 
> Hmmm, this is indeed a problem for pointers. I guess we do need the
> functions...
> 
> But this also prevents us from adding the transmute requirement, as it
> doesn't hold for pointers. Maybe we need to add the requirement that

The requirement is "transumability", which requires any valid binary
representation of `T` must be a valid binary representation of
`T::Repr`, and we need it regardless whether we use `transumate()` or
not in the implementation. Because for the current implementation,
`from_ptr()` and any atomics may read a value from `Atomic::new()` needs
this. Even if we change the implementation to `Opaque<T::Repr>`, we
still need it for `get_mut()`

> `into_repr`/`from_repr` preserve the binary representation?

We need this too, but just maybe not for safety reasons. Besides, the
precondition that we can say `into_repr`/`from_repr` can preserve binary
representation is the transmutability requirement.

Regards,
Boqun

> 
> ---
> Cheers,
> Benno