Re: [PATCH v6 8/9] rust: sync: Add memory barriers

[Ralf Jung <post@ralfj.de>]

Hi all,

On 11.07.25 20:20, Boqun Feng wrote:
> On Fri, Jul 11, 2025 at 10:57:48AM +0200, Benno Lossin wrote:
>> On Thu Jul 10, 2025 at 8:00 AM CEST, Boqun Feng wrote:
>>> diff --git a/rust/kernel/sync/barrier.rs b/rust/kernel/sync/barrier.rs
>>> new file mode 100644
>>> index 000000000000..df4015221503
>>> --- /dev/null
>>> +++ b/rust/kernel/sync/barrier.rs
>>> @@ -0,0 +1,65 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +//! Memory barriers.
>>> +//!
>>> +//! These primitives have the same semantics as their C counterparts: and the precise definitions
>>> +//! of semantics can be found at [`LKMM`].
>>> +//!
>>> +//! [`LKMM`]: srctree/tools/memory-model/
>>> +
>>> +/// A compiler barrier.
>>> +///
>>> +/// A barrier that prevents compiler from reordering memory accesses across the barrier.
>>> +pub(crate) fn barrier() {
>>> +    // By default, Rust inline asms are treated as being able to access any memory or flags, hence
>>> +    // it suffices as a compiler barrier.
>>
>> I don't know about this, but it also isn't my area of expertise... I
>> think I heard Ralf talk about this at Rust Week, but I don't remember...
>>
> 
> Easy, let's Cc Ralf ;-)
> 
> Ralf, I believe the question here is:
> 
> In kernel C, we define a compiler barrier (barrier()), which is
> implemented as:
> 
> # define barrier() __asm__ __volatile__("": : :"memory")
> 
> Now we want to have a Rust version, and I think an empty `asm!()` should
> be enough as an equivalent as a barrier() in C, because an empty
> `asm!()` in Rust implies "memory" as the clobber:
> 
> 	https://godbolt.org/z/3z3fnWYjs
> 
> ?
> 
> I know you have some opinions on C++ compiler_fence() [1]. But in LKMM,
> barrier() and other barriers work for all memory accesses not just
> atomics, so the problem "So, if your program contains no atomic
> accesses, but some atomic fences, those fences do nothing." doesn't
> exist for us. And our barrier() is strictly weaker than other barriers.
> 
> And based on my understanding of the consensus on Rust vs LKMM, "do
> whatever kernel C does and rely on whatever kernel C relies" is the
> general suggestion, so I think an empty `asm!()` works here. Of course
> if in practice, we find an issue, I'm happy to look for solutions ;-)
> 
> Thoughts?
> 
> [1]: https://github.com/rust-lang/unsafe-code-guidelines/issues/347

If I understood correctly, this is about using "compiler barriers" to order 
volatile accesses that the LKMM uses in lieu of atomic accesses?
I can't give a principled answer here, unfortunately -- as you know, the mapping 
of LKMM through the compiler isn't really in a state where we can make 
principled formal statements. And making principled formal statements is my main 
expertise so I am a bit out of my depth here. ;)

So I agree with your 2nd paragraph: I would say just like the fact that you are 
using volatile accesses in the first place, this falls under "do whatever the C 
code does, it shouldn't be any more broken in Rust than it is in C".

However, saying that it in general "prevents reordering all memory accesses" is 
unlikely to be fully correct -- if the compiler can prove that the inline asm 
block could not possibly have access to a local variable (e.g. because it never 
had its address taken), its accesses can still be reordered. This applies both 
to C compilers and Rust compilers. Extra annotations such as `noalias` (or 
`restrict` in C) can also give rise to reorderings around arbitrary code, 
including such barriers. This is not a problem for concurrent code since it 
would anyway be wrong to claim that some pointer doesn't have aliases when it is 
accessed by multiple threads, but it shows that the framing of barriers in terms 
of preventing reordering of accesses is too imprecise. That's why the C++ memory 
model uses a very different framing, and that's why I can't give a definite 
answer here. :)

Kind regards,
Ralf