Message-ID: <aGhFAlpOZJaLNekS@Mac.home>
Date: Fri, 4 Jul 2025 14:17:54 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Benno Lossin <lossin@...nel.org>
Cc: Gary Guo <gary@...yguo.net>, linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org, lkmm@...ts.linux.dev,
	linux-arch@...r.kernel.org, Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>, Will Deacon <will@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Mark Rutland <mark.rutland@....com>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Lyude Paul <lyude@...hat.com>, Ingo Molnar <mingo@...nel.org>,
	Mitchell Levy <levymitchell0@...il.com>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v5 04/10] rust: sync: atomic: Add generic atomics

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

Powered by blists - more mailing lists