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

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: <aF6yRIixTPx5YZbA@Mac.home>
Date: Fri, 27 Jun 2025 08:01:24 -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 04/10] rust: sync: atomic: Add generic atomics

On Thu, Jun 26, 2025 at 02:15:35PM +0200, Andreas Hindborg wrote:
> "Boqun Feng" <boqun.feng@...il.com> writes:
> 
> [...]
> 
> > +
> > +impl<T: AllowAtomic> Atomic<T> {
> > +    /// Creates a new atomic.
> > +    pub const fn new(v: T) -> Self {
> > +        Self(Opaque::new(v))
> > +    }
> > +
> > +    /// Creates a reference to [`Self`] from a pointer.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// - `ptr` has to be a valid pointer.
> > +    /// - `ptr` has to be valid for both reads and writes for the whole lifetime `'a`.
> > +    /// - For the whole lifetime of '`a`, other accesses to the object cannot cause data races
> > +    ///   (defined by [`LKMM`]) against atomic operations on the returned reference.
> 
> I feel the wording is a bit tangled here. How about something along the
> lines of
> 
>   For the duration of `'a`, all accesses to the object must be atomic.
> 

Well, a non-atomic read vs an atomic read is not a data race (for both
Rust memory model and LKMM), so your proposal is overly restricted.

I can s/the whole lifetime/the duration.

> > +    ///
> > +    /// [`LKMM`]: srctree/tools/memory-model
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// Using [`Atomic::from_ptr()`] combined with [`Atomic::load()`] or [`Atomic::store()`] can
> > +    /// achieve the same functionality as `READ_ONCE()`/`smp_load_acquire()` or
> > +    /// `WRITE_ONCE()`/`smp_store_release()` in C side:
> > +    ///
> > +    /// ```rust
> > +    /// # use kernel::types::Opaque;
> > +    /// use kernel::sync::atomic::{Atomic, Relaxed, Release};
> > +    ///
> > +    /// // Assume there is a C struct `Foo`.
> > +    /// mod cbindings {
> > +    ///     #[repr(C)]
> > +    ///     pub(crate) struct foo { pub(crate) a: i32, pub(crate) b: i32 }
> > +    /// }
> > +    ///
> > +    /// let tmp = Opaque::new(cbindings::foo { a: 1, b: 2});
> > +    ///
> > +    /// // struct foo *foo_ptr = ..;
> > +    /// let foo_ptr = tmp.get();
> > +    ///
> > +    /// // SAFETY: `foo_ptr` is a valid pointer, and `.a` is inbound.
> 
> Did you mean to say "in bounds"? Or what is "inbound"?
> 

I think I meant to say "inbounds", fixed.

> > +    /// let foo_a_ptr = unsafe { core::ptr::addr_of_mut!((*foo_ptr).a) };
> 
> This should be `&raw mut` by now, right?
> 

Right.

[...]
> 
> Typo `AllocAtomic`.
> 

All fixed! Thanks!

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
>