Re: [PATCH v1 1/2] rust: sync: atomic: Add perfromance-optimal Flag type for atomic booleans

[Boqun Feng <boqun@kernel.org>]

On Thu, Jan 29, 2026 at 08:22:43AM +0900, FUJITA Tomonori wrote:
> On Wed, 28 Jan 2026 09:19:14 -0800
> Boqun Feng <boqun@kernel.org> wrote:
> 
> > On Wed, Jan 28, 2026 at 01:41:41PM +0000, Gary Guo wrote:
> >> On Wed Jan 28, 2026 at 11:51 AM GMT, FUJITA Tomonori wrote:
> >> > From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> >> >
> >> > 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 <fujita.tomonori@gmail.com>
> >> > ---
> >> >  rust/kernel/sync/atomic.rs           | 121 +++++++++++++++++++++++++++
> >> >  rust/kernel/sync/atomic/predefine.rs |  17 ++++
> >> >  2 files changed, 138 insertions(+)
> >> >
> >> > diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> >> > index 4aebeacb961a..7d06193709c0 100644
> >> > --- a/rust/kernel/sync/atomic.rs
> >> > +++ b/rust/kernel/sync/atomic.rs
> >> > @@ -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).
> >> 
> > 
> > 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.
> 
> 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.
> 

Agreed ;-)

> 
> > 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.
> 
> 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