Re: [PATCH v1 1/2] rust: sync: atomic: Add atomic bool support via i8 representation

[Boqun Feng <boqun.feng@gmail.com>]

On Tue, Dec 30, 2025 at 01:50:27PM +0900, FUJITA Tomonori wrote:
> Add `bool` support, `Atomic<bool>` by using `i8` as its underlying
> representation.
> 
> Rust specifies that `bool` has size 1 and alignment 1 [1], so it
> matches `i8` on layout; keep `static_assert!()` checks to enforce this
> assumption at build time.
> 
> Implement `AtomicImpl` for `bool` under
> `CONFIG_ARCH_SUPPORTS_ATOMIC_RMW`, consistent with the existing
> `i8/i16` gating.
> 
> Document the additional safety requirement for
> `Atomic::<bool>::from_ptr`: only bit patterns 0 (false) and 1 (true)
> are valid.
> 
> Link: https://doc.rust-lang.org/reference/types/boolean.html [1]
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/sync/atomic.rs           |  1 +
>  rust/kernel/sync/atomic/internal.rs  |  8 ++++++++
>  rust/kernel/sync/atomic/predefine.rs | 11 +++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> index 4aebeacb961a..2c998cbd300e 100644
> --- a/rust/kernel/sync/atomic.rs
> +++ b/rust/kernel/sync/atomic.rs
> @@ -158,6 +158,7 @@ pub const fn new(v: T) -> Self {
>      ///
>      /// - `ptr` is aligned to `align_of::<T>()`.
>      /// - `ptr` is valid for reads and writes for `'a`.
> +    /// - If `T` is `bool`, only the bit patterns 0 (`false`) and 1 (`true`) are valid.

This line is unnecessary, since "`ptr` is valid for ..." means `*ptr`
has to have the valid binary representive of `T`.

>      /// - For the duration of `'a`, other accesses to `*ptr` must not cause data races (defined
>      ///   by [`LKMM`]) against atomic operations on the returned reference. Note that if all other
>      ///   accesses are atomic, then this safety requirement is trivially fulfilled.
> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> index 0dac58bca2b3..0e12955082e5 100644
> --- a/rust/kernel/sync/atomic/internal.rs
> +++ b/rust/kernel/sync/atomic/internal.rs
> @@ -16,6 +16,7 @@ pub trait Sealed {}
>  // The C side supports atomic primitives only for `i32` and `i64` (`atomic_t` and `atomic64_t`),
>  // while the Rust side also layers provides atomic support for `i8` and `i16`
>  // on top of lower-level C primitives.
> +impl private::Sealed for bool {}
>  impl private::Sealed for i8 {}
>  impl private::Sealed for i16 {}
>  impl private::Sealed for i32 {}
> @@ -37,6 +38,13 @@ pub trait AtomicImpl: Sized + Send + Copy + private::Sealed {
>      type Delta;
>  }
>  
> +// The current helpers of load/store uses `{WRITE,READ}_ONCE()` hence the atomicity is only
> +// guaranteed against read-modify-write operations if the architecture supports native atomic RmW.
> +#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
> +impl AtomicImpl for bool {
> +    type Delta = Self;
> +}

I don't think you need this impl block.

Regards,
Boqun

> +
>  // The current helpers of load/store uses `{WRITE,READ}_ONCE()` hence the atomicity is only
>  // guaranteed against read-modify-write operations if the architecture supports native atomic RmW.
>  #[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> index 248d26555ccf..3fc99174b086 100644
> --- a/rust/kernel/sync/atomic/predefine.rs
> +++ b/rust/kernel/sync/atomic/predefine.rs
> @@ -5,6 +5,17 @@
>  use crate::static_assert;
>  use core::mem::{align_of, size_of};
>  
> +// Ensure size and alignment requirements are checked.
> +static_assert!(size_of::<bool>() == size_of::<i8>());
> +static_assert!(align_of::<bool>() == align_of::<i8>());
> +
> +// SAFETY: `bool` has the same size and alignment as `i8`, and Rust guarantees that `bool` has
> +// only two valid bit patterns: 0 (false) and 1 (true). Those are valid `i8` values, so `bool` is
> +// round-trip transmutable to `i8`.
> +unsafe impl super::AtomicType for bool {
> +    type Repr = i8;
> +}
> +
>  // SAFETY: `i8` has the same size and alignment with itself, and is round-trip transmutable to
>  // itself.
>  unsafe impl super::AtomicType for i8 {
> -- 
> 2.43.0
>