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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aF9krO0nVjN0yoEC@Mac.home>
Date: Fri, 27 Jun 2025 20:42:36 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	lkmm@...ts.linux.dev, linux-arch@...r.kernel.org,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <lossin@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
	Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>, Will Deacon <will@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Mark Rutland <mark.rutland@....com>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Lyude Paul <lyude@...hat.com>, Ingo Molnar <mingo@...nel.org>,
	Mitchell Levy <levymitchell0@...il.com>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v5 10/10] rust: sync: Add memory barriers

On Thu, Jun 26, 2025 at 03:36:25PM +0200, Andreas Hindborg wrote:
> "Boqun Feng" <boqun.feng@...il.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
> 
>