Re: [PATCH v3 1/2] rust: sync: atomic: Add i32-backed Flag for atomic booleans

["Gary Guo" <gary@garyguo.net>]

On Mon Jan 12, 2026 at 10:30 PM GMT, FUJITA Tomonori wrote:
> On Mon, 12 Jan 2026 09:18:32 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
>
>> On Sun, Jan 11, 2026 at 02:05:57PM +0900, FUJITA Tomonori wrote:
>>> Add a new Flag enum (Clear/Set) and implement AtomicType for it, so
>>> users can use Atomic<Flag> for boolean flags.
>>> 
>>> The backing integer type is an implementation detail; it is currently
>>> i32 but may change in the future.
>>> 
>>> Document when Atomic<Flag> 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 <fujita.tomonori@gmail.com>
>>> ---
>>>  rust/kernel/sync/atomic.rs | 56 ++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 56 insertions(+)
>>> 
>>> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
>>> index ca9cab77abf0..473d3c07e234 100644
>>> --- a/rust/kernel/sync/atomic.rs
>>> +++ b/rust/kernel/sync/atomic.rs
>>> @@ -566,3 +566,59 @@ pub fn fetch_add<Rhs, Ordering: ordering::Ordering>(&self, v: Rhs, _: Ordering)
>>>          unsafe { from_repr(ret) }
>>>      }
>>>  }
>>> +
>>> +/// An atomic flag type intended to be backed by a performance-optimal integer type.
>>> +///
>>> +/// The backing integer type is an implementation detail; it is currently [`i32`] but may change
>>> +/// in the future.
>> 
>> How is it, could we make it u8 on some architectures without penalty,
>> and have it be i32 on others? Or is i32 better always.
>
> Does the following look reasonable?
>
> #[derive(Clone, Copy, PartialEq, Eq)]
> #[cfg_attr(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64), repr(i8))]
> #[cfg_attr(
>     not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64)),
>     repr(i32)
> )]

The arch list looks correct to me.

> pub enum Flag {
>     /// The flag is clear.
>     Clear = 0,
>     /// The flag is set.
>     Set = 1,
> }
>
> #[cfg(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64))]
> type FlagRepr = i8;
> #[cfg(not(any(CONFIG_X86_64, CONFIG_UML, CONFIG_ARM, CONFIG_ARM64)))]
> type FlagRepr = i32;

This doesn't need an extra typedef. You can just put `#[cfg]` on the associate
item below.

Best,
Gary

>
> unsafe impl AtomicType for Flag {
>     type Repr = FlagRepr;
> }