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

Benno Lossin
2025-07-05T08:04:11Z

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