public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] rust: add AtomicFlag::get_mut
@ 2026-01-27 12:52 FUJITA Tomonori
  2026-01-27 12:52 ` [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut FUJITA Tomonori
  2026-01-27 12:53 ` [PATCH v2 2/2] rust: list: Use AtomicFlag in AtomicTracker FUJITA Tomonori
  0 siblings, 2 replies; 15+ messages in thread
From: FUJITA Tomonori @ 2026-01-27 12:52 UTC (permalink / raw)
  To: boqun.feng, ojeda, peterz, will
  Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	mark.rutland, tmgross, rust-for-linux, FUJITA Tomonori

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

This series adds AtomicFlag::get_mut to expose a bool API under exclusive
access, and switches the list atomic tracker to use AtomicFlag.

Note that I sent this patchset yesterday, but it didn't seem to be
delivered properly, so I'm resending it via a different mail
server. Hopefully it goes through this time.

v2:
- Add AtomicFlag::get_mut()
- Make AtomicTracker use get_mut()
v1: https://lore.kernel.org/rust-for-linux/20260119231757.3460885-1-fujita.tomonori@gmail.com/

FUJITA Tomonori (2):
  rust: sync: atomic: Add AtomicFlag::get_mut
  rust: list: Use AtomicFlag in AtomicTracker

 rust/kernel/list/arc.rs              |  8 ++++----
 rust/kernel/sync/atomic.rs           | 20 ++++++++++++++++++++
 rust/kernel/sync/atomic/predefine.rs | 17 +++++++++++++++++
 3 files changed, 41 insertions(+), 4 deletions(-)


base-commit: ed45fc4e7712dabca517bee9f73b91e7da0030a6
-- 
2.43.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 12:52 [PATCH v2 0/2] rust: add AtomicFlag::get_mut FUJITA Tomonori
@ 2026-01-27 12:52 ` FUJITA Tomonori
  2026-01-27 13:53   ` Gary Guo
                     ` (2 more replies)
  2026-01-27 12:53 ` [PATCH v2 2/2] rust: list: Use AtomicFlag in AtomicTracker FUJITA Tomonori
  1 sibling, 3 replies; 15+ messages in thread
From: FUJITA Tomonori @ 2026-01-27 12:52 UTC (permalink / raw)
  To: boqun.feng, ojeda, peterz, will
  Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	mark.rutland, tmgross, rust-for-linux, FUJITA Tomonori

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

AtomicFlag exposes a bool API, but it lacks a get_mut() equivalent to
Atomic<T>::get_mut().

Also add kunit tests for AtomicFlag.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/sync/atomic.rs           | 20 ++++++++++++++++++++
 rust/kernel/sync/atomic/predefine.rs | 17 +++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index 6c46335bdb8c..b6c01d9f3a46 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -591,6 +591,26 @@ pub fn store<Ordering: ordering::ReleaseOrRelaxed>(&self, b: bool, o: Ordering)
         self.0.store(b.into(), o)
     }
 
