[PATCH v1 0/3] rust: Switch to kernel::sync atomic primitives

[PATCH v1 0/3] rust: Switch to kernel::sync atomic primitives

[FUJITA Tomonori]

Convert uses of core::sync::atomic to Rust LKMM atomics.

Each patch can be applied independently, but all of them depend on the
atomic bool series.

https://lore.kernel.org/all/20251230045028.1773445-1-fujita.tomonori@gmail.com/


FUJITA Tomonori (3):
  rust: revocable: Switch to kernel::sync atomic primitives
  rust: list: Switch to kernel::sync atomic primitives
  rust_binder: Switch to kernel::sync atomic primitives

 drivers/android/binder/rust_binder_main.rs | 20 ++++++++----------
 drivers/android/binder/stats.rs            |  8 ++++----
 drivers/android/binder/thread.rs           | 24 ++++++++++------------
 drivers/android/binder/transaction.rs      | 16 +++++++--------
 rust/kernel/list/arc.rs                    | 14 ++++++-------
 rust/kernel/revocable.rs                   | 24 ++++++++++++++--------
 6 files changed, 54 insertions(+), 52 deletions(-)


base-commit: d4cd35bacb06d53acf39ff248ea2b3eef50573f6
-- 
2.43.0

[PATCH v1 1/3] rust: revocable: Switch to kernel::sync atomic primitives

[FUJITA Tomonori]

Convert uses of `AtomicBool` to `Atomic<bool>`.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/revocable.rs | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 0f4ae673256d..47cd2b45eada 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -7,12 +7,20 @@
 
 use pin_init::Wrapper;
 
-use crate::{bindings, prelude::*, sync::rcu, types::Opaque};
+use crate::{
+    bindings,
+    prelude::*,
+    sync::atomic::{
+        ordering::Relaxed,
+        Atomic, //
+    },
+    sync::rcu,
+    types::Opaque, //
+};
 use core::{
     marker::PhantomData,
     ops::Deref,
-    ptr::drop_in_place,
-    sync::atomic::{AtomicBool, Ordering},
+    ptr::drop_in_place, //
 };
 
 /// An object that can become inaccessible at runtime.
@@ -65,7 +73,7 @@
 /// ```
 #[pin_data(PinnedDrop)]
 pub struct Revocable<T> {
-    is_available: AtomicBool,
+    is_available: Atomic<bool>,
     #[pin]
     data: Opaque<T>,
 }
