All of lore.kernel.org
 help / color / mirror / Atom feed
From: Elle Rhumsaa <elle@weathered-steel.dev>
To: Boqun Feng <boqun.feng@gmail.com>
Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	lkmm@lists.linux.dev, "Will Deacon" <will@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	stern@rowland.harvard.edu, "Miguel Ojeda" <ojeda@kernel.org>,
	alex.gaynor@gmail.com, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>
Subject: Re: [PATCH 05/14] rust: sync: atomic: Add atomic {cmp,}xchg operations
Date: Sat, 6 Sep 2025 04:23:27 +0000	[thread overview]
Message-ID: <aLu3P7-eV9yCGziJ@archiso> (raw)
In-Reply-To: <20250905044141.77868-6-boqun.feng@gmail.com>

On Thu, Sep 04, 2025 at 09:41:32PM -0700, Boqun Feng wrote:
> xchg() and cmpxchg() are basic operations on atomic. Provide these based
> on C APIs.
> 
> Note that cmpxchg() use the similar function signature as
> compare_exchange() in Rust std: returning a `Result`, `Ok(old)` means
> the operation succeeds and `Err(old)` means the operation fails.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Benno Lossin <lossin@kernel.org>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> Link: https://lore.kernel.org/all/20250719030827.61357-6-boqun.feng@gmail.com/
> ---
>  rust/kernel/sync/atomic.rs | 168 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 167 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> index ea5782b6ee95..4c32d12dc61e 100644
> --- a/rust/kernel/sync/atomic.rs
> +++ b/rust/kernel/sync/atomic.rs
> @@ -25,7 +25,7 @@
>  pub use ordering::{Acquire, Full, Relaxed, Release};
>  
>  use crate::build_error;
> -use internal::{AtomicBasicOps, AtomicRepr};
> +use internal::{AtomicBasicOps, AtomicExchangeOps, AtomicRepr};
>  use ordering::OrderingType;
>  
>  /// A memory location which can be safely modified from multiple execution contexts.
> @@ -293,3 +293,169 @@ pub fn store<Ordering: ordering::ReleaseOrRelaxed>(&self, v: T, _: Ordering) {
>          }
>      }
>  }
> +
> +impl<T: AtomicType> Atomic<T>
> +where
> +    T::Repr: AtomicExchangeOps,
> +{
> +    /// Atomic exchange.
> +    ///
> +    /// Atomically updates `*self` to `v` and returns the old value of `*self`.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::sync::atomic::{Atomic, Acquire, Relaxed};
> +    ///
> +    /// let x = Atomic::new(42);
> +    ///
> +    /// assert_eq!(42, x.xchg(52, Acquire));
> +    /// assert_eq!(52, x.load(Relaxed));
> +    /// ```
> +    #[doc(alias("atomic_xchg", "atomic64_xchg", "swap"))]
> +    #[inline(always)]
> +    pub fn xchg<Ordering: ordering::Ordering>(&self, v: T, _: Ordering) -> T {
> +        let v = into_repr(v);
> +
> +        // INVARIANT: `self.0` is a valid `T` after `atomic_xchg*()` because `v` is transmutable to
> +        // `T`.
> +        let ret = {
> +            match Ordering::TYPE {
> +                OrderingType::Full => T::Repr::atomic_xchg(&self.0, v),
> +                OrderingType::Acquire => T::Repr::atomic_xchg_acquire(&self.0, v),
> +                OrderingType::Release => T::Repr::atomic_xchg_release(&self.0, v),
> +                OrderingType::Relaxed => T::Repr::atomic_xchg_relaxed(&self.0, v),
> +            }
> +        };
> +
> +        // SAFETY: `ret` comes from reading `*self`, which is a valid `T` per type invariants.
> +        unsafe { from_repr(ret) }
> +    }
> +
> +    /// Atomic compare and exchange.
> +    ///
> +    /// If `*self` == `old`, atomically updates `*self` to `new`. Otherwise, `*self` is not
> +    /// modified.
> +    ///
> +    /// Compare: The comparison is done via the byte level comparison between `*self` and `old`.
> +    ///
> +    /// Ordering: When succeeds, provides the corresponding ordering as the `Ordering` type
> +    /// parameter indicates, and a failed one doesn't provide any ordering, the load part of a
> +    /// failed cmpxchg is a [`Relaxed`] load.
> +    ///
> +    /// Returns `Ok(value)` if cmpxchg succeeds, and `value` is guaranteed to be equal to `old`,
> +    /// otherwise returns `Err(value)`, and `value` is the current value of `*self`.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::sync::atomic::{Atomic, Full, Relaxed};
> +    ///
> +    /// let x = Atomic::new(42);
> +    ///
> +    /// // Checks whether cmpxchg succeeded.
> +    /// let success = x.cmpxchg(52, 64, Relaxed).is_ok();
> +    /// # assert!(!success);
> +    ///
> +    /// // Checks whether cmpxchg failed.
> +    /// let failure = x.cmpxchg(52, 64, Relaxed).is_err();
> +    /// # assert!(failure);
> +    ///
> +    /// // Uses the old value if failed, probably re-try cmpxchg.
> +    /// match x.cmpxchg(52, 64, Relaxed) {
> +    ///     Ok(_) => { },
> +    ///     Err(old) => {
> +    ///         // do something with `old`.
> +    ///         # assert_eq!(old, 42);
> +    ///     }
> +    /// }
> +    ///
> +    /// // Uses the latest value regardlessly, same as atomic_cmpxchg() in C.
> +    /// let latest = x.cmpxchg(42, 64, Full).unwrap_or_else(|old| old);
> +    /// # assert_eq!(42, latest);
> +    /// assert_eq!(64, x.load(Relaxed));
> +    /// ```
> +    ///
> +    /// [`Relaxed`]: ordering::Relaxed
> +    #[doc(alias(
> +        "atomic_cmpxchg",
> +        "atomic64_cmpxchg",
> +        "atomic_try_cmpxchg",
> +        "atomic64_try_cmpxchg",
> +        "compare_exchange"
> +    ))]
> +    #[inline(always)]
> +    pub fn cmpxchg<Ordering: ordering::Ordering>(
> +        &self,
> +        mut old: T,
> +        new: T,
> +        o: Ordering,
> +    ) -> Result<T, T> {
> +        // Note on code generation:
> +        //
> +        // try_cmpxchg() is used to implement cmpxchg(), and if the helper functions are inlined,
> +        // the compiler is able to figure out that branch is not needed if the users don't care
> +        // about whether the operation succeeds or not. One exception is on x86, due to commit
> +        // 44fe84459faf ("locking/atomic: Fix atomic_try_cmpxchg() semantics"), the
> +        // atomic_try_cmpxchg() on x86 has a branch even if the caller doesn't care about the
> +        // success of cmpxchg and only wants to use the old value. For example, for code like:
> +        //
> +        //     let latest = x.cmpxchg(42, 64, Full).unwrap_or_else(|old| old);
> +        //
> +        // It will still generate code:
> +        //
> +        //     movl    $0x40, %ecx
> +        //     movl    $0x34, %eax
> +        //     lock
> +        //     cmpxchgl        %ecx, 0x4(%rsp)
> +        //     jne     1f
> +        //     2:
> +        //     ...
> +        //     1:  movl    %eax, %ecx
> +        //     jmp 2b
> +        //
> +        // This might be "fixed" by introducing a try_cmpxchg_exclusive() that knows the "*old"
> +        // location in the C function is always safe to write.
> +        if self.try_cmpxchg(&mut old, new, o) {
> +            Ok(old)
> +        } else {
> +            Err(old)
> +        }
> +    }
> +
> +    /// Atomic compare and exchange and returns whether the operation succeeds.
> +    ///
> +    /// If `*self` == `old`, atomically updates `*self` to `new`. Otherwise, `*self` is not
> +    /// modified, `*old` is updated to the current value of `*self`.
> +    ///
> +    /// "Compare" and "Ordering" part are the same as [`Atomic::cmpxchg()`].
> +    ///
> +    /// Returns `true` means the cmpxchg succeeds otherwise returns `false`.
> +    #[inline(always)]
> +    fn try_cmpxchg<Ordering: ordering::Ordering>(&self, old: &mut T, new: T, _: Ordering) -> bool {
> +        let mut tmp = into_repr(*old);
> +        let new = into_repr(new);
> +
> +        // INVARIANT: `self.0` is a valid `T` after `atomic_try_cmpxchg*()` because `new` is
> +        // transmutable to `T`.
> +        let ret = {
> +            match Ordering::TYPE {
> +                OrderingType::Full => T::Repr::atomic_try_cmpxchg(&self.0, &mut tmp, new),
> +                OrderingType::Acquire => {
> +                    T::Repr::atomic_try_cmpxchg_acquire(&self.0, &mut tmp, new)
> +                }
> +                OrderingType::Release => {
> +                    T::Repr::atomic_try_cmpxchg_release(&self.0, &mut tmp, new)
> +                }
> +                OrderingType::Relaxed => {
> +                    T::Repr::atomic_try_cmpxchg_relaxed(&self.0, &mut tmp, new)
> +                }
> +            }
> +        };
> +
> +        // SAFETY: `tmp` comes from reading `*self`, which is a valid `T` per type invariants.
> +        *old = unsafe { from_repr(tmp) };
> +
> +        ret
> +    }
> +}
> -- 
> 2.51.0
> 
> 

Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>

  reply	other threads:[~2025-09-06  4:23 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-05  4:41 [GIT PULL] [PATCH 00/14] Rust atomic changes for v6.18 Boqun Feng
2025-09-05  4:41 ` [PATCH 01/14] rust: Introduce atomic API helpers Boqun Feng
2025-09-06  4:22   ` Elle Rhumsaa
2025-09-15  7:48   ` [tip: locking/core] " tip-bot2 for Boqun Feng
2025-09-05  4:41 ` [PATCH 02/14] rust: sync: Add basic atomic operation mapping framework Boqun Feng
2025-09-06  4:22   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Boqun Feng
2025-09-15  7:48   ` tip-bot2 for Boqun Feng
2025-09-05  4:41 ` [PATCH 03/14] rust: sync: atomic: Add ordering annotation types Boqun Feng
2025-09-06  4:22   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Boqun Feng
2025-09-15  7:48   ` tip-bot2 for Boqun Feng
2025-09-05  4:41 ` [PATCH 04/14] rust: sync: atomic: Add generic atomics Boqun Feng
2025-09-06  4:23   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Boqun Feng
2025-09-15  7:48   ` tip-bot2 for Boqun Feng
2025-09-05  4:41 ` [PATCH 05/14] rust: sync: atomic: Add atomic {cmp,}xchg operations Boqun Feng
2025-09-06  4:23   ` Elle Rhumsaa [this message]
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Boqun Feng
2025-09-15  7:48   ` tip-bot2 for Boqun Feng
2025-09-05  4:41 ` [PATCH 06/14] rust: sync: atomic: Add the framework of arithmetic operations Boqun Feng
2025-09-06  4:23   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Boqun Feng
2025-09-15  7:48   ` tip-bot2 for Boqun Feng
2025-09-05  4:41 ` [PATCH 07/14] rust: sync: atomic: Add Atomic<u{32,64}> Boqun Feng
2025-09-06  4:24   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Boqun Feng
2025-09-15  7:48   ` tip-bot2 for Boqun Feng
2025-09-05  4:41 ` [PATCH 08/14] rust: sync: atomic: Add Atomic<{usize,isize}> Boqun Feng
2025-09-06  4:24   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Boqun Feng
2025-09-15  7:48   ` tip-bot2 for Boqun Feng
2025-09-05  4:41 ` [PATCH 09/14] rust: sync: Add memory barriers Boqun Feng
2025-09-06  4:25   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Boqun Feng
2025-09-15  7:48   ` tip-bot2 for Boqun Feng
2025-09-05  4:41 ` [PATCH 10/14] rust: implement `kernel::sync::Refcount` Boqun Feng
2025-09-06  4:25   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Gary Guo
2025-09-15  7:48   ` tip-bot2 for Gary Guo
2025-09-05  4:41 ` [PATCH 11/14] rust: make `Arc::into_unique_or_drop` associated function Boqun Feng
2025-09-06  4:25   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Gary Guo
2025-09-15  7:48   ` tip-bot2 for Gary Guo
2025-09-05  4:41 ` [PATCH 12/14] rust: convert `Arc` to use `Refcount` Boqun Feng
2025-09-06  4:26   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Gary Guo
2025-09-15  7:48   ` tip-bot2 for Gary Guo
2025-09-05  4:41 ` [PATCH 13/14] rust: block: convert `block::mq` " Boqun Feng
2025-09-06  4:26   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Gary Guo
2025-09-15  7:48   ` tip-bot2 for Gary Guo
2025-09-05  4:41 ` [PATCH 14/14] MAINTAINERS: update atomic infrastructure entry to include Rust Boqun Feng
2025-09-06  4:26   ` Elle Rhumsaa
2025-09-13 10:15   ` [tip: locking/core] " tip-bot2 for Gary Guo
2025-09-15  7:48   ` tip-bot2 for Gary Guo
2025-09-10  5:27 ` [GIT PULL] [PATCH 00/14] Rust atomic changes for v6.18 Boqun Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aLu3P7-eV9yCGziJ@archiso \
    --to=elle@weathered-steel.dev \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkmm@lists.linux.dev \
    --cc=lossin@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=tglx@linutronix.de \
    --cc=tmgross@umich.edu \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.