| 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
| ||
|
Message-ID: <aHUJYTv4_wsatAw5@Mac.home>
Date: Mon, 14 Jul 2025 06:42:57 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Benno Lossin <lossin@...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>,
Andreas Hindborg <a.hindborg@...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>,
Alan Stern <stern@...land.harvard.edu>
Subject: Re: [PATCH v7 2/9] rust: sync: Add basic atomic operation mapping
framework
On Mon, Jul 14, 2025 at 12:03:11PM +0200, Benno Lossin wrote:
[...]
> > +declare_and_impl_atomic_methods!(
> > + /// Basic atomic operations
> > + pub trait AtomicHasBasicOps {
>
> I think we should drop the `Has` from the names. So this one can just be
> `AtomicBasicOps`. Or how about `BasicAtomic`, or `AtomicBase`?
>
Actually, given your feedback to `ordering::Any` vs `core::any::Any`,
I think it makes more sense to keep the current names ;-) These are only
trait names to describe what operations do an `AtomicImpl` has, and they
should be used only in atomic mod, so naming them a bit longer is not a
problem. And by doing so, we free the better names `BasicAtomic` or
`AtomicBase` for other future usages.
Best I could do is `AtomicBasicOps` or `HasAtomicBasicOps`.
> > + /// Atomic read (load).
> > + ///
> > + /// # Safety
> > + /// - `ptr` is aligned to [`align_of::<Self>()`].
> > + /// - `ptr` is valid for reads.
> > + ///
> > + /// [`align_of::<Self>()`]: core::mem::align_of
> > + unsafe fn read[acquire](ptr: *mut Self) -> Self {
> > + bindings::#call(ptr.cast())
> > + }
> > +
> > + /// Atomic set (store).
> > + ///
> > + /// # Safety
> > + /// - `ptr` is aligned to [`align_of::<Self>()`].
> > + /// - `ptr` is valid for writes.
> > + ///
> > + /// [`align_of::<Self>()`]: core::mem::align_of
> > + unsafe fn set[release](ptr: *mut Self, v: Self) {
> > + bindings::#call(ptr.cast(), v)
> > + }
> > + }
> > +);
> > +
> > +declare_and_impl_atomic_methods!(
> > + /// Exchange and compare-and-exchange atomic operations
> > + pub trait AtomicHasXchgOps {
>
> Same here `AtomicXchgOps` or `AtomicExchangeOps` or `AtomicExchange`?
> (I would prefer to not abbreviate it to `Xchg`)
>
The "Xchg" -> "Exchange" part seems fine to me.
> > + /// Atomic exchange.
> > + ///
> > + /// Atomically updates `*ptr` to `v` and returns the old value.
> > + ///
> > + /// # Safety
> > + /// - `ptr` is aligned to [`align_of::<Self>()`].
> > + /// - `ptr` is valid for reads and writes.
> > + ///
> > + /// [`align_of::<Self>()`]: core::mem::align_of
> > + unsafe fn xchg[acquire, release, relaxed](ptr: *mut Self, v: Self) -> Self {
> > + bindings::#call(ptr.cast(), v)
> > + }
> > +
> > + /// Atomic compare and exchange.
> > + ///
> > + /// If `*ptr` == `*old`, atomically updates `*ptr` to `new`. Otherwise, `*ptr` is not
> > + /// modified, `*old` is updated to the current value of `*ptr`.
> > + ///
> > + /// Return `true` if the update of `*ptr` occured, `false` otherwise.
> > + ///
> > + /// # Safety
> > + /// - `ptr` is aligned to [`align_of::<Self>()`].
> > + /// - `ptr` is valid for reads and writes.
> > + /// - `old` is aligned to [`align_of::<Self>()`].
> > + /// - `old` is valid for reads and writes.
> > + ///
> > + /// [`align_of::<Self>()`]: core::mem::align_of
> > + unsafe fn try_cmpxchg[acquire, release, relaxed](ptr: *mut Self, old: *mut Self, new: Self) -> bool {
> > + bindings::#call(ptr.cast(), old, new)
> > + )}
> > + }
> > +);
> > +
> > +declare_and_impl_atomic_methods!(
> > + /// Atomic arithmetic operations
> > + pub trait AtomicHasArithmeticOps {
>
> Forgot to rename this one to `Add`? I think `AtomicAdd` sounds best for
No, because at the `AtomicImpl` level, it's easy to know whether a type
has all the arithmetic operations or none (also the `Delta` type should
be known). But I don't have opinions on renaming it to `AtomicAddOps` or
`HasAtomicAddOps`.
Regards,
Boqun
> this one.
>
> ---
> Cheers,
> Benno
>
> > + /// Atomic add (wrapping).
> > + ///
> > + /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`.
> > + ///
> > + /// # Safety
> > + /// - `ptr` is aligned to `align_of::<Self>()`.
> > + /// - `ptr` is valid for reads and writes.
> > + ///
> > + /// [`align_of::<Self>()`]: core::mem::align_of
> > + unsafe fn add[](ptr: *mut Self, v: Self::Delta) {
> > + bindings::#call(v, ptr.cast())
> > + }
> > +
> > + /// Atomic fetch and add (wrapping).
> > + ///
> > + /// Atomically updates `*ptr` to `(*ptr).wrapping_add(v)`, and returns the value of `*ptr`
> > + /// before the update.
> > + ///
> > + /// # Safety
> > + /// - `ptr` is aligned to `align_of::<Self>()`.
> > + /// - `ptr` is valid for reads and writes.
> > + ///
> > + /// [`align_of::<Self>()`]: core::mem::align_of
> > + unsafe fn fetch_add[acquire, release, relaxed](ptr: *mut Self, v: Self::Delta) -> Self {
> > + bindings::#call(v, ptr.cast())
> > + }
> > + }
> > +);
>
Powered by blists - more mailing lists