@@ -84,7 +92,7 @@ impl<T> Revocable<T> {
     /// Creates a new revocable instance of the given data.
     pub fn new<E>(data: impl PinInit<T, E>) -> impl PinInit<Self, E> {
         try_pin_init!(Self {
-            is_available: AtomicBool::new(true),
+            is_available: Atomic::new(true),
             data <- Opaque::pin_init(data),
         }? E)
     }
@@ -98,7 +106,7 @@ pub fn new<E>(data: impl PinInit<T, E>) -> impl PinInit<Self, E> {
     /// because another CPU may be waiting to complete the revocation of this object.
     pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
         let guard = rcu::read_lock();
-        if self.is_available.load(Ordering::Relaxed) {
+        if self.is_available.load(Relaxed) {
             // Since `self.is_available` is true, data is initialised and has to remain valid
             // because the RCU read side lock prevents it from being dropped.
             Some(RevocableGuard::new(self.data.get(), guard))
@@ -116,7 +124,7 @@ pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
     /// allowed to sleep because another CPU may be waiting to complete the revocation of this
     /// object.
     pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a T> {
-        if self.is_available.load(Ordering::Relaxed) {
+        if self.is_available.load(Relaxed) {
             // SAFETY: Since `self.is_available` is true, data is initialised and has to remain
             // valid because the RCU read side lock prevents it from being dropped.
             Some(unsafe { &*self.data.get() })
@@ -157,7 +165,7 @@ pub unsafe fn access(&self) -> &T {
     ///
     /// Callers must ensure that there are no more concurrent users of the revocable object.
     unsafe fn revoke_internal<const SYNC: bool>(&self) -> bool {
-        let revoke = self.is_available.swap(false, Ordering::Relaxed);
+        let revoke = self.is_available.xchg(false, Relaxed);
 
         if revoke {
             if SYNC {
-- 
2.43.0

[PATCH v1 2/3] rust: list: Switch to kernel::sync atomic primitives

[FUJITA Tomonori]

Convert uses of `AtomicBool` to `Atomic<bool>`.

Note that the compare_exchange migration simplifies to
`try_cmpxchg()`, since `try_cmpxchg()` provides relaxed ordering on
failure, making the explicit failure ordering unnecessary.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/list/arc.rs | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/rust/kernel/list/arc.rs b/rust/kernel/list/arc.rs
index d92bcf665c89..2282f33913ee 100644
--- a/rust/kernel/list/arc.rs
+++ b/rust/kernel/list/arc.rs
@@ -6,11 +6,11 @@
 
 use crate::alloc::{AllocError, Flags};
 use crate::prelude::*;
+use crate::sync::atomic::{ordering, Atomic};
 use crate::sync::{Arc, ArcBorrow, UniqueArc};
 use core::marker::PhantomPinned;
 use core::ops::Deref;
 use core::pin::Pin;
-use core::sync::atomic::{AtomicBool, Ordering};
 
 /// Declares that this type has some way to ensure that there is exactly one `ListArc` instance for
 /// this id.
@@ -469,7 +469,7 @@ impl<T, U, const ID: u64> core::ops::DispatchFromDyn<ListArc<U, ID>> for ListArc
 /// If the boolean is `false`, then there is no [`ListArc`] for this value.
 #[repr(transparent)]
 pub struct AtomicTracker<const ID: u64 = 0> {
-    inner: AtomicBool,
+    inner: Atomic<bool>,
     // This value needs to be pinned to justify the INVARIANT: comment in `AtomicTracker::new`.
     _pin: PhantomPinned,
 }
@@ -480,12 +480,12 @@ pub fn new() -> impl PinInit<Self> {
         // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
         // not be constructed in an `Arc` that already has a `ListArc`.
         Self {
-            inner: AtomicBool::new(false),
+            inner: Atomic::new(false),
             _pin: PhantomPinned,
         }
     }
 
-    fn project_inner(self: Pin<&mut Self>) -> &mut AtomicBool {
+    fn project_inner(self: Pin<&mut Self>) -> &mut Atomic<bool> {
         // SAFETY: The `inner` field is not structurally pinned, so we may obtain a mutable
         // reference to it even if we only have a pinned reference to `self`.
         unsafe { &mut Pin::into_inner_unchecked(self).inner }
@@ -500,7 +500,7 @@ unsafe fn on_create_list_arc_from_unique(self: Pin<&mut Self>) {
 
     unsafe fn on_drop_list_arc(&self) {
         // INVARIANT: We just dropped a ListArc, so the boolean should be false.
-        self.inner.store(false, Ordering::Release);
+        self.inner.store(false, ordering::Release);
     }
 }
 
@@ -514,8 +514,6 @@ unsafe impl<const ID: u64> TryNewListArc<ID> for AtomicTracker<ID> {
     fn try_new_list_arc(&self) -> bool {
         // INVARIANT: If this method returns true, then the boolean used to be false, and is no
         // longer false, so it is okay for the caller to create a new [`ListArc`].
-        self.inner
-            .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
-            .is_ok()
+        self.inner.cmpxchg(false, true, ordering::Acquire).is_ok()
     }
 }
-- 
2.43.0

[PATCH v1 3/3] rust_binder: Switch to kernel::sync atomic primitives

[FUJITA Tomonori]

Convert uses of AtomicBool, AtomicUsize, and AtomicU32.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 drivers/android/binder/rust_binder_main.rs | 20 ++++++++----------
 drivers/android/binder/stats.rs            |  8 ++++----
 drivers/android/binder/thread.rs           | 24 ++++++++++------------
 drivers/android/binder/transaction.rs      | 16 +++++++--------
 4 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/drivers/android/binder/rust_binder_main.rs b/drivers/android/binder/rust_binder_main.rs
index c79a9e742240..47bfb114cabb 100644
--- a/drivers/android/binder/rust_binder_main.rs
+++ b/drivers/android/binder/rust_binder_main.rs
@@ -18,6 +18,7 @@
     prelude::*,
     seq_file::SeqFile,
     seq_print,
+    sync::atomic::{ordering::Relaxed, Atomic},
     sync::poll::PollTable,
     sync::Arc,
     task::Pid,
@@ -28,10 +29,7 @@
 
 use crate::{context::Context, page_range::Shrinker, process::Process, thread::Thread};
 
-use core::{
-    ptr::NonNull,
-    sync::atomic::{AtomicBool, AtomicUsize, Ordering},
-};
+use core::ptr::NonNull;
 
 mod allocation;
 mod context;
@@ -90,9 +88,9 @@ fn default() -> Self {
 }
 
 fn next_debug_id() -> usize {
-    static NEXT_DEBUG_ID: AtomicUsize = AtomicUsize::new(0);
+    static NEXT_DEBUG_ID: Atomic<usize> = Atomic::new(0);
 
-    NEXT_DEBUG_ID.fetch_add(1, Ordering::Relaxed)
+    NEXT_DEBUG_ID.fetch_add(1, Relaxed)
 }
 
 /// Provides a single place to write Binder return values via the
@@ -215,7 +213,7 @@ fn arc_pin_init(init: impl PinInit<T>) -> Result<DLArc<T>, kernel::error::Error>
 
 struct DeliverCode {
     code: u32,
-    skip: AtomicBool,
+    skip: Atomic<bool>,
 }
 
 kernel::list::impl_list_arc_safe! {
@@ -226,7 +224,7 @@ impl DeliverCode {
     fn new(code: u32) -> Self {
         Self {
             code,
-            skip: AtomicBool::new(false),
+            skip: Atomic::new(false),
         }
     }
 
@@ -235,7 +233,7 @@ fn new(code: u32) -> Self {
     /// This is used instead of removing it from the work list, since `LinkedList::remove` is
     /// unsafe, whereas this method is not.
     fn skip(&self) {
-        self.skip.store(true, Ordering::Relaxed);
+        self.skip.store(true, Relaxed);
     }
 }
 
@@ -245,7 +243,7 @@ fn do_work(
         _thread: &Thread,
         writer: &mut BinderReturnWriter<'_>,
     ) -> Result<bool> {
-        if !self.skip.load(Ordering::Relaxed) {
+        if !self.skip.load(Relaxed) {
             writer.write_code(self.code)?;
         }
         Ok(true)
@@ -259,7 +257,7 @@ fn should_sync_wakeup(&self) -> bool {
 
     fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result<()> {
         seq_print!(m, "{}", prefix);
-        if self.skip.load(Ordering::Relaxed) {
+        if self.skip.load(Relaxed) {
             seq_print!(m, "(skipped) ");
         }
         if self.code == defs::BR_TRANSACTION_COMPLETE {
diff --git a/drivers/android/binder/stats.rs b/drivers/android/binder/stats.rs
index 037002651941..ab75e9561cbf 100644
--- a/drivers/android/binder/stats.rs
+++ b/drivers/android/binder/stats.rs
@@ -5,7 +5,7 @@
 //! Keep track of statistics for binder_logs.
 
 use crate::defs::*;
-use core::sync::atomic::{AtomicU32, Ordering::Relaxed};
+use kernel::sync::atomic::{ordering::Relaxed, Atomic};
 use kernel::{ioctl::_IOC_NR, seq_file::SeqFile, seq_print};
 
 const BC_COUNT: usize = _IOC_NR(BC_REPLY_SG) as usize + 1;
@@ -14,14 +14,14 @@
 pub(crate) static GLOBAL_STATS: BinderStats = BinderStats::new();
 
 pub(crate) struct BinderStats {
-    bc: [AtomicU32; BC_COUNT],
-    br: [AtomicU32; BR_COUNT],
+    bc: [Atomic<u32>; BC_COUNT],
+    br: [Atomic<u32>; BR_COUNT],
 }
 
 impl BinderStats {
     pub(crate) const fn new() -> Self {
         #[expect(clippy::declare_interior_mutable_const)]
-        const ZERO: AtomicU32 = AtomicU32::new(0);
+        const ZERO: Atomic<u32> = Atomic::new(0);
 
         Self {
             bc: [ZERO; BC_COUNT],
diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
index 1a8e6fdc0dc4..82264db06507 100644
--- a/drivers/android/binder/thread.rs
+++ b/drivers/android/binder/thread.rs
@@ -15,6 +15,7 @@
     security,
     seq_file::SeqFile,
     seq_print,
+    sync::atomic::{ordering::Relaxed, Atomic},
     sync::poll::{PollCondVar, PollTable},
     sync::{Arc, SpinLock},
     task::Task,
@@ -34,10 +35,7 @@
     BinderReturnWriter, DArc, DLArc, DTRWrap, DeliverCode, DeliverToRead,
 };
 
-use core::{
-    mem::size_of,
-    sync::atomic::{AtomicU32, Ordering},
-};
+use core::mem::size_of;
 
 /// Stores the layout of the scatter-gather entries. This is used during the `translate_objects`
 /// call and is discarded when it returns.
@@ -273,8 +271,8 @@ struct InnerThread {
 impl InnerThread {
     fn new() -> Result<Self> {
         fn next_err_id() -> u32 {
-            static EE_ID: AtomicU32 = AtomicU32::new(0);
-            EE_ID.fetch_add(1, Ordering::Relaxed)
+            static EE_ID: Atomic<u32> = Atomic::new(0);
+            EE_ID.fetch_add(1, Relaxed)
         }
 
         Ok(Self {
@@ -1537,7 +1535,7 @@ pub(crate) fn release(self: &Arc<Self>) {
 
 #[pin_data]
 struct ThreadError {
-    error_code: AtomicU32,
+    error_code: Atomic<u32>,
     #[pin]
     links_track: AtomicTracker,
 }
@@ -1545,18 +1543,18 @@ struct ThreadError {
 impl ThreadError {
     fn try_new() -> Result<DArc<Self>> {
         DTRWrap::arc_pin_init(pin_init!(Self {
-            error_code: AtomicU32::new(BR_OK),
+            error_code: Atomic::new(BR_OK),
             links_track <- AtomicTracker::new(),
         }))
         .map(ListArc::into_arc)
     }
 
     fn set_error_code(&self, code: u32) {
-        self.error_code.store(code, Ordering::Relaxed);
+        self.error_code.store(code, Relaxed);
     }
 
     fn is_unused(&self) -> bool {
-        self.error_code.load(Ordering::Relaxed) == BR_OK
+        self.error_code.load(Relaxed) == BR_OK
     }
 }
 
@@ -1566,8 +1564,8 @@ fn do_work(
         _thread: &Thread,
         writer: &mut BinderReturnWriter<'_>,
     ) -> Result<bool> {
-        let code = self.error_code.load(Ordering::Relaxed);
-        self.error_code.store(BR_OK, Ordering::Relaxed);
+        let code = self.error_code.load(Relaxed);
+        self.error_code.store(BR_OK, Relaxed);
         writer.write_code(code)?;
         Ok(true)
     }
@@ -1583,7 +1581,7 @@ fn debug_print(&self, m: &SeqFile, prefix: &str, _tprefix: &str) -> Result<()> {
             m,
             "{}transaction error: {}\n",
             prefix,
-            self.error_code.load(Ordering::Relaxed)
+            self.error_code.load(Relaxed)
         );
         Ok(())
     }
diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs
index 4bd3c0e417eb..2273a8e9d01c 100644
--- a/drivers/android/binder/transaction.rs
+++ b/drivers/android/binder/transaction.rs
@@ -2,11 +2,11 @@
 
 // Copyright (C) 2025 Google LLC.
 
-use core::sync::atomic::{AtomicBool, Ordering};
 use kernel::{
     prelude::*,
     seq_file::SeqFile,
     seq_print,
+    sync::atomic::{ordering::Relaxed, Atomic},
     sync::{Arc, SpinLock},
     task::Kuid,
     time::{Instant, Monotonic},
@@ -33,7 +33,7 @@ pub(crate) struct Transaction {
     pub(crate) to: Arc<Process>,
     #[pin]
     allocation: SpinLock<Option<Allocation>>,
-    is_outstanding: AtomicBool,
+    is_outstanding: Atomic<bool>,
     code: u32,
     pub(crate) flags: u32,
     data_size: usize,
@@ -105,7 +105,7 @@ pub(crate) fn new(
             offsets_size: trd.offsets_size as _,
             data_address,
             allocation <- kernel::new_spinlock!(Some(alloc.success()), "Transaction::new"),
-            is_outstanding: AtomicBool::new(false),
+            is_outstanding: Atomic::new(false),
             txn_security_ctx_off,
             oneway_spam_detected,
             start_time: Instant::now(),
@@ -145,7 +145,7 @@ pub(crate) fn new_reply(
             offsets_size: trd.offsets_size as _,
             data_address: alloc.ptr,
             allocation <- kernel::new_spinlock!(Some(alloc.success()), "Transaction::new"),
-            is_outstanding: AtomicBool::new(false),
+            is_outstanding: Atomic::new(false),
             txn_security_ctx_off: None,
             oneway_spam_detected,
             start_time: Instant::now(),
@@ -215,8 +215,8 @@ pub(crate) fn find_from(&self, thread: &Thread) -> Option<&DArc<Transaction>> {
 
     pub(crate) fn set_outstanding(&self, to_process: &mut ProcessInner) {
         // No race because this method is only called once.
-        if !self.is_outstanding.load(Ordering::Relaxed) {
-            self.is_outstanding.store(true, Ordering::Relaxed);
+        if !self.is_outstanding.load(Relaxed) {
+            self.is_outstanding.store(true, Relaxed);
             to_process.add_outstanding_txn();
         }
     }
@@ -227,8 +227,8 @@ fn drop_outstanding_txn(&self) {
         // destructor, which is guaranteed to not race with any other operations on the
         // transaction. It also cannot race with `set_outstanding`, since submission happens
         // before delivery.
-        if self.is_outstanding.load(Ordering::Relaxed) {
-            self.is_outstanding.store(false, Ordering::Relaxed);
+        if self.is_outstanding.load(Relaxed) {
+            self.is_outstanding.store(false, Relaxed);
             self.to.drop_outstanding_txn();
         }
     }
-- 
2.43.0

Re: [PATCH v1 1/3] rust: revocable: Switch to kernel::sync atomic primitives

[Boqun Feng]

On Tue, Dec 30, 2025 at 06:37:16PM +0900, FUJITA Tomonori wrote:
> Convert uses of `AtomicBool` to `Atomic<bool>`.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/revocable.rs | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
> index 0f4ae673256d..47cd2b45eada 100644
> --- a/rust/kernel/revocable.rs
> +++ b/rust/kernel/revocable.rs
> @@ -7,12 +7,20 @@
>  
>  use pin_init::Wrapper;
>  
> -use crate::{bindings, prelude::*, sync::rcu, types::Opaque};
> +use crate::{
> +    bindings,
> +    prelude::*,
> +    sync::atomic::{
> +        ordering::Relaxed,
> +        Atomic, //
> +    },
> +    sync::rcu,
> +    types::Opaque, //
> +};
>  use core::{
>      marker::PhantomData,
>      ops::Deref,
> -    ptr::drop_in_place,
> -    sync::atomic::{AtomicBool, Ordering},
> +    ptr::drop_in_place, //
>  };
>  
>  /// An object that can become inaccessible at runtime.
> @@ -65,7 +73,7 @@
>  /// ```
>  #[pin_data(PinnedDrop)]
>  pub struct Revocable<T> {
> -    is_available: AtomicBool,
> +    is_available: Atomic<bool>,
>      #[pin]
>      data: Opaque<T>,

I actually think we should use Atomic<i32> instead of Atomic<bool> here,
because in most of the cases `T` is a aligned to at least 4-bytes (e.g.
Devres uses a Revocable<IoMem>) And in that case, using Atomic<bool>
doesn't save extra memory but makes the xchg() operation slower on
architectures that don't support byte-wise RmW instructions (e.g.
riscv).

My rule of thumb for using Atomic<bool> is: when you really care about
the memory usage, and you can guarantee using Atomic<bool> is saving
memory. For a general "flag" usage, Atomic<bool> is not strictly better
than Atomic<i32> in most cases imo.

Regards,
Boqun

>  }
> @@ -84,7 +92,7 @@ impl<T> Revocable<T> {
>      /// Creates a new revocable instance of the given data.
>      pub fn new<E>(data: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>          try_pin_init!(Self {
> -            is_available: AtomicBool::new(true),
> +            is_available: Atomic::new(true),
>              data <- Opaque::pin_init(data),
>          }? E)
>      }
> @@ -98,7 +106,7 @@ pub fn new<E>(data: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>      /// because another CPU may be waiting to complete the revocation of this object.
>      pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
>          let guard = rcu::read_lock();
> -        if self.is_available.load(Ordering::Relaxed) {
> +        if self.is_available.load(Relaxed) {
>              // Since `self.is_available` is true, data is initialised and has to remain valid
>              // because the RCU read side lock prevents it from being dropped.
>              Some(RevocableGuard::new(self.data.get(), guard))
> @@ -116,7 +124,7 @@ pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
>      /// allowed to sleep because another CPU may be waiting to complete the revocation of this
>      /// object.
>      pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a T> {
> -        if self.is_available.load(Ordering::Relaxed) {
> +        if self.is_available.load(Relaxed) {
>              // SAFETY: Since `self.is_available` is true, data is initialised and has to remain
>              // valid because the RCU read side lock prevents it from being dropped.
>              Some(unsafe { &*self.data.get() })
> @@ -157,7 +165,7 @@ pub unsafe fn access(&self) -> &T {
>      ///
>      /// Callers must ensure that there are no more concurrent users of the revocable object.
>      unsafe fn revoke_internal<const SYNC: bool>(&self) -> bool {
> -        let revoke = self.is_available.swap(false, Ordering::Relaxed);
> +        let revoke = self.is_available.xchg(false, Relaxed);
>  
>          if revoke {
>              if SYNC {
> -- 
> 2.43.0
> 

Re: [PATCH v1 1/3] rust: revocable: Switch to kernel::sync atomic primitives

[FUJITA Tomonori]

On Wed, 31 Dec 2025 07:29:17 +0800
Boqun Feng <boqun.feng@gmail.com> wrote:

>>  /// An object that can become inaccessible at runtime.
>> @@ -65,7 +73,7 @@
>>  /// ```
>>  #[pin_data(PinnedDrop)]
>>  pub struct Revocable<T> {
>> -    is_available: AtomicBool,
>> +    is_available: Atomic<bool>,
>>      #[pin]
>>      data: Opaque<T>,
> 
> I actually think we should use Atomic<i32> instead of Atomic<bool> here,
> because in most of the cases `T` is a aligned to at least 4-bytes (e.g.
> Devres uses a Revocable<IoMem>) And in that case, using Atomic<bool>
> doesn't save extra memory but makes the xchg() operation slower on
> architectures that don't support byte-wise RmW instructions (e.g.
> riscv).

I agree. Recent RISC-V support byte-wise swap instructions so this may
not appliy to them. Among the architectures currently supporting Rust,
perhaps only loongarch could have this issue.


> My rule of thumb for using Atomic<bool> is: when you really care about
> the memory usage, and you can guarantee using Atomic<bool> is saving
> memory. For a general "flag" usage, Atomic<bool> is not strictly better
> than Atomic<i32> in most cases imo.

I agree again.

Maybe we should provide an enum type with i32 representation to
express boolean values like Atomic<Flag>? That way, users could avoid
writing unsafe code:

unsafe impl AtomicType for Flag {
    type Repr = i32;
}

Re: [PATCH v1 1/3] rust: revocable: Switch to kernel::sync atomic primitives

[Alice Ryhl]

On Wed, Dec 31, 2025 at 07:29:17AM +0800, Boqun Feng wrote:
> On Tue, Dec 30, 2025 at 06:37:16PM +0900, FUJITA Tomonori wrote:
> > Convert uses of `AtomicBool` to `Atomic<bool>`.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> > ---
> >  rust/kernel/revocable.rs | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
> > index 0f4ae673256d..47cd2b45eada 100644
> > --- a/rust/kernel/revocable.rs
> > +++ b/rust/kernel/revocable.rs
> > @@ -7,12 +7,20 @@
> >  
> >  use pin_init::Wrapper;
> >  
> > -use crate::{bindings, prelude::*, sync::rcu, types::Opaque};
> > +use crate::{
> > +    bindings,
> > +    prelude::*,
> > +    sync::atomic::{
> > +        ordering::Relaxed,
> > +        Atomic, //
> > +    },
> > +    sync::rcu,
> > +    types::Opaque, //
> > +};
> >  use core::{
> >      marker::PhantomData,
> >      ops::Deref,
> > -    ptr::drop_in_place,
> > -    sync::atomic::{AtomicBool, Ordering},
> > +    ptr::drop_in_place, //
> >  };
> >  
> >  /// An object that can become inaccessible at runtime.
> > @@ -65,7 +73,7 @@
> >  /// ```
> >  #[pin_data(PinnedDrop)]
> >  pub struct Revocable<T> {
> > -    is_available: AtomicBool,
> > +    is_available: Atomic<bool>,
> >      #[pin]
> >      data: Opaque<T>,
> 
> I actually think we should use Atomic<i32> instead of Atomic<bool> here,
> because in most of the cases `T` is a aligned to at least 4-bytes (e.g.
> Devres uses a Revocable<IoMem>) And in that case, using Atomic<bool>
> doesn't save extra memory but makes the xchg() operation slower on
> architectures that don't support byte-wise RmW instructions (e.g.
> riscv).
> 
> My rule of thumb for using Atomic<bool> is: when you really care about
> the memory usage, and you can guarantee using Atomic<bool> is saving
> memory. For a general "flag" usage, Atomic<bool> is not strictly better
> than Atomic<i32> in most cases imo.

In Binder I believe we only use load/store with Atomic<bool>. Does it
matter in that case?

Alice

Re: [PATCH v1 1/3] rust: revocable: Switch to kernel::sync atomic primitives

[FUJITA Tomonori]

On Wed, 31 Dec 2025 10:12:07 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> On Wed, Dec 31, 2025 at 07:29:17AM +0800, Boqun Feng wrote:
>> On Tue, Dec 30, 2025 at 06:37:16PM +0900, FUJITA Tomonori wrote:
>> > Convert uses of `AtomicBool` to `Atomic<bool>`.
>> > 
>> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> > ---
>> >  rust/kernel/revocable.rs | 24 ++++++++++++++++--------
>> >  1 file changed, 16 insertions(+), 8 deletions(-)
>> > 
>> > diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
>> > index 0f4ae673256d..47cd2b45eada 100644
>> > --- a/rust/kernel/revocable.rs
>> > +++ b/rust/kernel/revocable.rs
>> > @@ -7,12 +7,20 @@
>> >  
>> >  use pin_init::Wrapper;
>> >  
>> > -use crate::{bindings, prelude::*, sync::rcu, types::Opaque};
>> > +use crate::{
>> > +    bindings,
>> > +    prelude::*,
>> > +    sync::atomic::{
>> > +        ordering::Relaxed,
>> > +        Atomic, //
>> > +    },
>> > +    sync::rcu,
>> > +    types::Opaque, //
>> > +};
>> >  use core::{
>> >      marker::PhantomData,
>> >      ops::Deref,
>> > -    ptr::drop_in_place,
>> > -    sync::atomic::{AtomicBool, Ordering},
>> > +    ptr::drop_in_place, //
>> >  };
>> >  
>> >  /// An object that can become inaccessible at runtime.
>> > @@ -65,7 +73,7 @@
>> >  /// ```
>> >  #[pin_data(PinnedDrop)]
>> >  pub struct Revocable<T> {
>> > -    is_available: AtomicBool,
>> > +    is_available: Atomic<bool>,
>> >      #[pin]
>> >      data: Opaque<T>,
>> 
>> I actually think we should use Atomic<i32> instead of Atomic<bool> here,
>> because in most of the cases `T` is a aligned to at least 4-bytes (e.g.
>> Devres uses a Revocable<IoMem>) And in that case, using Atomic<bool>
>> doesn't save extra memory but makes the xchg() operation slower on
>> architectures that don't support byte-wise RmW instructions (e.g.
>> riscv).
>> 
>> My rule of thumb for using Atomic<bool> is: when you really care about
>> the memory usage, and you can guarantee using Atomic<bool> is saving
>> memory. For a general "flag" usage, Atomic<bool> is not strictly better
>> than Atomic<i32> in most cases imo.
> 
> In Binder I believe we only use load/store with Atomic<bool>. Does it
> matter in that case?

In the case of Binder, since it only use load and store with
Atomic<bool>, the performance issue with xchg doesn't apply.

The concern is specifically about atomic read-modify-write operations
like xchg and cmpxchg, not simple atomic loads or stores.

Re: [PATCH v1 3/3] rust_binder: Switch to kernel::sync atomic primitives

[Alice Ryhl]

On Tue, Dec 30, 2025 at 06:37:18PM +0900, FUJITA Tomonori wrote:
> Convert uses of AtomicBool, AtomicUsize, and AtomicU32.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Thanks

Acked-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v1 2/3] rust: list: Switch to kernel::sync atomic primitives

[Alice Ryhl]

On Tue, Dec 30, 2025 at 06:37:17PM +0900, FUJITA Tomonori wrote:
> Convert uses of `AtomicBool` to `Atomic<bool>`.
> 
> Note that the compare_exchange migration simplifies to
> `try_cmpxchg()`, since `try_cmpxchg()` provides relaxed ordering on
> failure, making the explicit failure ordering unnecessary.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Thanks.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v1 1/3] rust: revocable: Switch to kernel::sync atomic primitives

[Alice Ryhl]

On Tue, Dec 30, 2025 at 06:37:16PM +0900, FUJITA Tomonori wrote:
> Convert uses of `AtomicBool` to `Atomic<bool>`.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

With the comment about i32 vs bool addressed:
Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v1 1/3] rust: revocable: Switch to kernel::sync atomic primitives

[Boqun Feng]

On Wed, Dec 31, 2025 at 06:35:02PM +0900, FUJITA Tomonori wrote:
> On Wed, 31 Dec 2025 07:29:17 +0800
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> >>  /// An object that can become inaccessible at runtime.
> >> @@ -65,7 +73,7 @@
> >>  /// ```
> >>  #[pin_data(PinnedDrop)]
> >>  pub struct Revocable<T> {
> >> -    is_available: AtomicBool,
> >> +    is_available: Atomic<bool>,
> >>      #[pin]
> >>      data: Opaque<T>,
> > 
> > I actually think we should use Atomic<i32> instead of Atomic<bool> here,
> > because in most of the cases `T` is a aligned to at least 4-bytes (e.g.
> > Devres uses a Revocable<IoMem>) And in that case, using Atomic<bool>
> > doesn't save extra memory but makes the xchg() operation slower on
> > architectures that don't support byte-wise RmW instructions (e.g.
> > riscv).
> 
> I agree. Recent RISC-V support byte-wise swap instructions so this may
> not appliy to them. Among the architectures currently supporting Rust,
> perhaps only loongarch could have this issue.
> 
> 
> > My rule of thumb for using Atomic<bool> is: when you really care about
> > the memory usage, and you can guarantee using Atomic<bool> is saving
> > memory. For a general "flag" usage, Atomic<bool> is not strictly better
> > than Atomic<i32> in most cases imo.
> 
> I agree again.
> 
> Maybe we should provide an enum type with i32 representation to
> express boolean values like Atomic<Flag>? That way, users could avoid

Yes, that sounds good to me.

Regards,
Boqun

> writing unsafe code:
> 
> unsafe impl AtomicType for Flag {
>     type Repr = i32;
> }
> 

Re: [PATCH v1 1/3] rust: revocable: Switch to kernel::sync atomic primitives

[Gary Guo]

On Wed, 31 Dec 2025 18:35:02 +0900 (JST)
FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:

> On Wed, 31 Dec 2025 07:29:17 +0800
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> >>  /// An object that can become inaccessible at runtime.
> >> @@ -65,7 +73,7 @@
> >>  /// ```
> >>  #[pin_data(PinnedDrop)]
> >>  pub struct Revocable<T> {
> >> -    is_available: AtomicBool,
> >> +    is_available: Atomic<bool>,
> >>      #[pin]
> >>      data: Opaque<T>,  
> > 
> > I actually think we should use Atomic<i32> instead of Atomic<bool> here,
> > because in most of the cases `T` is a aligned to at least 4-bytes (e.g.
> > Devres uses a Revocable<IoMem>) And in that case, using Atomic<bool>
> > doesn't save extra memory but makes the xchg() operation slower on
> > architectures that don't support byte-wise RmW instructions (e.g.
> > riscv).  
> 
> I agree. Recent RISC-V support byte-wise swap instructions so this may
> not appliy to them. Among the architectures currently supporting Rust,
> perhaps only loongarch could have this issue.

The narrower atomic instructions are extensions and not part of the base
ISA. RISC-V arch use alternative mechanisms for this, but using the
mechanism has an overhead, let alone the hardwares that do not have the
extensions.

Best,
Gary

Re: [PATCH v1 0/3] rust: Switch to kernel::sync atomic primitives

[Boqun Feng]

On Tue, Dec 30, 2025 at 06:37:15PM +0900, FUJITA Tomonori wrote:
> Convert uses of core::sync::atomic to Rust LKMM atomics.
> 
> Each patch can be applied independently, but all of them depend on the
> atomic bool series.
> 
> https://lore.kernel.org/all/20251230045028.1773445-1-fujita.tomonori@gmail.com/
> 
> 
> FUJITA Tomonori (3):
>   rust: revocable: Switch to kernel::sync atomic primitives
>   rust: list: Switch to kernel::sync atomic primitives
>   rust_binder: Switch to kernel::sync atomic primitives
> 

I queued patch #2 and #3 with tags from Alice, thanks!

Regards,
Boqun

>  drivers/android/binder/rust_binder_main.rs | 20 ++++++++----------
>  drivers/android/binder/stats.rs            |  8 ++++----
>  drivers/android/binder/thread.rs           | 24 ++++++++++------------
>  drivers/android/binder/transaction.rs      | 16 +++++++--------
>  rust/kernel/list/arc.rs                    | 14 ++++++-------
>  rust/kernel/revocable.rs                   | 24 ++++++++++++++--------
>  6 files changed, 54 insertions(+), 52 deletions(-)
> 
> 
> base-commit: d4cd35bacb06d53acf39ff248ea2b3eef50573f6
> -- 
> 2.43.0
>