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

[Boqun Feng <boqun.feng@gmail.com>]

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.

Regards,
Boqun

> > +pub fn smp_rmb() {
> > +    if cfg!(CONFIG_SMP) {
> > +        // SAFETY: `smp_rmb()` is safe to call.
> > +        unsafe {
> > +            bindings::smp_rmb();
> > +        }
> > +    } else {
> > +        barrier();
> > +    }
> > +}
> 
> 
> Best regards,
> Andreas Hindborg
> 
>