rust: sync: Add AtomicFlag type
Subsystems (3)
cover rust: sync: Add AtomicFlag type
From: FUJITA Tomonori <[email protected]>
This series adds AtomicFlag and switches the list atomic tracker to use it.
Unlike the previous design, we avoid exposing Atomic<Flag> and always use AtomicFlag.
FUJITA Tomonori (2):
rust: sync: atomic: Add perfromance-optimal Flag type for atomic
booleans
rust: list: Use AtomicFlag in AtomicTracker
rust/kernel/list/arc.rs | 8 +- rust/kernel/sync/atomic.rs | 121 +++++++++++++++++++++++++++ rust/kernel/sync/atomic/predefine.rs | 17 ++++ 3 files changed, 142 insertions(+), 4 deletions(-)
base-commit: 6583920e15fc567109e1c64ca58c917f52f40736
Signature · 1 line
2.43.0
PATCH 1/2 rust: sync: atomic: Add perfromance-optimal Flag type for atomic booleans
From: FUJITA Tomonori <[email protected]>
Add AtomicFlag type for boolean flags.
Document when AtomicFlag is generally preferable to Atomic<bool>: in particular, when RMW operations such as xchg()/cmpxchg() may be used and minimizing memory usage is not the top priority. On some architectures without byte-sized RMW instructions, Atomic<bool> can be slower for RMW operations.
--- rust/kernel/sync/atomic.rs | 121 +++++++++++++++++++++++++++ rust/kernel/sync/atomic/predefine.rs | 17 ++++ 2 files changed, 138 insertions(+)
rust/kernel/sync/atomic.rs +121 -0
@@ -560,3 +560,124 @@ pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering)unsafe {from_repr (ret) } } } + +/// # Invariants +/// +/// `padding` must be all zeroes. +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +#[repr( C , align( 4 ))] +#[derive( Clone , Copy )] +struct Flag { + bool_field:bool , + padding: [u8 ;3 ], +} + +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +impl Flag { +#[inline(always)] +const fn new (b:bool ) ->Self { +// INVARIANT: `padding` is all zeroes. +Self { + bool_field: b, + padding: [0 ;3 ], + } + } +} + +// SAFETY: `Flag` and `i32` have the same size and alignment, and it's round-trip +// transmutable to `i32`. +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +unsafe impl AtomicType for Flag { +type Repr =i32 ; +} + +macro_rules! atomic_flag_doc { + () => { +concat! ( +"An atomic flag type intended to be backed by performance-optimal integer type.\n\n" , +"The backing integer type is an implementation detail; it may vary by architecture and change\n" , +"in the future.\n\n" , +"[`AtomicFlag`] is generally preferable to [`Atomic<bool>`] when you need read-modify-write\n" , +"(RMW) operations (e.g. [`Atomic::xchg()`]/[`Atomic::cmpxchg()`]) or when [`Atomic<bool>`] does\n" , +"not save memory due to padding. On some architectures that do not support byte-sized atomic\n" , +"RMW operations, RMW operations on [`Atomic<bool>`] are slower.\n\n" , +"If you only use [`Atomic::load()`]/[`Atomic::store()`], [`Atomic<bool>`] is fine.\n\n" , +"# Examples\n\n" , +"```\n" , +"use kernel::sync::atomic::{Atomic, AtomicFlag, Relaxed};\n\n" , +"let flag = AtomicFlag::new(false);\n" , +"assert_eq!(false, flag.load(Relaxed));\n" , +"flag.store(true, Relaxed);\n" , +"assert_eq!(true, flag.load(Relaxed));\n" , +"```\n" + ) + }; +} + +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +#[doc = atomic_flag_doc! ()] +pub struct AtomicFlag (Atomic <Flag >); + +#[cfg(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 ))] +#[doc = atomic_flag_doc! ()] +pub type AtomicFlag =Atomic <bool >; + +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +impl AtomicFlag { +/// Creates a new atomic flag. +#[inline(always)] +pub const fn new (b:bool ) ->Self { +Self (Atomic ::new (Flag ::new (b))) + } + +/// 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. +/// +/// # Examples +/// +/// ``` +/// use kernel::sync::atomic::{AtomicFlag, Relaxed}; +/// +/// let mut atomic_flag = AtomicFlag::new(false); +/// assert_eq!(false, atomic_flag.load(Relaxed)); +/// *atomic_flag.get_mut() = true; +/// assert_eq!(true, atomic_flag.load(Relaxed)); +/// ``` +#[inline(always)] +pub fn get_mut (&mut self) -> &mut bool { + &mut self.0 .get_mut ().bool_field + } + +/// Loads the value from the atomic flag. +#[inline(always)] +pub fn load <Ordering : ordering::AcquireOrRelaxed >(&self, o:Ordering ) ->bool { + self.0 .load (o).bool_field + } + +/// Stores a value to the atomic flag. +#[inline(always)] +pub fn store <Ordering : ordering::ReleaseOrRelaxed >(&self, v:bool , o:Ordering ) { + self.0 .store (Flag ::new (v), o); + } + +/// Stores a value to the atomic flag and returns the previous value. +#[inline(always)] +pub fn xchg <Ordering : ordering::Ordering >(&self, new:bool , o:Ordering ) ->bool { + self.0 .xchg (Flag ::new (new), o).bool_field + } + +/// Store a value to the atomic flag if the current value is equal to `old`. +#[inline(always)] +pub fn cmpxchg <Ordering : ordering::Ordering >( + &self, + old:bool , + new:bool , + o:Ordering , + ) ->Result <bool ,bool > { +match self.0 .cmpxchg (Flag ::new (old),Flag ::new (new), o) { +Ok (_) =>Ok (old), +Err (f) =>Err (f.bool_field), + } + } +}
rust/kernel/sync/atomic/predefine.rs +17 -0
@@ -215,4 +215,21 @@ fn atomic_bool_tests() {assert_eq! (false , x.load (Relaxed ));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 )); + } }
Signature · 1 line
2.43.0
Gary Guo
maintainerRe: [PATCH v1 1/2] rust: sync: atomic: Add perfromance-optimal Flag type for atomic booleans
Quoted 21 lines · FUJITA Tomonori
From: FUJITA Tomonori <[email protected]>
Add AtomicFlag type for boolean flags.
Document when AtomicFlag is generally preferable to Atomic<bool>: in particular, when RMW operations such as xchg()/cmpxchg() may be used and minimizing memory usage is not the top priority. On some architectures without byte-sized RMW instructions, Atomic<bool> can be slower for RMW operations.
Signed-off-by FUJITA Tomonori <[email protected]>--- rust/kernel/sync/atomic.rs | 121 +++++++++++++++++++++++++++ rust/kernel/sync/atomic/predefine.rs | 17 ++++ 2 files changed, 138 insertions(+)rust/kernel/sync/atomic.rs +0
-0
rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 567
Quoted 8 lines
rust/kernel/sync/atomic.rs +5
-0@@ -560,3 +560,124 @@ pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering)unsafe {from_repr (ret) } } } + +/// # Invariants +/// +/// `padding` must be all zeroes. +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))]
This config repeats too much.
I think probably we should just not let AtomicFlag alias Atomic<u8> (this has the benefit of creating a type mismatch, so code cannot rely on this on x86 and fail to compile on, say, RV).
This way the struct Flag, struct AtomicFlag and impl AtomicFlag would always exist and the config is only needed for much fewer times (plus, you don't need to macro to avoid duplicating docs).
rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 572
Quoted 5 lines
rust/kernel/sync/atomic.rs +5
-0+#[repr( C , align( 4 ))] +#[derive( Clone , Copy )] +struct Flag { + bool_field:bool , + padding: [u8 ;3 ],
You probably still want, on big endian platforms, put padding first, so the generated instruction uses small immediates (0 & 1) instead of 0 & 0x01000000.
Best,
Gary
Quoted 137 lines
rust/kernel/sync/atomic.rs +111
-0+} + +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +impl Flag { +#[inline(always)] +const fn new (b:bool ) ->Self { +// INVARIANT: `padding` is all zeroes. +Self { + bool_field: b, + padding: [0 ;3 ], + } + } +} + +// SAFETY: `Flag` and `i32` have the same size and alignment, and it's round-trip +// transmutable to `i32`. +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +unsafe impl AtomicType for Flag { +type Repr =i32 ; +} + +macro_rules! atomic_flag_doc { + () => { +concat! ( +"An atomic flag type intended to be backed by performance-optimal integer type.\n\n" , +"The backing integer type is an implementation detail; it may vary by architecture and change\n" , +"in the future.\n\n" , +"[`AtomicFlag`] is generally preferable to [`Atomic<bool>`] when you need read-modify-write\n" , +"(RMW) operations (e.g. [`Atomic::xchg()`]/[`Atomic::cmpxchg()`]) or when [`Atomic<bool>`] does\n" , +"not save memory due to padding. On some architectures that do not support byte-sized atomic\n" , +"RMW operations, RMW operations on [`Atomic<bool>`] are slower.\n\n" , +"If you only use [`Atomic::load()`]/[`Atomic::store()`], [`Atomic<bool>`] is fine.\n\n" , +"# Examples\n\n" , +"```\n" , +"use kernel::sync::atomic::{Atomic, AtomicFlag, Relaxed};\n\n" , +"let flag = AtomicFlag::new(false);\n" , +"assert_eq!(false, flag.load(Relaxed));\n" , +"flag.store(true, Relaxed);\n" , +"assert_eq!(true, flag.load(Relaxed));\n" , +"```\n" + ) + }; +} + +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +#[doc = atomic_flag_doc! ()] +pub struct AtomicFlag (Atomic <Flag >); + +#[cfg(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 ))] +#[doc = atomic_flag_doc! ()] +pub type AtomicFlag =Atomic <bool >; + +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +impl AtomicFlag { +/// Creates a new atomic flag. +#[inline(always)] +pub const fn new (b:bool ) ->Self { +Self (Atomic ::new (Flag ::new (b))) + } + +/// 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. +/// +/// # Examples +/// +/// ``` +/// use kernel::sync::atomic::{AtomicFlag, Relaxed}; +/// +/// let mut atomic_flag = AtomicFlag::new(false); +/// assert_eq!(false, atomic_flag.load(Relaxed)); +/// *atomic_flag.get_mut() = true; +/// assert_eq!(true, atomic_flag.load(Relaxed)); +/// ``` +#[inline(always)] +pub fn get_mut (&mut self) -> &mut bool { + &mut self.0 .get_mut ().bool_field + } + +/// Loads the value from the atomic flag. +#[inline(always)] +pub fn load <Ordering : ordering::AcquireOrRelaxed >(&self, o:Ordering ) ->bool { + self.0 .load (o).bool_field + } + +/// Stores a value to the atomic flag. +#[inline(always)] +pub fn store <Ordering : ordering::ReleaseOrRelaxed >(&self, v:bool , o:Ordering ) { + self.0 .store (Flag ::new (v), o); + } + +/// Stores a value to the atomic flag and returns the previous value. +#[inline(always)] +pub fn xchg <Ordering : ordering::Ordering >(&self, new:bool , o:Ordering ) ->bool { + self.0 .xchg (Flag ::new (new), o).bool_field + } + +/// Store a value to the atomic flag if the current value is equal to `old`. +#[inline(always)] +pub fn cmpxchg <Ordering : ordering::Ordering >( + &self, + old:bool , + new:bool , + o:Ordering , + ) ->Result <bool ,bool > { +match self.0 .cmpxchg (Flag ::new (old),Flag ::new (new), o) { +Ok (_) =>Ok (old), +Err (f) =>Err (f.bool_field), + } + } +}rust/kernel/sync/atomic/predefine.rs +17
-0@@ -215,4 +215,21 @@ fn atomic_bool_tests() {assert_eq! (false , x.load (Relaxed ));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 )); + } }
Boqun Feng
maintainerRe: [PATCH v1 1/2] rust: sync: atomic: Add perfromance-optimal Flag type for atomic booleans
Quoted 33 lines · Gary Guo
Quoted 21 lines · FUJITA Tomonori
From: FUJITA Tomonori <[email protected]>
Add AtomicFlag type for boolean flags.
Document when AtomicFlag is generally preferable to Atomic<bool>: in particular, when RMW operations such as xchg()/cmpxchg() may be used and minimizing memory usage is not the top priority. On some architectures without byte-sized RMW instructions, Atomic<bool> can be slower for RMW operations.
Signed-off-by FUJITA Tomonori <[email protected]>--- rust/kernel/sync/atomic.rs | 121 +++++++++++++++++++++++++++ rust/kernel/sync/atomic/predefine.rs | 17 ++++ 2 files changed, 138 insertions(+)rust/kernel/sync/atomic.rs +0
-0rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 567
Quoted 8 lines
rust/kernel/sync/atomic.rs +5
-0@@ -560,3 +560,124 @@ pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering)unsafe {from_repr (ret) } } } + +/// # Invariants +/// +/// `padding` must be all zeroes. +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] This config repeats too much.
Quoted 8 lines
I think probably we should just not let
AtomicFlagaliasAtomic<u8>(this has the benefit of creating a type mismatch, so code cannot rely on this on x86 and fail to compile on, say, RV).This way the
struct Flag,struct AtomicFlagandimpl AtomicFlagwould always exist and the config is only needed for much fewer times (plus, you don't need to macro to avoid duplicating docs).
You probably still need configs for #[repr(align(_))] in that case or two definitions of Flag anyway. but yes the duplicate docs can be avoided, so are some impl blocks.
BTW, while we are at it, maybe we should use arches that don't support byte-wise atomic instructions here instead of the ones do, i.e.
#[cfg(not(any(CONFIG_RISCV, CONFIG_LOONGARCH)))]
#[repr(C)]
#[derive(Clone, Copy)]
struct Flag {
bool_flag: bool,
}
#[cfg(any(CONFIG_RISCV, CONFIG_LOONGARCH))]
#[repr(C, align(4)]
#[derive(Clone, Copy)]
struct Flag {
#[cfg(target_endian = "big")]
padding: [u8; 3],
bool_flag: bool,
#[cfg(target_endian = "little")]
padding: [u8; 3],
}and we should do
unsafe impl AtomicType for Flag {
#[cfg(any(CONFIG_RISCV, CONFIG_LOONGARCH)))]
type Repr = i32;
#[cfg(not(any(CONFIG_RISCV, CONFIG_LOONGARCH)))]
type Repr = i8;
}as well.
Quoted 87 lines
rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 572
Quoted 5 lines
rust/kernel/sync/atomic.rs +5
-0+#[repr( C , align( 4 ))] +#[derive( Clone , Copy )] +struct Flag { + bool_field:bool , + padding: [u8 ;3 ],You probably still want, on big endian platforms, put padding first, so the generated instruction uses small immediates (0 & 1) instead of 0 &
0x01000000.Best,
GaryQuoted 75 lines
rust/kernel/sync/atomic.rs +75
-0+} + +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +impl Flag { +#[inline(always)] +const fn new (b:bool ) ->Self { +// INVARIANT: `padding` is all zeroes. +Self { + bool_field: b, + padding: [0 ;3 ], + } + } +} + +// SAFETY: `Flag` and `i32` have the same size and alignment, and it's round-trip +// transmutable to `i32`. +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +unsafe impl AtomicType for Flag { +type Repr =i32 ; +} + +macro_rules! atomic_flag_doc { + () => { +concat! ( +"An atomic flag type intended to be backed by performance-optimal integer type.\n\n" , +"The backing integer type is an implementation detail; it may vary by architecture and change\n" , +"in the future.\n\n" , +"[`AtomicFlag`] is generally preferable to [`Atomic<bool>`] when you need read-modify-write\n" , +"(RMW) operations (e.g. [`Atomic::xchg()`]/[`Atomic::cmpxchg()`]) or when [`Atomic<bool>`] does\n" , +"not save memory due to padding. On some architectures that do not support byte-sized atomic\n" , +"RMW operations, RMW operations on [`Atomic<bool>`] are slower.\n\n" , +"If you only use [`Atomic::load()`]/[`Atomic::store()`], [`Atomic<bool>`] is fine.\n\n" , +"# Examples\n\n" , +"```\n" , +"use kernel::sync::atomic::{Atomic, AtomicFlag, Relaxed};\n\n" , +"let flag = AtomicFlag::new(false);\n" , +"assert_eq!(false, flag.load(Relaxed));\n" , +"flag.store(true, Relaxed);\n" , +"assert_eq!(true, flag.load(Relaxed));\n" , +"```\n" + ) + }; +} + +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +#[doc = atomic_flag_doc! ()] +pub struct AtomicFlag (Atomic <Flag >); + +#[cfg(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 ))] +#[doc = atomic_flag_doc! ()] +pub type AtomicFlag =Atomic <bool >; + +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] +impl AtomicFlag { +/// Creates a new atomic flag. +#[inline(always)] +pub const fn new (b:bool ) ->Self { +Self (Atomic ::new (Flag ::new (b))) + } + +/// 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. +/// +/// # Examples +/// +/// ``` +/// use kernel::sync::atomic::{AtomicFlag, Relaxed}; +/// +/// let mut atomic_flag = AtomicFlag::new(false); +/// assert_eq!(false, atomic_flag.load(Relaxed)); +/// *atomic_flag.get_mut() = true; +/// assert_eq!(true, atomic_flag.load(Relaxed)); +/// ``` +#[inline(always)]
Quoted 8 lines
rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 655
Quoted 8 lines
rust/kernel/sync/atomic.rs +8
-0+pub fn get_mut (&mut self) -> &mut bool { + &mut self.0 .get_mut ().bool_field + } + +/// Loads the value from the atomic flag. +#[inline(always)] +pub fn load <Ordering : ordering::AcquireOrRelaxed >(&self, o:Ordering ) ->bool { + self.0 .load (o).bool_field
For load(), xchg() and cmpxchg(), I think we should not use .bool_field, because we know that padding is always zero, but compilers don't. Using .bool_field will make the compiler generate i32 to i8 or a bit mask instruction to get the boolean. We need to implement From<Flag> for bool and use .into() here.
Regards,
Boqun
Quoted 55 lines
Quoted 54 lines
rust/kernel/sync/atomic.rs +28
-0+ } + +/// Stores a value to the atomic flag. +#[inline(always)] +pub fn store <Ordering : ordering::ReleaseOrRelaxed >(&self, v:bool , o:Ordering ) { + self.0 .store (Flag ::new (v), o); + } + +/// Stores a value to the atomic flag and returns the previous value. +#[inline(always)] +pub fn xchg <Ordering : ordering::Ordering >(&self, new:bool , o:Ordering ) ->bool { + self.0 .xchg (Flag ::new (new), o).bool_field + } + +/// Store a value to the atomic flag if the current value is equal to `old`. +#[inline(always)] +pub fn cmpxchg <Ordering : ordering::Ordering >( + &self, + old:bool , + new:bool , + o:Ordering , + ) ->Result <bool ,bool > { +match self.0 .cmpxchg (Flag ::new (old),Flag ::new (new), o) { +Ok (_) =>Ok (old), +Err (f) =>Err (f.bool_field), + } + } +}rust/kernel/sync/atomic/predefine.rs +17
-0@@ -215,4 +215,21 @@ fn atomic_bool_tests() {assert_eq! (false , x.load (Relaxed ));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 )); + } }
Boqun Feng
maintainerRe: [PATCH v1 1/2] rust: sync: atomic: Add perfromance-optimal Flag type for atomic booleans
On Wed, Jan 28, 2026 at 09:19:14AM -0800, Boqun Feng wrote:
[...]
Quoted 10 lines
Quoted 9 lines
rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 655
Quoted 9 lines
rust/kernel/sync/atomic.rs +9
-0+#[inline(always)] +pub fn get_mut (&mut self) -> &mut bool { + &mut self.0 .get_mut ().bool_field + } + +/// Loads the value from the atomic flag. +#[inline(always)] +pub fn load <Ordering : ordering::AcquireOrRelaxed >(&self, o:Ordering ) ->bool { + self.0 .load (o).bool_field
Quoted 6 lines
For
load(),xchg()andcmpxchg(), I think we should not use.bool_field, because we know thatpaddingis always zero, but compilers don't. Using.bool_fieldwill make the compiler generate i32 to i8 or a bit mask instruction to get the boolean. We need to implementFrom<Flag>forbooland use.into()here.
Scratch this for a moment, I need to do some more investigation.
Regards,
Boqun
Quoted 4 lines
Regards,
Boqun
[..]
Re: [PATCH v1 1/2] rust: sync: atomic: Add perfromance-optimal Flag type for atomic booleans
In reply to Boqun FengQuoted 38 lines · On Wed, 28 Jan 2026 09:19:14 -0800 Boqun Feng <[email protected]> wrote:
Quoted 37 lines · Gary Guo
Quoted 21 lines · FUJITA Tomonori
From: FUJITA Tomonori <[email protected]>
Add AtomicFlag type for boolean flags.
Document when AtomicFlag is generally preferable to Atomic<bool>: in particular, when RMW operations such as xchg()/cmpxchg() may be used and minimizing memory usage is not the top priority. On some architectures without byte-sized RMW instructions, Atomic<bool> can be slower for RMW operations.
Signed-off-by FUJITA Tomonori <[email protected]>--- rust/kernel/sync/atomic.rs | 121 +++++++++++++++++++++++++++ rust/kernel/sync/atomic/predefine.rs | 17 ++++ 2 files changed, 138 insertions(+)rust/kernel/sync/atomic.rs +0
-0rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 567
Quoted 8 lines
rust/kernel/sync/atomic.rs +5
-0@@ -560,3 +560,124 @@ pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering)unsafe {from_repr (ret) } } } + +/// # Invariants +/// +/// `padding` must be all zeroes. +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] This config repeats too much.
I think probably we should just not let
AtomicFlagaliasAtomic<u8>(this has the benefit of creating a type mismatch, so code cannot rely on this on x86 and fail to compile on, say, RV).
Quoted 8 lines
Quoted 4 lines
This way the
struct Flag,struct AtomicFlagandimpl AtomicFlagwould always exist and the config is only needed for much fewer times (plus, you don't need to macro to avoid duplicating docs).You probably still need configs for
#[repr(align(_))]in that case or two definitions ofFlaganyway. but yes the duplicate docs can be avoided, so are some impl blocks.
If we go with cfg_attr, we get something like the followings:
#[cfg_attr(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64), repr(C))] #[cfg_attr(
not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64)),
repr(C, align(4))
)]
#[derive(Clone, Copy)]
struct Flag {
#[cfg(all(
target_endian = "big",
not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64))
))]
padding: [u8; 3],
bool_field: bool,
#[cfg(all(
target_endian = "little",
not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64))
))]
padding: [u8; 3],
}I think that two definitions of Flag is more readable here.
Quoted 23 lines
BTW, while we are at it, maybe we should use arches that don't support byte-wise atomic instructions here instead of the ones do, i.e.
#[cfg(not(any(CONFIG_RISCV, CONFIG_LOONGARCH)))] #[repr(C)] #[derive(Clone, Copy)] struct Flag { bool_flag: bool, } #[cfg(any(CONFIG_RISCV, CONFIG_LOONGARCH))] #[repr(C, align(4)] #[derive(Clone, Copy)] struct Flag { #[cfg(target_endian = "big")] padding: [u8; 3], bool_flag: bool, #[cfg(target_endian = "little")] padding: [u8; 3], }and we should do
Quoted 8 lines
unsafe impl AtomicType for Flag { #[cfg(any(CONFIG_RISCV, CONFIG_LOONGARCH)))] type Repr = i32; #[cfg(not(any(CONFIG_RISCV, CONFIG_LOONGARCH)))] type Repr = i8; }as well.
Is the idea to list only arches without byte-wise atomics so new byte-atomic-capable arches don't require updates?
Boqun Feng
maintainerRe: [PATCH v1 1/2] rust: sync: atomic: Add perfromance-optimal Flag type for atomic booleans
Quoted 72 lines · FUJITA Tomonori
Quoted 38 lines · On Wed, 28 Jan 2026 09:19:14 -0800 Boqun Feng <[email protected]> wrote:
Quoted 37 lines · Gary Guo
Quoted 21 lines · FUJITA Tomonori
From: FUJITA Tomonori <[email protected]>
Add AtomicFlag type for boolean flags.
Document when AtomicFlag is generally preferable to Atomic<bool>: in particular, when RMW operations such as xchg()/cmpxchg() may be used and minimizing memory usage is not the top priority. On some architectures without byte-sized RMW instructions, Atomic<bool> can be slower for RMW operations.
Signed-off-by FUJITA Tomonori <[email protected]>--- rust/kernel/sync/atomic.rs | 121 +++++++++++++++++++++++++++ rust/kernel/sync/atomic/predefine.rs | 17 ++++ 2 files changed, 138 insertions(+)rust/kernel/sync/atomic.rs +0
-0rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 567
Quoted 8 lines
rust/kernel/sync/atomic.rs +5
-0@@ -560,3 +560,124 @@ pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering)unsafe {from_repr (ret) } } } + +/// # Invariants +/// +/// `padding` must be all zeroes. +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] This config repeats too much.
I think probably we should just not let
AtomicFlagaliasAtomic<u8>(this has the benefit of creating a type mismatch, so code cannot rely on this on x86 and fail to compile on, say, RV).Quoted 8 lines
Quoted 4 lines
This way the
struct Flag,struct AtomicFlagandimpl AtomicFlagwould always exist and the config is only needed for much fewer times (plus, you don't need to macro to avoid duplicating docs).You probably still need configs for
#[repr(align(_))]in that case or two definitions ofFlaganyway. but yes the duplicate docs can be avoided, so are some impl blocks.If we go with
cfg_attr, we get something like the followings:#[cfg_attr(any(CONFIG_X86_64,
CONFIG_UML,CONFIG_ARM,CONFIG_ARM64), repr(C))] #[cfg_attr(not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64)), repr(C, align(4)) )] #[derive(Clone, Copy)] struct Flag { #[cfg(all( target_endian = "big", not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64)) ))] padding: [u8; 3], bool_field: bool, #[cfg(all( target_endian = "little", not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64)) ))] padding: [u8; 3], }
Quoted 2 lines
I think that two definitions of
Flagis more readable here.
Agreed ;-)
Quoted 31 lines
Quoted 31 lines
BTW, while we are at it, maybe we should use arches that don't support byte-wise atomic instructions here instead of the ones do, i.e.
#[cfg(not(any(CONFIG_RISCV, CONFIG_LOONGARCH)))] #[repr(C)] #[derive(Clone, Copy)] struct Flag { bool_flag: bool, } #[cfg(any(CONFIG_RISCV, CONFIG_LOONGARCH))] #[repr(C, align(4)] #[derive(Clone, Copy)] struct Flag { #[cfg(target_endian = "big")] padding: [u8; 3], bool_flag: bool, #[cfg(target_endian = "little")] padding: [u8; 3], }and we should do
unsafe impl AtomicType for Flag { #[cfg(any(CONFIG_RISCV, CONFIG_LOONGARCH)))] type Repr = i32; #[cfg(not(any(CONFIG_RISCV, CONFIG_LOONGARCH)))] type Repr = i8; }
Quoted 4 lines
Quoted 1 line
as well.
Is the idea to list only arches without byte-wise atomics so new byte-atomic-capable arches don't require updates?
Yes, but I guess I was wrong here. More archs are actually byte-atomic-incapable, so what you had is making sense. Feel free to ignore that.
Regards,
Boqun
Re: [PATCH v1 1/2] rust: sync: atomic: Add perfromance-optimal Flag type for atomic booleans
In reply to Gary GuoQuoted 33 lines · On Wed, 28 Jan 2026 13:41:41 +0000 "Gary Guo" <[email protected]> wrote:
Quoted 21 lines · FUJITA Tomonori
From: FUJITA Tomonori <[email protected]>
Add AtomicFlag type for boolean flags.
Document when AtomicFlag is generally preferable to Atomic<bool>: in particular, when RMW operations such as xchg()/cmpxchg() may be used and minimizing memory usage is not the top priority. On some architectures without byte-sized RMW instructions, Atomic<bool> can be slower for RMW operations.
Signed-off-by FUJITA Tomonori <[email protected]>--- rust/kernel/sync/atomic.rs | 121 +++++++++++++++++++++++++++ rust/kernel/sync/atomic/predefine.rs | 17 ++++ 2 files changed, 138 insertions(+)rust/kernel/sync/atomic.rs +0
-0rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 567
Quoted 8 lines
rust/kernel/sync/atomic.rs +5
-0@@ -560,3 +560,124 @@ pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering)unsafe {from_repr (ret) } } } + +/// # Invariants +/// +/// `padding` must be all zeroes. +#[cfg(not(any( CONFIG_X86_64 , CONFIG_UML , CONFIG_ARM , CONFIG_ARM64 )))] This config repeats too much.
Quoted 7 lines
I think probably we should just not let
AtomicFlagaliasAtomic<u8>(this has the benefit of creating a type mismatch, so code cannot rely on this on x86 and fail to compile on, say, RV).This way the
struct Flag,struct AtomicFlagandimpl AtomicFlagwould always exist and the config is only needed for much fewer times (plus, you don't need to macro to avoid duplicating docs).
Agreed, I'll do.
Quoted 8 lines
rust/kernel/sync/atomic.rs pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering) · line 572
Quoted 5 lines
rust/kernel/sync/atomic.rs +5
-0+#[repr( C , align( 4 ))] +#[derive( Clone , Copy )] +struct Flag { + bool_field:bool , + padding: [u8 ;3 ],You probably still want, on big endian platforms, put padding first, so the generated instruction uses small immediates (0 & 1) instead of 0 &
0x01000000.
Thanks, I'll handle this in v2.
PATCH 2/2 rust: list: Use AtomicFlag in AtomicTracker
In reply to FUJITA TomonoriFrom: FUJITA Tomonori <[email protected]>
Make AtomicTracker use AtomicFlag instead of Atomic<bool> to avoid slow byte-sized RMWs on architectures that don't support them.
--- rust/kernel/list/arc.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
rust/kernel/list/arc.rs +4 -4
@@ -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 }
Signature · 1 line
2.43.0