Re: [PATCH 2/2] rust: sync: atomic: Add atomic operation helpers over raw pointers

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

On Wed Jan 21, 2026 at 12:19 PM GMT, Alice Ryhl wrote:
> On Tue, Jan 20, 2026 at 04:47:00PM +0000, Gary Guo wrote:
>> On Tue Jan 20, 2026 at 4:23 PM GMT, Marco Elver wrote:
>> > On Tue, Jan 20, 2026 at 07:52PM +0800, Boqun Feng wrote:
>> >> In order to synchronize with C or external, atomic operations over raw
>> >> pointers, althought previously there is always an `Atomic::from_ptr()`
>> >> to provide a `&Atomic<T>`. However it's more convenient to have helpers
>> >> that directly perform atomic operations on raw pointers. Hence a few are
>> >> added, which are basically a `Atomic::from_ptr().op()` wrapper.
>> >> 
>> >> Note: for naming, since `atomic_xchg()` and `atomic_cmpxchg()` has a
>> >> conflict naming to 32bit C atomic xchg/cmpxchg, hence they are just
>> >> named as `xchg()` and `cmpxchg()`. For `atomic_load()` and
>> >> `atomic_store()`, their 32bit C counterparts are `atomic_read()` and
>> >> `atomic_set()`, so keep the `atomic_` prefix.
>> >> 
>> >> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
>> >> ---
>> >>  rust/kernel/sync/atomic.rs           | 104 +++++++++++++++++++++++++++
>> >>  rust/kernel/sync/atomic/predefine.rs |  46 ++++++++++++
>> >>  2 files changed, 150 insertions(+)
>> >> 
>> >> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
>> >> index d49ee45c6eb7..6c46335bdb8c 100644
>> >> --- a/rust/kernel/sync/atomic.rs
>> >> +++ b/rust/kernel/sync/atomic.rs
>> >> @@ -611,3 +611,107 @@ pub fn cmpxchg<Ordering: ordering::Ordering>(
>> >>          }
>> >>      }
>> >>  }
>> >> +
>> >> +/// Atomic load over raw pointers.
>> >> +///
>> >> +/// This function provides a short-cut of `Atomic::from_ptr().load(..)`, and can be used to work
>> >> +/// with C side on synchronizations:
>> >> +///
>> >> +/// - `atomic_load(.., Relaxed)` maps to `READ_ONCE()` when using for inter-thread communication.
>> >> +/// - `atomic_load(.., Acquire)` maps to `smp_load_acquire()`.
>> >
>> > I'm late to the party and may have missed some discussion, but it might
>> > want restating in the documentation and/or commit log:
>> >
>> > READ_ONCE is meant to be a dependency-ordering primitive, i.e. be more
>> > like memory_order_consume than it is memory_order_relaxed. This has, to
>> > the best of my knowledge, not changed; otherwise lots of kernel code
>> > would be broken.
>> 
>> On the Rust-side documentation we mentioned that `Relaxed` always preserve
>> dependency ordering, so yes, it is closer to `consume` in the C11 model.
>
> Like in the other thread, I still think this is a mistake. Let's be
> explicit about intent and call things that they are.
> https://lore.kernel.org/all/aXDCTvyneWOeok2L@google.com/
>
>> If the idea is to add an explicit `Consume` ordering on the Rust side to
>> document the intent clearly, then I am actually somewhat in favour.
>> 
>> This way, we can for example, map it to a `READ_ONCE` in most cases, but we can
>> also provide an option to upgrade such calls to `smp_load_acquire` in certain
>> cases when needed, e.g. LTO arm64.
>
> It always maps to READ_ONCE(), no? It's just that on LTO arm64 the
> READ_ONCE() macro is implemented like smp_load_acquire().

If we split out two separate orderings then we can make things that don't need
dependency ordering not be upgraded to `smp_load_acquire` and still be
implemented using volatile read.

>
>> However this will mean that Rust code will have one more ordering than the C
>> API, so I am keen on knowing how Boqun, Paul, Peter and others think about this.
>
> On that point, my suggestion would be to use the standard LKMM naming
> such as rcu_dereference() or READ_ONCE().
>
> I'm told that READ_ONCE() apparently has stronger guarantees than an
> atomic consume load, but I'm not clear on what they are.

The semantic is different for a 64-bit read on 32-bit platforms; our
`Atomic::from_ptr().load()` will be atomic (backed by atomic64 where `READ_ONCE`
will tear) -- so if you actually want a atomicity then `READ_ONCE` can be a
pitfall.

On the other hand, if you don't want atomicity (and dependency ordering), e.g.
just doing MMIO read / reading DMA allocation where you only need the "once"
semantics of `READ_ONCE`, then `READ_ONCE` provides you with too much guarantees
that you don't care about.

We'd better not to mix them together, because confusion lead to bugs. I have
described such an example in the HrTimer expires patch where the code assumes
`READ_ONCE()` to be atomic and it actually could break in 32-bit systems, but
probably nobody noticed because 32-bit systems using DRM is rare and the race
condition is hard to trigger.

My suggestion is just use things with atomic in its name for anything that
requires atomicity or ordering, and UB-free volatile access to
`read_volatile()`.

Best,
Gary