* [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