Re: [PATCH v5 04/10] rust: sync: atomic: Add generic atomics

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/[email protected]/
>
> 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/[email protected]/