+    /// Returns a mutable reference to the underlying flag as a `bool`.
+    ///
+    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
+    pub fn get_mut(&mut self) -> &mut bool {
+        let byte_ptr = {
+            let ptr = self.0.as_ptr().cast::<u8>();
+            let offset = if cfg!(target_endian = "big") {
+                core::mem::size_of::<Flag>() - 1
+            } else {
+                0
+            };
+
+            // SAFETY: `ptr` is valid for `size_of::<Flag>()` bytes; `offset` selects the LSB.
+            unsafe { ptr.add(offset) }
+        };
+
+        // SAFETY: The LSB holds `0`/`1` for `Flag::Clear/Set`, and `bool` is `i8`-sized/aligned.
+        unsafe { &mut *byte_ptr.cast::<bool>() }
+    }
+
     /// Stores a value to the atomic flag and returns the previous value.
     #[inline(always)]
     pub fn xchg<Ordering: ordering::Ordering>(&self, b: bool, o: Ordering) -> bool {
diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
index 11bc67ab70a3..e413b9e9fe21 100644
--- a/rust/kernel/sync/atomic/predefine.rs
+++ b/rust/kernel/sync/atomic/predefine.rs
@@ -351,6 +351,23 @@ fn atomic_bool_tests() {
         assert_eq!(Ok(false), x.cmpxchg(false, true, Full));
     }
 
+    #[test]
+    fn atomic_flag_tests() {
+        let mut flag = AtomicFlag::new(false);
+
+        assert_eq!(false, flag.load(Relaxed));
+
+        *flag.get_mut() = true;
+        assert_eq!(true, flag.load(Relaxed));
+
+        assert_eq!(true, flag.xchg(false, Relaxed));
+        assert_eq!(false, flag.load(Relaxed));
+
+        *flag.get_mut() = true;
+        assert_eq!(Ok(true), flag.cmpxchg(true, false, Full));
+        assert_eq!(false, flag.load(Relaxed));
+    }
+
     #[test]
     fn atomic_ptr_tests() {
         let mut v = 42;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/2] rust: list: Use AtomicFlag in AtomicTracker
  2026-01-27 12:52 [PATCH v2 0/2] rust: add AtomicFlag::get_mut FUJITA Tomonori
  2026-01-27 12:52 ` [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut FUJITA Tomonori
@ 2026-01-27 12:53 ` FUJITA Tomonori
  2026-01-27 13:54   ` Gary Guo
  1 sibling, 1 reply; 15+ messages in thread
From: FUJITA Tomonori @ 2026-01-27 12:53 UTC (permalink / raw)
  To: boqun.feng, ojeda, peterz, will
  Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	mark.rutland, tmgross, rust-for-linux, FUJITA Tomonori

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

Make AtomicTracker use AtomicFlag instead of Atomic<bool> to avoid
slow byte-sized RMWs on architectures that don't support them.

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

diff --git a/rust/kernel/list/arc.rs b/rust/kernel/list/arc.rs
index 2282f33913ee..5e84f500a3fe 100644
--- a/rust/kernel/list/arc.rs
+++ b/rust/kernel/list/arc.rs
@@ -6,7 +6,7 @@
 
 use crate::alloc::{AllocError, Flags};
 use crate::prelude::*;
-use crate::sync::atomic::{ordering, Atomic};
+use crate::sync::atomic::{ordering, AtomicFlag};
 use crate::sync::{Arc, ArcBorrow, UniqueArc};
 use core::marker::PhantomPinned;
 use core::ops::Deref;
@@ -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: Atomic<bool>,
+    inner: AtomicFlag,
     // 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: Atomic::new(false),
+            inner: AtomicFlag::new(false),
             _pin: PhantomPinned,
         }
     }
 
-    fn project_inner(self: Pin<&mut Self>) -> &mut Atomic<bool> {
+    fn project_inner(self: Pin<&mut Self>) -> &mut AtomicFlag {
         // 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 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 12:52 ` [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut FUJITA Tomonori
@ 2026-01-27 13:53   ` Gary Guo
  2026-01-27 14:32   ` Miguel Ojeda
  2026-01-27 15:35   ` Boqun Feng
  2 siblings, 0 replies; 15+ messages in thread
From: Gary Guo @ 2026-01-27 13:53 UTC (permalink / raw)
  To: FUJITA Tomonori, boqun.feng, ojeda, peterz, will
  Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	mark.rutland, tmgross, rust-for-linux, FUJITA Tomonori

On Tue Jan 27, 2026 at 12:52 PM GMT, FUJITA Tomonori wrote:
> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> AtomicFlag exposes a bool API, but it lacks a get_mut() equivalent to
> Atomic<T>::get_mut().
> 
> Also add kunit tests for AtomicFlag.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/sync/atomic.rs           | 20 ++++++++++++++++++++
>  rust/kernel/sync/atomic/predefine.rs | 17 +++++++++++++++++
>  2 files changed, 37 insertions(+)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] rust: list: Use AtomicFlag in AtomicTracker
  2026-01-27 12:53 ` [PATCH v2 2/2] rust: list: Use AtomicFlag in AtomicTracker FUJITA Tomonori
@ 2026-01-27 13:54   ` Gary Guo
  0 siblings, 0 replies; 15+ messages in thread
From: Gary Guo @ 2026-01-27 13:54 UTC (permalink / raw)
  To: FUJITA Tomonori, boqun.feng, ojeda, peterz, will
  Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	mark.rutland, tmgross, rust-for-linux, FUJITA Tomonori

On Tue Jan 27, 2026 at 12:53 PM GMT, FUJITA Tomonori wrote:
> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> Make AtomicTracker use AtomicFlag instead of Atomic<bool> to avoid
> slow byte-sized RMWs on architectures that don't support them.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

The diff is neat!

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/list/arc.rs | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 12:52 ` [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut FUJITA Tomonori
  2026-01-27 13:53   ` Gary Guo
@ 2026-01-27 14:32   ` Miguel Ojeda
  2026-01-28  3:47     ` FUJITA Tomonori
  2026-01-27 15:35   ` Boqun Feng
  2 siblings, 1 reply; 15+ messages in thread
From: Miguel Ojeda @ 2026-01-27 14:32 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: boqun.feng, ojeda, peterz, will, a.hindborg, aliceryhl, bjorn3_gh,
	dakr, gary, lossin, mark.rutland, tmgross, rust-for-linux,
	FUJITA Tomonori

On Tue, Jan 27, 2026 at 1:53 PM FUJITA Tomonori <tomo@aliasing.net> wrote:
>
> +    /// Returns a mutable reference to the underlying flag as a `bool`.

    [`bool`]

> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
> +    pub fn get_mut(&mut self) -> &mut bool {

Should we give an example here, perhaps promoting (part of) the test below?

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 12:52 ` [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut FUJITA Tomonori
  2026-01-27 13:53   ` Gary Guo
  2026-01-27 14:32   ` Miguel Ojeda
@ 2026-01-27 15:35   ` Boqun Feng
  2026-01-27 15:42     ` Gary Guo
  2 siblings, 1 reply; 15+ messages in thread
From: Boqun Feng @ 2026-01-27 15:35 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: ojeda, peterz, will, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary,
	lossin, mark.rutland, tmgross, rust-for-linux, FUJITA Tomonori

[For some unknown reasons, I cannot send my reply via gmail hence reply
 via kernel.org account, I might switch from gmail later on]

On Tue, Jan 27, 2026 at 09:52:59PM +0900, FUJITA Tomonori wrote:
> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> AtomicFlag exposes a bool API, but it lacks a get_mut() equivalent to
> Atomic<T>::get_mut().
> 
> Also add kunit tests for AtomicFlag.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/sync/atomic.rs           | 20 ++++++++++++++++++++
>  rust/kernel/sync/atomic/predefine.rs | 17 +++++++++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> index 6c46335bdb8c..b6c01d9f3a46 100644
> --- a/rust/kernel/sync/atomic.rs
> +++ b/rust/kernel/sync/atomic.rs
> @@ -591,6 +591,26 @@ pub fn store<Ordering: ordering::ReleaseOrRelaxed>(&self, b: bool, o: Ordering)
>          self.0.store(b.into(), o)
>      }
>  
> +    /// Returns a mutable reference to the underlying flag as a `bool`.
> +    ///
> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
> +    pub fn get_mut(&mut self) -> &mut bool {
> +        let byte_ptr = {
> +            let ptr = self.0.as_ptr().cast::<u8>();
> +            let offset = if cfg!(target_endian = "big") {
> +                core::mem::size_of::<Flag>() - 1
> +            } else {
> +                0
> +            };

The idea is solid, but I want to avoid endian handling in the function,
I would prefer a "struct declaration" solution like:

    #[cfg(target_endian = "big")]
    #[repr(align(4))]
    pub(super) struct FlagInner {
        _pad: [i8; 3],
	bool_field: bool,
    }

    #[cfg(target_endian = "little")]
    #[repr(align(4))]
    struct FlagInner {
	bool_field: bool,
        _pad: [i8; 3],
    }

redefine `Flag` as `BoolFlag`

    #[repr(i32)]
    pub enum BoolFlag {
        Clear = 0,
	Set = 1,
    }

and `Flag` becomes a union of `BoolFlag` and `FlagInner`:

    /// # Invariants
    /// `Flag` is either 0 or 1 in a i32 representation which implies
    /// that `inner` is always valid as long as `_pad` stays 0.
    pub union Flag {
        pub(super) inner: FlagInner,
        pub flag: BoolFlag,
    }

    // can static_assert that `Flag` and `BoolFlag` has the same
    // alignement and size.

then

    impl AtomicFlag {
        pub fn get_mut(&mut self) -> &mut bool {
	    let flag = self.0.get_mut(); // <- &mut Flag

            // INVARIANTS: flag.inner._pad cannot be modified via the
	    // returned reference.
	    // SAFETY: Per type invariants, `flag.inner.bool_field` is
	    // always a valid bool.
	    unsafe { &mut flag.inner.bool_field }
	}
    }

Thoughts?

Regards,
Boqun

> +
> +            // SAFETY: `ptr` is valid for `size_of::<Flag>()` bytes; `offset` selects the LSB.
> +            unsafe { ptr.add(offset) }
> +        };
> +
> +        // SAFETY: The LSB holds `0`/`1` for `Flag::Clear/Set`, and `bool` is `i8`-sized/aligned.
> +        unsafe { &mut *byte_ptr.cast::<bool>() }
> +    }
> +
>      /// Stores a value to the atomic flag and returns the previous value.
>      #[inline(always)]
>      pub fn xchg<Ordering: ordering::Ordering>(&self, b: bool, o: Ordering) -> bool {
> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> index 11bc67ab70a3..e413b9e9fe21 100644
> --- a/rust/kernel/sync/atomic/predefine.rs
> +++ b/rust/kernel/sync/atomic/predefine.rs
> @@ -351,6 +351,23 @@ fn atomic_bool_tests() {
>          assert_eq!(Ok(false), x.cmpxchg(false, true, Full));
>      }
>  
> +    #[test]
> +    fn atomic_flag_tests() {
> +        let mut flag = AtomicFlag::new(false);
> +
> +        assert_eq!(false, flag.load(Relaxed));
> +
> +        *flag.get_mut() = true;
> +        assert_eq!(true, flag.load(Relaxed));
> +
> +        assert_eq!(true, flag.xchg(false, Relaxed));
> +        assert_eq!(false, flag.load(Relaxed));
> +
> +        *flag.get_mut() = true;
> +        assert_eq!(Ok(true), flag.cmpxchg(true, false, Full));
> +        assert_eq!(false, flag.load(Relaxed));
> +    }
> +
>      #[test]
>      fn atomic_ptr_tests() {
>          let mut v = 42;
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 15:35   ` Boqun Feng
@ 2026-01-27 15:42     ` Gary Guo
  2026-01-27 15:59       ` Boqun Feng
  0 siblings, 1 reply; 15+ messages in thread
From: Gary Guo @ 2026-01-27 15:42 UTC (permalink / raw)
  To: Boqun Feng, FUJITA Tomonori
  Cc: ojeda, peterz, will, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary,
	lossin, mark.rutland, tmgross, rust-for-linux, FUJITA Tomonori

On Tue Jan 27, 2026 at 3:35 PM GMT, Boqun Feng wrote:
> [For some unknown reasons, I cannot send my reply via gmail hence reply
>  via kernel.org account, I might switch from gmail later on]
>
> On Tue, Jan 27, 2026 at 09:52:59PM +0900, FUJITA Tomonori wrote:
>> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> 
>> AtomicFlag exposes a bool API, but it lacks a get_mut() equivalent to
>> Atomic<T>::get_mut().
>> 
>> Also add kunit tests for AtomicFlag.
>> 
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> ---
>>  rust/kernel/sync/atomic.rs           | 20 ++++++++++++++++++++
>>  rust/kernel/sync/atomic/predefine.rs | 17 +++++++++++++++++
>>  2 files changed, 37 insertions(+)
>> 
>> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
>> index 6c46335bdb8c..b6c01d9f3a46 100644
>> --- a/rust/kernel/sync/atomic.rs
>> +++ b/rust/kernel/sync/atomic.rs
>> @@ -591,6 +591,26 @@ pub fn store<Ordering: ordering::ReleaseOrRelaxed>(&self, b: bool, o: Ordering)
>>          self.0.store(b.into(), o)
>>      }
>>  
>> +    /// Returns a mutable reference to the underlying flag as a `bool`.
>> +    ///
>> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
>> +    pub fn get_mut(&mut self) -> &mut bool {
>> +        let byte_ptr = {
>> +            let ptr = self.0.as_ptr().cast::<u8>();
>> +            let offset = if cfg!(target_endian = "big") {
>> +                core::mem::size_of::<Flag>() - 1
>> +            } else {
>> +                0
>> +            };
>
> The idea is solid, but I want to avoid endian handling in the function,
> I would prefer a "struct declaration" solution like:
>
>     #[cfg(target_endian = "big")]
>     #[repr(align(4))]
>     pub(super) struct FlagInner {
>         _pad: [i8; 3],
> 	bool_field: bool,
>     }
>
>     #[cfg(target_endian = "little")]
>     #[repr(align(4))]
>     struct FlagInner {
> 	bool_field: bool,
>         _pad: [i8; 3],
>     }
>
> redefine `Flag` as `BoolFlag`
>
>     #[repr(i32)]
>     pub enum BoolFlag {
>         Clear = 0,
> 	Set = 1,
>     }
>
> and `Flag` becomes a union of `BoolFlag` and `FlagInner`:
>
>     /// # Invariants
>     /// `Flag` is either 0 or 1 in a i32 representation which implies
>     /// that `inner` is always valid as long as `_pad` stays 0.
>     pub union Flag {
>         pub(super) inner: FlagInner,
>         pub flag: BoolFlag,
>     }
>
>     // can static_assert that `Flag` and `BoolFlag` has the same
>     // alignement and size.
>
> then
>
>     impl AtomicFlag {
>         pub fn get_mut(&mut self) -> &mut bool {
> 	    let flag = self.0.get_mut(); // <- &mut Flag
>
>             // INVARIANTS: flag.inner._pad cannot be modified via the
> 	    // returned reference.
> 	    // SAFETY: Per type invariants, `flag.inner.bool_field` is
> 	    // always a valid bool.
> 	    unsafe { &mut flag.inner.bool_field }
> 	}
>     }
>
> Thoughts?
>
> Regards,
> Boqun

I think we really need special handling for endianness for this one single
function, so doing all the extra stuff feels really unnecessary.

I prefer Fujita's current version.

Best,
Gary

>
>> +
>> +            // SAFETY: `ptr` is valid for `size_of::<Flag>()` bytes; `offset` selects the LSB.
>> +            unsafe { ptr.add(offset) }
>> +        };
>> +
>> +        // SAFETY: The LSB holds `0`/`1` for `Flag::Clear/Set`, and `bool` is `i8`-sized/aligned.
>> +        unsafe { &mut *byte_ptr.cast::<bool>() }
>> +    }
>> +
>>      /// Stores a value to the atomic flag and returns the previous value.
>>      #[inline(always)]
>>      pub fn xchg<Ordering: ordering::Ordering>(&self, b: bool, o: Ordering) -> bool {
>> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
>> index 11bc67ab70a3..e413b9e9fe21 100644
>> --- a/rust/kernel/sync/atomic/predefine.rs
>> +++ b/rust/kernel/sync/atomic/predefine.rs
>> @@ -351,6 +351,23 @@ fn atomic_bool_tests() {
>>          assert_eq!(Ok(false), x.cmpxchg(false, true, Full));
>>      }
>>  
>> +    #[test]
>> +    fn atomic_flag_tests() {
>> +        let mut flag = AtomicFlag::new(false);
>> +
>> +        assert_eq!(false, flag.load(Relaxed));
>> +
>> +        *flag.get_mut() = true;
>> +        assert_eq!(true, flag.load(Relaxed));
>> +
>> +        assert_eq!(true, flag.xchg(false, Relaxed));
>> +        assert_eq!(false, flag.load(Relaxed));
>> +
>> +        *flag.get_mut() = true;
>> +        assert_eq!(Ok(true), flag.cmpxchg(true, false, Full));
>> +        assert_eq!(false, flag.load(Relaxed));
>> +    }
>> +
>>      #[test]
>>      fn atomic_ptr_tests() {
>>          let mut v = 42;
>> -- 
>> 2.43.0
>> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 15:42     ` Gary Guo
@ 2026-01-27 15:59       ` Boqun Feng
  2026-01-27 16:10         ` Gary Guo
  0 siblings, 1 reply; 15+ messages in thread
From: Boqun Feng @ 2026-01-27 15:59 UTC (permalink / raw)
  To: Gary Guo
  Cc: FUJITA Tomonori, ojeda, peterz, will, a.hindborg, aliceryhl,
	bjorn3_gh, dakr, lossin, mark.rutland, tmgross, rust-for-linux,
	FUJITA Tomonori

On Tue, Jan 27, 2026 at 03:42:42PM +0000, Gary Guo wrote:
> On Tue Jan 27, 2026 at 3:35 PM GMT, Boqun Feng wrote:
> > [For some unknown reasons, I cannot send my reply via gmail hence reply
> >  via kernel.org account, I might switch from gmail later on]
> >
> > On Tue, Jan 27, 2026 at 09:52:59PM +0900, FUJITA Tomonori wrote:
> >> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> >> 
> >> AtomicFlag exposes a bool API, but it lacks a get_mut() equivalent to
> >> Atomic<T>::get_mut().
> >> 
> >> Also add kunit tests for AtomicFlag.
> >> 
> >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> >> ---
> >>  rust/kernel/sync/atomic.rs           | 20 ++++++++++++++++++++
> >>  rust/kernel/sync/atomic/predefine.rs | 17 +++++++++++++++++
> >>  2 files changed, 37 insertions(+)
> >> 
> >> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> >> index 6c46335bdb8c..b6c01d9f3a46 100644
> >> --- a/rust/kernel/sync/atomic.rs
> >> +++ b/rust/kernel/sync/atomic.rs
> >> @@ -591,6 +591,26 @@ pub fn store<Ordering: ordering::ReleaseOrRelaxed>(&self, b: bool, o: Ordering)
> >>          self.0.store(b.into(), o)
> >>      }
> >>  
> >> +    /// Returns a mutable reference to the underlying flag as a `bool`.
> >> +    ///
> >> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
> >> +    pub fn get_mut(&mut self) -> &mut bool {
> >> +        let byte_ptr = {
> >> +            let ptr = self.0.as_ptr().cast::<u8>();
> >> +            let offset = if cfg!(target_endian = "big") {
> >> +                core::mem::size_of::<Flag>() - 1
> >> +            } else {
> >> +                0
> >> +            };
> >
> > The idea is solid, but I want to avoid endian handling in the function,
> > I would prefer a "struct declaration" solution like:
> >
> >     #[cfg(target_endian = "big")]
> >     #[repr(align(4))]
> >     pub(super) struct FlagInner {
> >         _pad: [i8; 3],
> > 	bool_field: bool,
> >     }
> >
> >     #[cfg(target_endian = "little")]
> >     #[repr(align(4))]
> >     struct FlagInner {
> > 	bool_field: bool,
> >         _pad: [i8; 3],
> >     }
> >
> > redefine `Flag` as `BoolFlag`
> >
> >     #[repr(i32)]
> >     pub enum BoolFlag {
> >         Clear = 0,
> > 	Set = 1,
> >     }
> >
> > and `Flag` becomes a union of `BoolFlag` and `FlagInner`:
> >
> >     /// # Invariants
> >     /// `Flag` is either 0 or 1 in a i32 representation which implies
> >     /// that `inner` is always valid as long as `_pad` stays 0.
> >     pub union Flag {
> >         pub(super) inner: FlagInner,
> >         pub flag: BoolFlag,
> >     }
> >
> >     // can static_assert that `Flag` and `BoolFlag` has the same
> >     // alignement and size.
> >
> > then
> >
> >     impl AtomicFlag {
> >         pub fn get_mut(&mut self) -> &mut bool {
> > 	    let flag = self.0.get_mut(); // <- &mut Flag
> >
> >             // INVARIANTS: flag.inner._pad cannot be modified via the
> > 	    // returned reference.
> > 	    // SAFETY: Per type invariants, `flag.inner.bool_field` is
> > 	    // always a valid bool.
> > 	    unsafe { &mut flag.inner.bool_field }
> > 	}
> >     }
> >
> > Thoughts?
> >
> > Regards,
> > Boqun
> 
> I think we really need special handling for endianness for this one single
> function, so doing all the extra stuff feels really unnecessary.
> 

First, this one single function changes the design actually, previously
you can even implement a Flag as:

   enum Flag {
       Clear = 6,
       Set = 7,
   }

and it'll work, that is as long as `Flag` behaves like a bool, it's
fine. But now this function implies there is actually a bool in `Flag`,
which is kinda totally different.

Besides, by using the current implement, we set an example about "how to
do a byte offset in an i32 for different endians", and then if anyone
wanted to do something similar, very likely they would copy-paste and
modify what we have here. The potential tech debts are significant. So I
would like to do it in a right way ("right" is probably subjective, but
it comes from someone who needs to live with the code as a maintainer
;-) and I'm happy to switch to a better way if necessary).

Hope this can explain why I want to do this right now.

Regards,
Boqun

> I prefer Fujita's current version.
> 
> Best,
> Gary
> 
> >
> >> +
> >> +            // SAFETY: `ptr` is valid for `size_of::<Flag>()` bytes; `offset` selects the LSB.
> >> +            unsafe { ptr.add(offset) }
> >> +        };
> >> +
> >> +        // SAFETY: The LSB holds `0`/`1` for `Flag::Clear/Set`, and `bool` is `i8`-sized/aligned.
> >> +        unsafe { &mut *byte_ptr.cast::<bool>() }
> >> +    }
> >> +
> >>      /// Stores a value to the atomic flag and returns the previous value.
> >>      #[inline(always)]
> >>      pub fn xchg<Ordering: ordering::Ordering>(&self, b: bool, o: Ordering) -> bool {
> >> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> >> index 11bc67ab70a3..e413b9e9fe21 100644
> >> --- a/rust/kernel/sync/atomic/predefine.rs
> >> +++ b/rust/kernel/sync/atomic/predefine.rs
> >> @@ -351,6 +351,23 @@ fn atomic_bool_tests() {
> >>          assert_eq!(Ok(false), x.cmpxchg(false, true, Full));
> >>      }
> >>  
> >> +    #[test]
> >> +    fn atomic_flag_tests() {
> >> +        let mut flag = AtomicFlag::new(false);
> >> +
> >> +        assert_eq!(false, flag.load(Relaxed));
> >> +
> >> +        *flag.get_mut() = true;
> >> +        assert_eq!(true, flag.load(Relaxed));
> >> +
> >> +        assert_eq!(true, flag.xchg(false, Relaxed));
> >> +        assert_eq!(false, flag.load(Relaxed));
> >> +
> >> +        *flag.get_mut() = true;
> >> +        assert_eq!(Ok(true), flag.cmpxchg(true, false, Full));
> >> +        assert_eq!(false, flag.load(Relaxed));
> >> +    }
> >> +
> >>      #[test]
> >>      fn atomic_ptr_tests() {
> >>          let mut v = 42;
> >> -- 
> >> 2.43.0
> >> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 15:59       ` Boqun Feng
@ 2026-01-27 16:10         ` Gary Guo
  2026-01-27 16:34           ` Boqun Feng
  0 siblings, 1 reply; 15+ messages in thread
From: Gary Guo @ 2026-01-27 16:10 UTC (permalink / raw)
  To: Boqun Feng, Gary Guo
  Cc: FUJITA Tomonori, ojeda, peterz, will, a.hindborg, aliceryhl,
	bjorn3_gh, dakr, lossin, mark.rutland, tmgross, rust-for-linux,
	FUJITA Tomonori

On Tue Jan 27, 2026 at 3:59 PM GMT, Boqun Feng wrote:
> On Tue, Jan 27, 2026 at 03:42:42PM +0000, Gary Guo wrote:
>> On Tue Jan 27, 2026 at 3:35 PM GMT, Boqun Feng wrote:
>> > [For some unknown reasons, I cannot send my reply via gmail hence reply
>> >  via kernel.org account, I might switch from gmail later on]
>> >
>> > On Tue, Jan 27, 2026 at 09:52:59PM +0900, FUJITA Tomonori wrote:
>> >> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> >> 
>> >> AtomicFlag exposes a bool API, but it lacks a get_mut() equivalent to
>> >> Atomic<T>::get_mut().
>> >> 
>> >> Also add kunit tests for AtomicFlag.
>> >> 
>> >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> >> ---
>> >>  rust/kernel/sync/atomic.rs           | 20 ++++++++++++++++++++
>> >>  rust/kernel/sync/atomic/predefine.rs | 17 +++++++++++++++++
>> >>  2 files changed, 37 insertions(+)
>> >> 
>> >> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
>> >> index 6c46335bdb8c..b6c01d9f3a46 100644
>> >> --- a/rust/kernel/sync/atomic.rs
>> >> +++ b/rust/kernel/sync/atomic.rs
>> >> @@ -591,6 +591,26 @@ pub fn store<Ordering: ordering::ReleaseOrRelaxed>(&self, b: bool, o: Ordering)
>> >>          self.0.store(b.into(), o)
>> >>      }
>> >>  
>> >> +    /// Returns a mutable reference to the underlying flag as a `bool`.
>> >> +    ///
>> >> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
>> >> +    pub fn get_mut(&mut self) -> &mut bool {
>> >> +        let byte_ptr = {
>> >> +            let ptr = self.0.as_ptr().cast::<u8>();
>> >> +            let offset = if cfg!(target_endian = "big") {
>> >> +                core::mem::size_of::<Flag>() - 1
>> >> +            } else {
>> >> +                0
>> >> +            };
>> >
>> > The idea is solid, but I want to avoid endian handling in the function,
>> > I would prefer a "struct declaration" solution like:
>> >
>> >     #[cfg(target_endian = "big")]
>> >     #[repr(align(4))]
>> >     pub(super) struct FlagInner {
>> >         _pad: [i8; 3],
>> > 	bool_field: bool,
>> >     }
>> >
>> >     #[cfg(target_endian = "little")]
>> >     #[repr(align(4))]
>> >     struct FlagInner {
>> > 	bool_field: bool,
>> >         _pad: [i8; 3],
>> >     }
>> >
>> > redefine `Flag` as `BoolFlag`
>> >
>> >     #[repr(i32)]
>> >     pub enum BoolFlag {
>> >         Clear = 0,
>> > 	Set = 1,
>> >     }
>> >
>> > and `Flag` becomes a union of `BoolFlag` and `FlagInner`:
>> >
>> >     /// # Invariants
>> >     /// `Flag` is either 0 or 1 in a i32 representation which implies
>> >     /// that `inner` is always valid as long as `_pad` stays 0.
>> >     pub union Flag {
>> >         pub(super) inner: FlagInner,
>> >         pub flag: BoolFlag,
>> >     }
>> >
>> >     // can static_assert that `Flag` and `BoolFlag` has the same
>> >     // alignement and size.
>> >
>> > then
>> >
>> >     impl AtomicFlag {
>> >         pub fn get_mut(&mut self) -> &mut bool {
>> > 	    let flag = self.0.get_mut(); // <- &mut Flag
>> >
>> >             // INVARIANTS: flag.inner._pad cannot be modified via the
>> > 	    // returned reference.
>> > 	    // SAFETY: Per type invariants, `flag.inner.bool_field` is
>> > 	    // always a valid bool.
>> > 	    unsafe { &mut flag.inner.bool_field }
>> > 	}
>> >     }
>> >
>> > Thoughts?
>> >
>> > Regards,
>> > Boqun
>> 
>> I think we really need special handling for endianness for this one single
>> function, so doing all the extra stuff feels really unnecessary.
>> 
>
> First, this one single function changes the design actually, previously
> you can even implement a Flag as:
>
>    enum Flag {
>        Clear = 6,
>        Set = 7,
>    }
>
> and it'll work, that is as long as `Flag` behaves like a bool, it's
> fine. But now this function implies there is actually a bool in `Flag`,
> which is kinda totally different.
>
> Besides, by using the current implement, we set an example about "how to
> do a byte offset in an i32 for different endians", and then if anyone
> wanted to do something similar, very likely they would copy-paste and
> modify what we have here. The potential tech debts are significant. So I
> would like to do it in a right way ("right" is probably subjective, but
> it comes from someone who needs to live with the code as a maintainer
> ;-) and I'm happy to switch to a better way if necessary).

I think what Fujita has is more "proper". Your approach still have the issue of
requiring a specific ordering of the fields. If this is messed up, then the
entire thing is broken. I.e. the safety proof of `get_mut` depends on the fields
being ordered correctly in `FlagInner`.

If you want to go down this route then I would just scrap `enum Flag` all
together and always define it as struct, with an internal `bool` + 3 bytes of
zero padding. This way we don't even need unsafe for `get_mut`.

Best,
Gary
>
> Hope this can explain why I want to do this right now.
>
> Regards,
> Boqun
>
>> I prefer Fujita's current version.
>> 
>> Best,
>> Gary
>> 
>> >
>> >> +
>> >> +            // SAFETY: `ptr` is valid for `size_of::<Flag>()` bytes; `offset` selects the LSB.
>> >> +            unsafe { ptr.add(offset) }
>> >> +        };
>> >> +
>> >> +        // SAFETY: The LSB holds `0`/`1` for `Flag::Clear/Set`, and `bool` is `i8`-sized/aligned.
>> >> +        unsafe { &mut *byte_ptr.cast::<bool>() }
>> >> +    }
>> >> +
>> >>      /// Stores a value to the atomic flag and returns the previous value.
>> >>      #[inline(always)]
>> >>      pub fn xchg<Ordering: ordering::Ordering>(&self, b: bool, o: Ordering) -> bool {
>> >> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
>> >> index 11bc67ab70a3..e413b9e9fe21 100644
>> >> --- a/rust/kernel/sync/atomic/predefine.rs
>> >> +++ b/rust/kernel/sync/atomic/predefine.rs
>> >> @@ -351,6 +351,23 @@ fn atomic_bool_tests() {
>> >>          assert_eq!(Ok(false), x.cmpxchg(false, true, Full));
>> >>      }
>> >>  
>> >> +    #[test]
>> >> +    fn atomic_flag_tests() {
>> >> +        let mut flag = AtomicFlag::new(false);
>> >> +
>> >> +        assert_eq!(false, flag.load(Relaxed));
>> >> +
>> >> +        *flag.get_mut() = true;
>> >> +        assert_eq!(true, flag.load(Relaxed));
>> >> +
>> >> +        assert_eq!(true, flag.xchg(false, Relaxed));
>> >> +        assert_eq!(false, flag.load(Relaxed));
>> >> +
>> >> +        *flag.get_mut() = true;
>> >> +        assert_eq!(Ok(true), flag.cmpxchg(true, false, Full));
>> >> +        assert_eq!(false, flag.load(Relaxed));
>> >> +    }
>> >> +
>> >>      #[test]
>> >>      fn atomic_ptr_tests() {
>> >>          let mut v = 42;
>> >> -- 
>> >> 2.43.0
>> >> 
>> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 16:10         ` Gary Guo
@ 2026-01-27 16:34           ` Boqun Feng
  2026-01-28  3:32             ` FUJITA Tomonori
  0 siblings, 1 reply; 15+ messages in thread
From: Boqun Feng @ 2026-01-27 16:34 UTC (permalink / raw)
  To: Gary Guo
  Cc: FUJITA Tomonori, ojeda, peterz, will, a.hindborg, aliceryhl,
	bjorn3_gh, dakr, lossin, mark.rutland, tmgross, rust-for-linux,
	FUJITA Tomonori

On Tue, Jan 27, 2026 at 04:10:35PM +0000, Gary Guo wrote:
> On Tue Jan 27, 2026 at 3:59 PM GMT, Boqun Feng wrote:
> > On Tue, Jan 27, 2026 at 03:42:42PM +0000, Gary Guo wrote:
> >> On Tue Jan 27, 2026 at 3:35 PM GMT, Boqun Feng wrote:
> >> > [For some unknown reasons, I cannot send my reply via gmail hence reply
> >> >  via kernel.org account, I might switch from gmail later on]
> >> >
> >> > On Tue, Jan 27, 2026 at 09:52:59PM +0900, FUJITA Tomonori wrote:
> >> >> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> >> >> 
> >> >> AtomicFlag exposes a bool API, but it lacks a get_mut() equivalent to
> >> >> Atomic<T>::get_mut().
> >> >> 
> >> >> Also add kunit tests for AtomicFlag.
> >> >> 
> >> >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> >> >> ---
> >> >>  rust/kernel/sync/atomic.rs           | 20 ++++++++++++++++++++
> >> >>  rust/kernel/sync/atomic/predefine.rs | 17 +++++++++++++++++
> >> >>  2 files changed, 37 insertions(+)
> >> >> 
> >> >> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> >> >> index 6c46335bdb8c..b6c01d9f3a46 100644
> >> >> --- a/rust/kernel/sync/atomic.rs
> >> >> +++ b/rust/kernel/sync/atomic.rs
> >> >> @@ -591,6 +591,26 @@ pub fn store<Ordering: ordering::ReleaseOrRelaxed>(&self, b: bool, o: Ordering)
> >> >>          self.0.store(b.into(), o)
> >> >>      }
> >> >>  
> >> >> +    /// Returns a mutable reference to the underlying flag as a `bool`.
> >> >> +    ///
> >> >> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
> >> >> +    pub fn get_mut(&mut self) -> &mut bool {
> >> >> +        let byte_ptr = {
> >> >> +            let ptr = self.0.as_ptr().cast::<u8>();
> >> >> +            let offset = if cfg!(target_endian = "big") {
> >> >> +                core::mem::size_of::<Flag>() - 1
> >> >> +            } else {
> >> >> +                0
> >> >> +            };
> >> >
> >> > The idea is solid, but I want to avoid endian handling in the function,
> >> > I would prefer a "struct declaration" solution like:
> >> >
> >> >     #[cfg(target_endian = "big")]
> >> >     #[repr(align(4))]
> >> >     pub(super) struct FlagInner {
> >> >         _pad: [i8; 3],
> >> > 	bool_field: bool,
> >> >     }
> >> >
> >> >     #[cfg(target_endian = "little")]
> >> >     #[repr(align(4))]
> >> >     struct FlagInner {
> >> > 	bool_field: bool,
> >> >         _pad: [i8; 3],
> >> >     }
> >> >
> >> > redefine `Flag` as `BoolFlag`
> >> >
> >> >     #[repr(i32)]
> >> >     pub enum BoolFlag {
> >> >         Clear = 0,
> >> > 	Set = 1,
> >> >     }
> >> >
> >> > and `Flag` becomes a union of `BoolFlag` and `FlagInner`:
> >> >
> >> >     /// # Invariants
> >> >     /// `Flag` is either 0 or 1 in a i32 representation which implies
> >> >     /// that `inner` is always valid as long as `_pad` stays 0.
> >> >     pub union Flag {
> >> >         pub(super) inner: FlagInner,
> >> >         pub flag: BoolFlag,
> >> >     }
> >> >
> >> >     // can static_assert that `Flag` and `BoolFlag` has the same
> >> >     // alignement and size.
> >> >
> >> > then
> >> >
> >> >     impl AtomicFlag {
> >> >         pub fn get_mut(&mut self) -> &mut bool {
> >> > 	    let flag = self.0.get_mut(); // <- &mut Flag
> >> >
> >> >             // INVARIANTS: flag.inner._pad cannot be modified via the
> >> > 	    // returned reference.
> >> > 	    // SAFETY: Per type invariants, `flag.inner.bool_field` is
> >> > 	    // always a valid bool.
> >> > 	    unsafe { &mut flag.inner.bool_field }
> >> > 	}
> >> >     }
> >> >
> >> > Thoughts?
> >> >
> >> > Regards,
> >> > Boqun
> >> 
> >> I think we really need special handling for endianness for this one single
> >> function, so doing all the extra stuff feels really unnecessary.
> >> 
> >
> > First, this one single function changes the design actually, previously
> > you can even implement a Flag as:
> >
> >    enum Flag {
> >        Clear = 6,
> >        Set = 7,
> >    }
> >
> > and it'll work, that is as long as `Flag` behaves like a bool, it's
> > fine. But now this function implies there is actually a bool in `Flag`,
> > which is kinda totally different.
> >
> > Besides, by using the current implement, we set an example about "how to
> > do a byte offset in an i32 for different endians", and then if anyone
> > wanted to do something similar, very likely they would copy-paste and
> > modify what we have here. The potential tech debts are significant. So I
> > would like to do it in a right way ("right" is probably subjective, but
> > it comes from someone who needs to live with the code as a maintainer
> > ;-) and I'm happy to switch to a better way if necessary).
> 
> I think what Fujita has is more "proper". Your approach still have the issue of
> requiring a specific ordering of the fields. If this is messed up, then the
> entire thing is broken. I.e. the safety proof of `get_mut` depends on the fields
> being ordered correctly in `FlagInner`.
> 
> If you want to go down this route then I would just scrap `enum Flag` all
> together and always define it as struct, with an internal `bool` + 3 bytes of
> zero padding. This way we don't even need unsafe for `get_mut`.
> 

Hmm.. so like:

    const PAD_SIZE: usize = <3 or 0 depending on ARCHs>

    /// # Invariants
    /// `pad` has to be all zero.
    struct Flag {
        bool_field: bool,
	pad: [i8; PAD_SIZE],
    }

    impl Flag {
        pub const fn set() -> Flag {
	    Self { true, pad: [0; PAD_SIZE] }
	}

        pub const fn clear() -> Flag {
	    Self { false, pad: [0; PAD_SIZE] }
	}
    }

?

Yes, I think it's better ;-)

Also, now given that `AtomicFlag` behaves exactly like a `Atomic<bool>`,
should we do:

    /// `AtomicFlag` documentation here.
    #[cfg(<Arch supports byte-wise atomic>)]
    type AtomicFlag = Atomic<bool>;
    #[cfg(<Arch doesn't supprot byte-wise atomic>)]
    struct AtomicFlag(Atomic<BooleanFlag>);

    // `Flag` doesn't even need to be public.
    #[cfg(<Arch doesn't supprot byte-wise atomic>)]
    struct BooleanFlag { ... }

(I renamed `Flag` -> `BooleanFlag`)

Thoughts? I don't think there is any extra benefit of exposing
`Atomic<BooleanFlag>`.

Regards,
Boqun

> Best,
> Gary
> >
> > Hope this can explain why I want to do this right now.
> >
> > Regards,
> > Boqun
> >
> >> I prefer Fujita's current version.
> >> 
> >> Best,
> >> Gary
> >> 
> >> >
> >> >> +
> >> >> +            // SAFETY: `ptr` is valid for `size_of::<Flag>()` bytes; `offset` selects the LSB.
> >> >> +            unsafe { ptr.add(offset) }
> >> >> +        };
> >> >> +
> >> >> +        // SAFETY: The LSB holds `0`/`1` for `Flag::Clear/Set`, and `bool` is `i8`-sized/aligned.
> >> >> +        unsafe { &mut *byte_ptr.cast::<bool>() }
> >> >> +    }
> >> >> +
> >> >>      /// Stores a value to the atomic flag and returns the previous value.
> >> >>      #[inline(always)]
> >> >>      pub fn xchg<Ordering: ordering::Ordering>(&self, b: bool, o: Ordering) -> bool {
> >> >> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> >> >> index 11bc67ab70a3..e413b9e9fe21 100644
> >> >> --- a/rust/kernel/sync/atomic/predefine.rs
> >> >> +++ b/rust/kernel/sync/atomic/predefine.rs
> >> >> @@ -351,6 +351,23 @@ fn atomic_bool_tests() {
> >> >>          assert_eq!(Ok(false), x.cmpxchg(false, true, Full));
> >> >>      }
> >> >>  
> >> >> +    #[test]
> >> >> +    fn atomic_flag_tests() {
> >> >> +        let mut flag = AtomicFlag::new(false);
> >> >> +
> >> >> +        assert_eq!(false, flag.load(Relaxed));
> >> >> +
> >> >> +        *flag.get_mut() = true;
> >> >> +        assert_eq!(true, flag.load(Relaxed));
> >> >> +
> >> >> +        assert_eq!(true, flag.xchg(false, Relaxed));
> >> >> +        assert_eq!(false, flag.load(Relaxed));
> >> >> +
> >> >> +        *flag.get_mut() = true;
> >> >> +        assert_eq!(Ok(true), flag.cmpxchg(true, false, Full));
> >> >> +        assert_eq!(false, flag.load(Relaxed));
> >> >> +    }
> >> >> +
> >> >>      #[test]
> >> >>      fn atomic_ptr_tests() {
> >> >>          let mut v = 42;
> >> >> -- 
> >> >> 2.43.0
> >> >> 
> >> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 16:34           ` Boqun Feng
@ 2026-01-28  3:32             ` FUJITA Tomonori
  2026-01-28  3:38               ` Boqun Feng
  0 siblings, 1 reply; 15+ messages in thread
From: FUJITA Tomonori @ 2026-01-28  3:32 UTC (permalink / raw)
  To: boqun
  Cc: gary, tomo, ojeda, peterz, will, a.hindborg, aliceryhl, bjorn3_gh,
	dakr, lossin, mark.rutland, tmgross, rust-for-linux,
	fujita.tomonori

On Tue, 27 Jan 2026 08:34:58 -0800
Boqun Feng <boqun@kernel.org> wrote:

> On Tue, Jan 27, 2026 at 04:10:35PM +0000, Gary Guo wrote:
>> On Tue Jan 27, 2026 at 3:59 PM GMT, Boqun Feng wrote:
>> > On Tue, Jan 27, 2026 at 03:42:42PM +0000, Gary Guo wrote:
>> >> On Tue Jan 27, 2026 at 3:35 PM GMT, Boqun Feng wrote:
>> >> > [For some unknown reasons, I cannot send my reply via gmail hence reply
>> >> >  via kernel.org account, I might switch from gmail later on]
>> >> >
>> >> > On Tue, Jan 27, 2026 at 09:52:59PM +0900, FUJITA Tomonori wrote:
>> >> >> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> >> >> 
>> >> >> AtomicFlag exposes a bool API, but it lacks a get_mut() equivalent to
>> >> >> Atomic<T>::get_mut().
>> >> >> 
>> >> >> Also add kunit tests for AtomicFlag.
>> >> >> 
>> >> >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> >> >> ---
>> >> >>  rust/kernel/sync/atomic.rs           | 20 ++++++++++++++++++++
>> >> >>  rust/kernel/sync/atomic/predefine.rs | 17 +++++++++++++++++
>> >> >>  2 files changed, 37 insertions(+)
>> >> >> 
>> >> >> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
>> >> >> index 6c46335bdb8c..b6c01d9f3a46 100644
>> >> >> --- a/rust/kernel/sync/atomic.rs
>> >> >> +++ b/rust/kernel/sync/atomic.rs
>> >> >> @@ -591,6 +591,26 @@ pub fn store<Ordering: ordering::ReleaseOrRelaxed>(&self, b: bool, o: Ordering)
>> >> >>          self.0.store(b.into(), o)
>> >> >>      }
>> >> >>  
>> >> >> +    /// Returns a mutable reference to the underlying flag as a `bool`.
>> >> >> +    ///
>> >> >> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
>> >> >> +    pub fn get_mut(&mut self) -> &mut bool {
>> >> >> +        let byte_ptr = {
>> >> >> +            let ptr = self.0.as_ptr().cast::<u8>();
>> >> >> +            let offset = if cfg!(target_endian = "big") {
>> >> >> +                core::mem::size_of::<Flag>() - 1
>> >> >> +            } else {
>> >> >> +                0
>> >> >> +            };
>> >> >
>> >> > The idea is solid, but I want to avoid endian handling in the function,
>> >> > I would prefer a "struct declaration" solution like:
>> >> >
>> >> >     #[cfg(target_endian = "big")]
>> >> >     #[repr(align(4))]
>> >> >     pub(super) struct FlagInner {
>> >> >         _pad: [i8; 3],
>> >> > 	bool_field: bool,
>> >> >     }
>> >> >
>> >> >     #[cfg(target_endian = "little")]
>> >> >     #[repr(align(4))]
>> >> >     struct FlagInner {
>> >> > 	bool_field: bool,
>> >> >         _pad: [i8; 3],
>> >> >     }
>> >> >
>> >> > redefine `Flag` as `BoolFlag`
>> >> >
>> >> >     #[repr(i32)]
>> >> >     pub enum BoolFlag {
>> >> >         Clear = 0,
>> >> > 	Set = 1,
>> >> >     }
>> >> >
>> >> > and `Flag` becomes a union of `BoolFlag` and `FlagInner`:
>> >> >
>> >> >     /// # Invariants
>> >> >     /// `Flag` is either 0 or 1 in a i32 representation which implies
>> >> >     /// that `inner` is always valid as long as `_pad` stays 0.
>> >> >     pub union Flag {
>> >> >         pub(super) inner: FlagInner,
>> >> >         pub flag: BoolFlag,
>> >> >     }
>> >> >
>> >> >     // can static_assert that `Flag` and `BoolFlag` has the same
>> >> >     // alignement and size.
>> >> >
>> >> > then
>> >> >
>> >> >     impl AtomicFlag {
>> >> >         pub fn get_mut(&mut self) -> &mut bool {
>> >> > 	    let flag = self.0.get_mut(); // <- &mut Flag
>> >> >
>> >> >             // INVARIANTS: flag.inner._pad cannot be modified via the
>> >> > 	    // returned reference.
>> >> > 	    // SAFETY: Per type invariants, `flag.inner.bool_field` is
>> >> > 	    // always a valid bool.
>> >> > 	    unsafe { &mut flag.inner.bool_field }
>> >> > 	}
>> >> >     }
>> >> >
>> >> > Thoughts?
>> >> >
>> >> > Regards,
>> >> > Boqun
>> >> 
>> >> I think we really need special handling for endianness for this one single
>> >> function, so doing all the extra stuff feels really unnecessary.
>> >> 
>> >
>> > First, this one single function changes the design actually, previously
>> > you can even implement a Flag as:
>> >
>> >    enum Flag {
>> >        Clear = 6,
>> >        Set = 7,
>> >    }
>> >
>> > and it'll work, that is as long as `Flag` behaves like a bool, it's
>> > fine. But now this function implies there is actually a bool in `Flag`,
>> > which is kinda totally different.
>> >
>> > Besides, by using the current implement, we set an example about "how to
>> > do a byte offset in an i32 for different endians", and then if anyone
>> > wanted to do something similar, very likely they would copy-paste and
>> > modify what we have here. The potential tech debts are significant. So I
>> > would like to do it in a right way ("right" is probably subjective, but
>> > it comes from someone who needs to live with the code as a maintainer
>> > ;-) and I'm happy to switch to a better way if necessary).
>> 
>> I think what Fujita has is more "proper". Your approach still have the issue of
>> requiring a specific ordering of the fields. If this is messed up, then the
>> entire thing is broken. I.e. the safety proof of `get_mut` depends on the fields
>> being ordered correctly in `FlagInner`.
>> 
>> If you want to go down this route then I would just scrap `enum Flag` all
>> together and always define it as struct, with an internal `bool` + 3 bytes of
>> zero padding. This way we don't even need unsafe for `get_mut`.
>> 
> 
> Hmm.. so like:
> 
>     const PAD_SIZE: usize = <3 or 0 depending on ARCHs>
> 
>     /// # Invariants
>     /// `pad` has to be all zero.
>     struct Flag {
>         bool_field: bool,
> 	pad: [i8; PAD_SIZE],
>     }
> 
>     impl Flag {
>         pub const fn set() -> Flag {
> 	    Self { true, pad: [0; PAD_SIZE] }
> 	}
> 
>         pub const fn clear() -> Flag {
> 	    Self { false, pad: [0; PAD_SIZE] }
> 	}
>     }
> 
> ?
> 
> Yes, I think it's better ;-)
> 
> Also, now given that `AtomicFlag` behaves exactly like a `Atomic<bool>`,
> should we do:
> 
>     /// `AtomicFlag` documentation here.
>     #[cfg(<Arch supports byte-wise atomic>)]
>     type AtomicFlag = Atomic<bool>;
>     #[cfg(<Arch doesn't supprot byte-wise atomic>)]
>     struct AtomicFlag(Atomic<BooleanFlag>);
> 
>     // `Flag` doesn't even need to be public.
>     #[cfg(<Arch doesn't supprot byte-wise atomic>)]
>     struct BooleanFlag { ... }
> 
> (I renamed `Flag` -> `BooleanFlag`)
> 
> Thoughts? I don't think there is any extra benefit of exposing
> `Atomic<BooleanFlag>`.

Agreed.

You prefer the cleaner git history? Then I'll send the next version
for rust-sync-fixes, to implement BooleanFlag (or Flag) and AtomicFlag
from scratch.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-28  3:32             ` FUJITA Tomonori
@ 2026-01-28  3:38               ` Boqun Feng
  0 siblings, 0 replies; 15+ messages in thread
From: Boqun Feng @ 2026-01-28  3:38 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: gary, ojeda, peterz, will, a.hindborg, aliceryhl, bjorn3_gh, dakr,
	lossin, mark.rutland, tmgross, rust-for-linux, fujita.tomonori

On Wed, Jan 28, 2026 at 12:32:12PM +0900, FUJITA Tomonori wrote:
[...]
> >> 
> >> I think what Fujita has is more "proper". Your approach still have the issue of
> >> requiring a specific ordering of the fields. If this is messed up, then the
> >> entire thing is broken. I.e. the safety proof of `get_mut` depends on the fields
> >> being ordered correctly in `FlagInner`.
> >> 
> >> If you want to go down this route then I would just scrap `enum Flag` all
> >> together and always define it as struct, with an internal `bool` + 3 bytes of
> >> zero padding. This way we don't even need unsafe for `get_mut`.
> >> 
> > 
> > Hmm.. so like:
> > 
> >     const PAD_SIZE: usize = <3 or 0 depending on ARCHs>
> > 
> >     /// # Invariants
> >     /// `pad` has to be all zero.
> >     struct Flag {
> >         bool_field: bool,
> > 	pad: [i8; PAD_SIZE],
> >     }
> > 
> >     impl Flag {
> >         pub const fn set() -> Flag {
> > 	    Self { true, pad: [0; PAD_SIZE] }
> > 	}
> > 
> >         pub const fn clear() -> Flag {
> > 	    Self { false, pad: [0; PAD_SIZE] }
> > 	}
> >     }
> > 
> > ?
> > 
> > Yes, I think it's better ;-)
> > 
> > Also, now given that `AtomicFlag` behaves exactly like a `Atomic<bool>`,
> > should we do:
> > 
> >     /// `AtomicFlag` documentation here.
> >     #[cfg(<Arch supports byte-wise atomic>)]
> >     type AtomicFlag = Atomic<bool>;
> >     #[cfg(<Arch doesn't supprot byte-wise atomic>)]
> >     struct AtomicFlag(Atomic<BooleanFlag>);
> > 
> >     // `Flag` doesn't even need to be public.
> >     #[cfg(<Arch doesn't supprot byte-wise atomic>)]
> >     struct BooleanFlag { ... }
> > 
> > (I renamed `Flag` -> `BooleanFlag`)
> > 
> > Thoughts? I don't think there is any extra benefit of exposing
> > `Atomic<BooleanFlag>`.
> 
> Agreed.
> 
> You prefer the cleaner git history? Then I'll send the next version
> for rust-sync-fixes, to implement BooleanFlag (or Flag) and AtomicFlag
> from scratch.
> 

Yes, that'll be great, thank you!

Regards,
Boqun

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-27 14:32   ` Miguel Ojeda
@ 2026-01-28  3:47     ` FUJITA Tomonori
  2026-01-28  3:55       ` Boqun Feng
  0 siblings, 1 reply; 15+ messages in thread
From: FUJITA Tomonori @ 2026-01-28  3:47 UTC (permalink / raw)
  To: miguel.ojeda.sandonis, boqun.feng
  Cc: ojeda, peterz, will, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary,
	lossin, mark.rutland, tmgross, rust-for-linux, fujita.tomonori

On Tue, 27 Jan 2026 15:32:29 +0100
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Tue, Jan 27, 2026 at 1:53 PM FUJITA Tomonori <tomo@aliasing.net> wrote:
>>
>> +    /// Returns a mutable reference to the underlying flag as a `bool`.
> 
>     [`bool`]

Oops, I'll fix.

>> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
>> +    pub fn get_mut(&mut self) -> &mut bool {
> 
> Should we give an example here, perhaps promoting (part of) the test below?

Boqun, you prefer to have the examples of get_mut() for AtomicFlag and
Atomic<T>?


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut
  2026-01-28  3:47     ` FUJITA Tomonori
@ 2026-01-28  3:55       ` Boqun Feng
  0 siblings, 0 replies; 15+ messages in thread
From: Boqun Feng @ 2026-01-28  3:55 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: miguel.ojeda.sandonis, boqun.feng, ojeda, peterz, will,
	a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	mark.rutland, tmgross, rust-for-linux, fujita.tomonori

On Wed, Jan 28, 2026 at 12:47:34PM +0900, FUJITA Tomonori wrote:
> On Tue, 27 Jan 2026 15:32:29 +0100
> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> 
> > On Tue, Jan 27, 2026 at 1:53 PM FUJITA Tomonori <tomo@aliasing.net> wrote:
> >>
> >> +    /// Returns a mutable reference to the underlying flag as a `bool`.
> > 
> >     [`bool`]
> 
> Oops, I'll fix.
> 
> >> +    /// This is safe because the mutable reference of the atomic flag guarantees exclusive access.
> >> +    pub fn get_mut(&mut self) -> &mut bool {
> > 
> > Should we give an example here, perhaps promoting (part of) the test below?
> 
> Boqun, you prefer to have the examples of get_mut() for AtomicFlag and
> Atomic<T>?
> 

Sure, that makes sense to me, thank you!

Regards,
Boqun

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-01-28  3:55 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-27 12:52 [PATCH v2 0/2] rust: add AtomicFlag::get_mut FUJITA Tomonori
2026-01-27 12:52 ` [PATCH v2 1/2] rust: sync: atomic: Add AtomicFlag::get_mut FUJITA Tomonori
2026-01-27 13:53   ` Gary Guo
2026-01-27 14:32   ` Miguel Ojeda
2026-01-28  3:47     ` FUJITA Tomonori
2026-01-28  3:55       ` Boqun Feng
2026-01-27 15:35   ` Boqun Feng
2026-01-27 15:42     ` Gary Guo
2026-01-27 15:59       ` Boqun Feng
2026-01-27 16:10         ` Gary Guo
2026-01-27 16:34           ` Boqun Feng
2026-01-28  3:32             ` FUJITA Tomonori
2026-01-28  3:38               ` Boqun Feng
2026-01-27 12:53 ` [PATCH v2 2/2] rust: list: Use AtomicFlag in AtomicTracker FUJITA Tomonori
2026-01-27 13:54   ` Gary Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox