Re: [PATCH v5 10/10] rust: sync: Add memory barriers

Andreas Hindborg
2025-06-30T09:54:46Z

"Boqun Feng" <boqun.feng@gmail.com> writes:

> On Thu, Jun 26, 2025 at 03:36:25PM +0200, Andreas Hindborg wrote:
>> "Boqun Feng" <boqun.feng@gmail.com> writes:
> [...]
>> > +//! [`LKMM`]: srctree/tools/memory-mode/
>>
>> Typo in link target.
>>
>> > +
>> > +/// A compiler barrier.
>> > +///
>> > +/// An explicic compiler barrier function that prevents the compiler from moving the memory
>> > +/// accesses either side of it to the other side.
>>
>> Typo in "explicit".
>>
>
> Fixed.
>
>> How about:
>>
>> A compiler barrier. Prevents the compiler from reordering
>> memory access instructions 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.
>> > + //
>> > + // SAFETY: An empty asm block should be safe.
>> > + unsafe {
>> > + core::arch::asm!("");
>> > + }
>> > +}
>> > +
>> > +/// A full memory barrier.
>> > +///
>> > +/// A barrier function that prevents both the compiler and the CPU from moving the memory accesses
>> > +/// either side of it to the other side.
>>
>>
>> A barrier that prevents compiler and CPU from reordering memory access
>> instructions across the barrier.
>>
>> > +pub fn smp_mb() {
>> > + if cfg!(CONFIG_SMP) {
>> > + // SAFETY: `smp_mb()` is safe to call.
>> > + unsafe {
>> > + bindings::smp_mb();
>> > + }
>> > + } else {
>> > + barrier();
>> > + }
>> > +}
>> > +
>> > +/// A write-write memory barrier.
>> > +///
>> > +/// A barrier function that prevents both the compiler and the CPU from moving the memory write
>> > +/// accesses either side of it to the other side.
>>
>> A barrier that prevents compiler and CPU from reordering memory write
>> instructions across the barrier.
>>
>> > +pub fn smp_wmb() {
>> > + if cfg!(CONFIG_SMP) {
>> > + // SAFETY: `smp_wmb()` is safe to call.
>> > + unsafe {
>> > + bindings::smp_wmb();
>> > + }
>> > + } else {
>> > + barrier();
>> > + }
>> > +}
>> > +
>> > +/// A read-read memory barrier.
>> > +///
>> > +/// A barrier function that prevents both the compiler and the CPU from moving the memory read
>> > +/// accesses either side of it to the other side.
>>
>> A barrier that prevents compiler and CPU from reordering memory read
>> instructions across the barrier.
>>
>
> These are good wording, except that I will use "memory (read/write)
> accesses" instead of "memory (read/write) instructions" because:
>
> 1) "instructions" are at lower level than the language, and memory
> barriers function are provided as synchonization primitives, so I
> feel we should describe memory barrier effects at language level,
> i.e. mention how it would interact with objects and accesses to them.
>
> 2) There are instructions can do read and write in one instruction, it
> might be unclear when we say "prevents reordering an instruction"
> whether both parts are included, for example:
>
> r1 = atomic_add(x, 1); // <- this can be one instruction.
> smp_rmb();
> r2 = atomic_read(y);
>
> people may think because the smp_rmb() prevents read instructions
> reordering, and atomic_add() is one instruction in this case,
> smp_rmb() can prevents the write part of that instruction from
> reordering, but that's not the case.
>
>
> So I will do:
>
> A barrier that prevents compiler and CPU from reordering memory read
> accesses across the barrier.

Right, that makes sense 👍

Best regards,
Andreas Hindborg