public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/3] rust: Add xchg and cmpxchg support on i8/i16
@ 2025-12-28 12:05 FUJITA Tomonori
  2025-12-28 12:05 ` [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support FUJITA Tomonori
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: FUJITA Tomonori @ 2025-12-28 12:05 UTC (permalink / raw)
  To: boqun.feng, ojeda
  Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin, tmgross,
	acourbot, rust-for-linux, linux-arch

This series extends the Rust atomic internal to support i8 and i16
exchange operations (xchg and cmpxchg), in addition to the existing
i32/i64 support.

The macro structure is specialized around i32/i64 so i8/i16 has a
separate path for Basic operations only.

This series generalizes the existing declare_and_impl_atomic_methods!
macro so it can generate implementations for multiple backends via a
type-to-C-backend mapping, and uses that to add i8/i16 support for
AtomicBasicOps (load/store) and AtomicExchangeOps (xchg/cmpxchg).

With these changes in place, the i8/i16-only
impl_atomic_only_load_and_store_ops macro becomes redundant and is
removed.

AtomicArithmeticOps for i8/i16 isn't addressed here. There is no
corresponding C API for i8/i16 arithmetic atomics. Adding arithmetic
support in Rust would require a Rust-defined implementation that is
consistent with the kernel's atomic semantics and the LKMM
expectations. That direction would move away from the current design
approach of "using LKMM-backed C atomics for Rust code", and I'm not
sure yet whether we should provide i8/i16 arithmetic atomics.

This series depends on the i8/i16 cmpxchg helpers series [1].

[1] https://lore.kernel.org/all/20251227115951.1424458-1-fujita.tomonori@gmail.com/

FUJITA Tomonori (3):
  rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support
  rust: sync: atomic: Remove workaround macro for i8/i16 BasicOps
  rust: sync: atomic: Add i8/i16 xchg and cmpxchg support

 rust/helpers/atomic_ext.c            |  16 ++--
 rust/kernel/sync/atomic/internal.rs  | 131 ++++++++++++++-------------
 rust/kernel/sync/atomic/predefine.rs |   4 +-
 3 files changed, 76 insertions(+), 75 deletions(-)


base-commit: e24011042180d038293182de25eaaa21b8829050
-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support
  2025-12-28 12:05 [PATCH v1 0/3] rust: Add xchg and cmpxchg support on i8/i16 FUJITA Tomonori
@ 2025-12-28 12:05 ` FUJITA Tomonori
  2025-12-29 11:13   ` Boqun Feng
  2025-12-28 12:05 ` [PATCH v1 2/3] rust: sync: atomic: Remove workaround macro for i8/i16 BasicOps FUJITA Tomonori
  2025-12-28 12:05 ` [PATCH v1 3/3] rust: sync: atomic: Add i8/i16 xchg and cmpxchg support FUJITA Tomonori
  2 siblings, 1 reply; 16+ messages in thread
From: FUJITA Tomonori @ 2025-12-28 12:05 UTC (permalink / raw)
  To: boqun.feng, ojeda
  Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin, tmgross,
	acourbot, rust-for-linux, linux-arch

Rework the internal AtomicOps macro plumbing to generate per-type
implementations from a mapping list.

Capture the trait definition once and reuse it for both declaration
and per-type impl expansion to reduce duplication and keep future
extensions simple.

This is a preparatory refactor for enabling i8/i16 atomics cleanly.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/sync/atomic/internal.rs | 85 ++++++++++++++++++++++-------
 1 file changed, 66 insertions(+), 19 deletions(-)

diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
index 51c5750d7986..0634368d10d2 100644
--- a/rust/kernel/sync/atomic/internal.rs
+++ b/rust/kernel/sync/atomic/internal.rs
@@ -169,16 +169,17 @@ fn [< atomic_ $func >]($($arg: $arg_type,)*) $(-> $ret)? {
     }
 }
 
-// Delcares $ops trait with methods and implements the trait for `i32` and `i64`.
-macro_rules! declare_and_impl_atomic_methods {
-    ($(#[$attr:meta])* $pub:vis trait $ops:ident {
-        $(
-            $(#[doc=$doc:expr])*
-            fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
-                $unsafe:tt { bindings::#call($($arg:tt)*) }
-            }
-        )*
-    }) => {
+macro_rules! declare_atomic_ops_trait {
+    (
+        $(#[$attr:meta])* $pub:vis trait $ops:ident {
+            $(
+                $(#[doc=$doc:expr])*
+                fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
+                    $unsafe:tt { bindings::#call($($arg:tt)*) }
+                }
+            )*
+        }
+    ) => {
         $(#[$attr])*
         $pub trait $ops: AtomicImpl {
             $(
@@ -188,21 +189,25 @@ fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
                 );
             )*
         }
+    }
+}
 
-        impl $ops for i32 {
+macro_rules! impl_atomic_ops_for_one {
+    (
+        $ty:ty => $ctype:ident,
+        $(#[$attr:meta])* $pub:vis trait $ops:ident {
             $(
-                impl_atomic_method!(
-                    (atomic) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
-                        $unsafe { call($($arg)*) }
-                    }
-                );
+                $(#[doc=$doc:expr])*
+                fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
+                    $unsafe:tt { bindings::#call($($arg:tt)*) }
+                }
             )*
         }
-
-        impl $ops for i64 {
+    ) => {
+        impl $ops for $ty {
             $(
                 impl_atomic_method!(
-                    (atomic64) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
+                    ($ctype) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
                         $unsafe { call($($arg)*) }
                     }
                 );
@@ -211,7 +216,47 @@ impl $ops for i64 {
     }
 }
 
+// Declares $ops trait with methods and implements the trait.
+macro_rules! declare_and_impl_atomic_methods {
+    (
+        [ $($map:tt)* ]
+        $(#[$attr:meta])* $pub:vis trait $ops:ident { $($body:tt)* }
+    ) => {
+        declare_and_impl_atomic_methods!(
+            @with_ops_def
+            [ $($map)* ]
+            ( $(#[$attr])* $pub trait $ops { $($body)* } )
+        );
+    };
+
+    (@with_ops_def [ $($map:tt)* ] ( $($ops_def:tt)* )) => {
+        declare_atomic_ops_trait!( $($ops_def)* );
+
+        declare_and_impl_atomic_methods!(
+            @munch
+            [ $($map)* ]
+            ( $($ops_def)* )
+        );
+    };
+
+    (@munch [] ( $($ops_def:tt)* )) => {};
+
+    (@munch [ $ty:ty => $ctype:ident $(, $($rest:tt)*)? ] ( $($ops_def:tt)* )) => {
+        impl_atomic_ops_for_one!(
+            $ty => $ctype,
+            $($ops_def)*
+        );
+
+        declare_and_impl_atomic_methods!(
+            @munch
+            [ $($($rest)*)? ]
+            ( $($ops_def)* )
+        );
+    };
+}
+
 declare_and_impl_atomic_methods!(
+    [ i32 => atomic, i64 => atomic64 ]
     /// Basic atomic operations
     pub trait AtomicBasicOps {
         /// Atomic read (load).
@@ -238,6 +283,7 @@ fn set[release](a: &AtomicRepr<Self>, v: Self) {
 // used for now, leaving the existing macros untouched until the overall
 // design requirements are settled.
 declare_and_impl_atomic_methods!(
+    [ i32 => atomic, i64 => atomic64 ]
     /// Exchange and compare-and-exchange atomic operations
     pub trait AtomicExchangeOps {
         /// Atomic exchange.
@@ -265,6 +311,7 @@ fn try_cmpxchg[acquire, release, relaxed](
 );
 
 declare_and_impl_atomic_methods!(
+    [ i32 => atomic, i64 => atomic64 ]
     /// Atomic arithmetic operations
     pub trait AtomicArithmeticOps {
         /// Atomic add (wrapping).
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 2/3] rust: sync: atomic: Remove workaround macro for i8/i16 BasicOps
  2025-12-28 12:05 [PATCH v1 0/3] rust: Add xchg and cmpxchg support on i8/i16 FUJITA Tomonori
  2025-12-28 12:05 ` [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support FUJITA Tomonori
@ 2025-12-28 12:05 ` FUJITA Tomonori
  2025-12-29 11:58   ` Boqun Feng
  2025-12-28 12:05 ` [PATCH v1 3/3] rust: sync: atomic: Add i8/i16 xchg and cmpxchg support FUJITA Tomonori
  2 siblings, 1 reply; 16+ messages in thread
From: FUJITA Tomonori @ 2025-12-28 12:05 UTC (permalink / raw)
  To: boqun.feng, ojeda
  Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin, tmgross,
	acourbot, rust-for-linux, linux-arch

Remove workaround impl_atomic_only_load_and_store_ops macro and use
declare_and_impl_atomic_methods to add AtomicBasicOps support for
i8/i16.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/helpers/atomic_ext.c           | 16 +++++-----
 rust/kernel/sync/atomic/internal.rs | 48 +----------------------------
 2 files changed, 9 insertions(+), 55 deletions(-)

diff --git a/rust/helpers/atomic_ext.c b/rust/helpers/atomic_ext.c
index 3a5ef6bb2776..10733bb4a75e 100644
--- a/rust/helpers/atomic_ext.c
+++ b/rust/helpers/atomic_ext.c
@@ -4,42 +4,42 @@
 #include <asm/rwonce.h>
 #include <linux/atomic.h>
 
-__rust_helper s8 rust_helper_atomic_i8_load(s8 *ptr)
+__rust_helper s8 rust_helper_atomic_i8_read(s8 *ptr)
 {
 	return READ_ONCE(*ptr);
 }
 
-__rust_helper s8 rust_helper_atomic_i8_load_acquire(s8 *ptr)
+__rust_helper s8 rust_helper_atomic_i8_read_acquire(s8 *ptr)
 {
 	return smp_load_acquire(ptr);
 }
 
-__rust_helper s16 rust_helper_atomic_i16_load(s16 *ptr)
+__rust_helper s16 rust_helper_atomic_i16_read(s16 *ptr)
 {
 	return READ_ONCE(*ptr);
 }
 
-__rust_helper s16 rust_helper_atomic_i16_load_acquire(s16 *ptr)
+__rust_helper s16 rust_helper_atomic_i16_read_acquire(s16 *ptr)
 {
 	return smp_load_acquire(ptr);
 }
 
-__rust_helper void rust_helper_atomic_i8_store(s8 *ptr, s8 val)
+__rust_helper void rust_helper_atomic_i8_set(s8 *ptr, s8 val)
 {
 	WRITE_ONCE(*ptr, val);
 }
 
-__rust_helper void rust_helper_atomic_i8_store_release(s8 *ptr, s8 val)
+__rust_helper void rust_helper_atomic_i8_set_release(s8 *ptr, s8 val)
 {
 	smp_store_release(ptr, val);
 }
 
-__rust_helper void rust_helper_atomic_i16_store(s16 *ptr, s16 val)
+__rust_helper void rust_helper_atomic_i16_set(s16 *ptr, s16 val)
 {
 	WRITE_ONCE(*ptr, val);
 }
 
-__rust_helper void rust_helper_atomic_i16_store_release(s16 *ptr, s16 val)
+__rust_helper void rust_helper_atomic_i16_set_release(s16 *ptr, s16 val)
 {
 	smp_store_release(ptr, val);
 }
diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
index 0634368d10d2..1b2a7933bc14 100644
--- a/rust/kernel/sync/atomic/internal.rs
+++ b/rust/kernel/sync/atomic/internal.rs
@@ -256,7 +256,7 @@ macro_rules! declare_and_impl_atomic_methods {
 }
 
 declare_and_impl_atomic_methods!(
-    [ i32 => atomic, i64 => atomic64 ]
+    [ i8 => atomic_i8, i16 => atomic_i16, i32 => atomic, i64 => atomic64 ]
     /// Basic atomic operations
     pub trait AtomicBasicOps {
         /// Atomic read (load).
@@ -273,15 +273,6 @@ fn set[release](a: &AtomicRepr<Self>, v: Self) {
     }
 );
 
-// It is still unclear whether i8/i16 atomics will eventually support
-// the same set of operations as i32/i64, because some architectures
-// do not provide hardware support for the required atomic primitives.
-// Furthermore, supporting Atomic<bool> will require even more
-// significant structural changes.
-//
-// To avoid premature refactoring, a separate macro for i8 and i16 is
-// used for now, leaving the existing macros untouched until the overall
-// design requirements are settled.
 declare_and_impl_atomic_methods!(
     [ i32 => atomic, i64 => atomic64 ]
     /// Exchange and compare-and-exchange atomic operations
@@ -332,40 +323,3 @@ fn fetch_add[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self::Delta) ->
         }
     }
 );
-
-macro_rules! impl_atomic_only_load_and_store_ops {
-    ($($ty:ty),* $(,)?) => {
-        $(
-            impl AtomicBasicOps for $ty {
-                paste! {
-                    #[inline(always)]
-                    fn atomic_read(a: &AtomicRepr<Self>) -> Self {
-                        // SAFETY: `a.as_ptr()` is valid and properly aligned.
-                        unsafe { bindings::[< atomic_ $ty _load >](a.as_ptr().cast()) }
-                    }
-
-                    #[inline(always)]
-                    fn atomic_read_acquire(a: &AtomicRepr<Self>) -> Self {
-                        // SAFETY: `a.as_ptr()` is valid and properly aligned.
-                        unsafe { bindings::[< atomic_ $ty _load_acquire >](a.as_ptr().cast()) }
-                    }
-
-                    // Generate atomic_set and atomic_set_release
-                    #[inline(always)]
-                    fn atomic_set(a: &AtomicRepr<Self>, v: Self) {
-                        // SAFETY: `a.as_ptr()` is valid and properly aligned.
-                        unsafe { bindings::[< atomic_ $ty _store >](a.as_ptr().cast(), v) }
-                    }
-
-                    #[inline(always)]
-                    fn atomic_set_release(a: &AtomicRepr<Self>, v: Self) {
-                        // SAFETY: `a.as_ptr()` is valid and properly aligned.
-                        unsafe { bindings::[< atomic_ $ty _store_release >](a.as_ptr().cast(), v) }
-                    }
-                }
-            }
-        )*
-    };
-}
-
-impl_atomic_only_load_and_store_ops!(i8, i16);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v1 3/3] rust: sync: atomic: Add i8/i16 xchg and cmpxchg support
  2025-12-28 12:05 [PATCH v1 0/3] rust: Add xchg and cmpxchg support on i8/i16 FUJITA Tomonori
  2025-12-28 12:05 ` [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support FUJITA Tomonori
  2025-12-28 12:05 ` [PATCH v1 2/3] rust: sync: atomic: Remove workaround macro for i8/i16 BasicOps FUJITA Tomonori
@ 2025-12-28 12:05 ` FUJITA Tomonori
  2025-12-29 12:27   ` Boqun Feng
  2 siblings, 1 reply; 16+ messages in thread
From: FUJITA Tomonori @ 2025-12-28 12:05 UTC (permalink / raw)
  To: boqun.feng, ojeda
  Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin, tmgross,
	acourbot, rust-for-linux, linux-arch

Add atomic xchg and cmpxchg operation support for i8 and i16 types
with tests.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/sync/atomic/internal.rs  | 2 +-
 rust/kernel/sync/atomic/predefine.rs | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
index 1b2a7933bc14..55a80e22a7a0 100644
--- a/rust/kernel/sync/atomic/internal.rs
+++ b/rust/kernel/sync/atomic/internal.rs
@@ -274,7 +274,7 @@ fn set[release](a: &AtomicRepr<Self>, v: Self) {
 );
 
 declare_and_impl_atomic_methods!(
-    [ i32 => atomic, i64 => atomic64 ]
+    [ i8 => atomic_i8, i16 => atomic_i16, i32 => atomic, i64 => atomic64 ]
     /// Exchange and compare-and-exchange atomic operations
     pub trait AtomicExchangeOps {
         /// Atomic exchange.
diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
index 51e9df0cf56e..248d26555ccf 100644
--- a/rust/kernel/sync/atomic/predefine.rs
+++ b/rust/kernel/sync/atomic/predefine.rs
@@ -149,7 +149,7 @@ fn atomic_acquire_release_tests() {
 
     #[test]
     fn atomic_xchg_tests() {
-        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
+        for_each_type!(42 in [i8, i16, i32, i64, u32, u64, isize, usize] |v| {
             let x = Atomic::new(v);
 
             let old = v;
@@ -162,7 +162,7 @@ fn atomic_xchg_tests() {
 
     #[test]
     fn atomic_cmpxchg_tests() {
-        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
+        for_each_type!(42 in [i8, i16, i32, i64, u32, u64, isize, usize] |v| {
             let x = Atomic::new(v);
 
             let old = v;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support
  2025-12-28 12:05 ` [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support FUJITA Tomonori
@ 2025-12-29 11:13   ` Boqun Feng
  2025-12-29 11:54     ` FUJITA Tomonori
  2025-12-29 16:36     ` Gary Guo
  0 siblings, 2 replies; 16+ messages in thread
From: Boqun Feng @ 2025-12-29 11:13 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	tmgross, acourbot, rust-for-linux, linux-arch

On Sun, Dec 28, 2025 at 09:05:44PM +0900, FUJITA Tomonori wrote:
> Rework the internal AtomicOps macro plumbing to generate per-type
> implementations from a mapping list.
> 
> Capture the trait definition once and reuse it for both declaration
> and per-type impl expansion to reduce duplication and keep future
> extensions simple.
> 
> This is a preparatory refactor for enabling i8/i16 atomics cleanly.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Thanks! I have an idea that uses proc-macro to generate the Atomic*Ops
impls, e.g.

    #[atomic_ops(i8, i16, i32, i64)]
    pub trait AtomicBasicOps {
        #[variant(acquire)]
        fn read(a: &AtomicRepr<Self>) -> Self {
	    unsafe { binding_call!(a.as_ptr().cast()) }
	}
    }

But I think the current solution in your patch suffices as a temporary
solution at least.

Regards,
Boqun

> ---
>  rust/kernel/sync/atomic/internal.rs | 85 ++++++++++++++++++++++-------
>  1 file changed, 66 insertions(+), 19 deletions(-)
> 
> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> index 51c5750d7986..0634368d10d2 100644
> --- a/rust/kernel/sync/atomic/internal.rs
> +++ b/rust/kernel/sync/atomic/internal.rs
> @@ -169,16 +169,17 @@ fn [< atomic_ $func >]($($arg: $arg_type,)*) $(-> $ret)? {
>      }
>  }
>  
> -// Delcares $ops trait with methods and implements the trait for `i32` and `i64`.
> -macro_rules! declare_and_impl_atomic_methods {
> -    ($(#[$attr:meta])* $pub:vis trait $ops:ident {
> -        $(
> -            $(#[doc=$doc:expr])*
> -            fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
> -                $unsafe:tt { bindings::#call($($arg:tt)*) }
> -            }
> -        )*
> -    }) => {
> +macro_rules! declare_atomic_ops_trait {
> +    (
> +        $(#[$attr:meta])* $pub:vis trait $ops:ident {
> +            $(
> +                $(#[doc=$doc:expr])*
> +                fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
> +                    $unsafe:tt { bindings::#call($($arg:tt)*) }
> +                }
> +            )*
> +        }
> +    ) => {
>          $(#[$attr])*
>          $pub trait $ops: AtomicImpl {
>              $(
> @@ -188,21 +189,25 @@ fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
>                  );
>              )*
>          }
> +    }
> +}
>  
> -        impl $ops for i32 {
> +macro_rules! impl_atomic_ops_for_one {
> +    (
> +        $ty:ty => $ctype:ident,
> +        $(#[$attr:meta])* $pub:vis trait $ops:ident {
>              $(
> -                impl_atomic_method!(
> -                    (atomic) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
> -                        $unsafe { call($($arg)*) }
> -                    }
> -                );
> +                $(#[doc=$doc:expr])*
> +                fn $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
> +                    $unsafe:tt { bindings::#call($($arg:tt)*) }
> +                }
>              )*
>          }
> -
> -        impl $ops for i64 {
> +    ) => {
> +        impl $ops for $ty {
>              $(
>                  impl_atomic_method!(
> -                    (atomic64) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
> +                    ($ctype) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
>                          $unsafe { call($($arg)*) }
>                      }
>                  );
> @@ -211,7 +216,47 @@ impl $ops for i64 {
>      }
>  }
>  
> +// Declares $ops trait with methods and implements the trait.
> +macro_rules! declare_and_impl_atomic_methods {
> +    (
> +        [ $($map:tt)* ]
> +        $(#[$attr:meta])* $pub:vis trait $ops:ident { $($body:tt)* }
> +    ) => {
> +        declare_and_impl_atomic_methods!(
> +            @with_ops_def
> +            [ $($map)* ]
> +            ( $(#[$attr])* $pub trait $ops { $($body)* } )
> +        );
> +    };
> +
> +    (@with_ops_def [ $($map:tt)* ] ( $($ops_def:tt)* )) => {
> +        declare_atomic_ops_trait!( $($ops_def)* );
> +
> +        declare_and_impl_atomic_methods!(
> +            @munch
> +            [ $($map)* ]
> +            ( $($ops_def)* )
> +        );
> +    };
> +
> +    (@munch [] ( $($ops_def:tt)* )) => {};
> +
> +    (@munch [ $ty:ty => $ctype:ident $(, $($rest:tt)*)? ] ( $($ops_def:tt)* )) => {
> +        impl_atomic_ops_for_one!(
> +            $ty => $ctype,
> +            $($ops_def)*
> +        );
> +
> +        declare_and_impl_atomic_methods!(
> +            @munch
> +            [ $($($rest)*)? ]
> +            ( $($ops_def)* )
> +        );
> +    };
> +}
> +
>  declare_and_impl_atomic_methods!(
> +    [ i32 => atomic, i64 => atomic64 ]
>      /// Basic atomic operations
>      pub trait AtomicBasicOps {
>          /// Atomic read (load).
> @@ -238,6 +283,7 @@ fn set[release](a: &AtomicRepr<Self>, v: Self) {
>  // used for now, leaving the existing macros untouched until the overall
>  // design requirements are settled.
>  declare_and_impl_atomic_methods!(
> +    [ i32 => atomic, i64 => atomic64 ]
>      /// Exchange and compare-and-exchange atomic operations
>      pub trait AtomicExchangeOps {
>          /// Atomic exchange.
> @@ -265,6 +311,7 @@ fn try_cmpxchg[acquire, release, relaxed](
>  );
>  
>  declare_and_impl_atomic_methods!(
> +    [ i32 => atomic, i64 => atomic64 ]
>      /// Atomic arithmetic operations
>      pub trait AtomicArithmeticOps {
>          /// Atomic add (wrapping).
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support
  2025-12-29 11:13   ` Boqun Feng
@ 2025-12-29 11:54     ` FUJITA Tomonori
  2025-12-29 16:36     ` Gary Guo
  1 sibling, 0 replies; 16+ messages in thread
From: FUJITA Tomonori @ 2025-12-29 11:54 UTC (permalink / raw)
  To: boqun.feng
  Cc: fujita.tomonori, ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr,
	gary, lossin, tmgross, acourbot, rust-for-linux, linux-arch

On Mon, 29 Dec 2025 19:13:11 +0800
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Sun, Dec 28, 2025 at 09:05:44PM +0900, FUJITA Tomonori wrote:
>> Rework the internal AtomicOps macro plumbing to generate per-type
>> implementations from a mapping list.
>> 
>> Capture the trait definition once and reuse it for both declaration
>> and per-type impl expansion to reduce duplication and keep future
>> extensions simple.
>> 
>> This is a preparatory refactor for enabling i8/i16 atomics cleanly.
>> 
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> Thanks! I have an idea that uses proc-macro to generate the Atomic*Ops
> impls, e.g.
> 
>     #[atomic_ops(i8, i16, i32, i64)]
>     pub trait AtomicBasicOps {
>         #[variant(acquire)]
>         fn read(a: &AtomicRepr<Self>) -> Self {
> 	    unsafe { binding_call!(a.as_ptr().cast()) }
> 	}
>     }
> 
> But I think the current solution in your patch suffices as a temporary
> solution at least.

Thanks!

I'll leave the proc-macro approach to you. If this can be merged as a
temporary, incremental solution, that would be great. Once this is in,
I'll send follow-up patches shortly to add Atomic<bool> support and
migrate the remaining core::sync users in the tree.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/3] rust: sync: atomic: Remove workaround macro for i8/i16 BasicOps
  2025-12-28 12:05 ` [PATCH v1 2/3] rust: sync: atomic: Remove workaround macro for i8/i16 BasicOps FUJITA Tomonori
@ 2025-12-29 11:58   ` Boqun Feng
  2025-12-29 12:55     ` FUJITA Tomonori
  0 siblings, 1 reply; 16+ messages in thread
From: Boqun Feng @ 2025-12-29 11:58 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	tmgross, acourbot, rust-for-linux, linux-arch

On Sun, Dec 28, 2025 at 09:05:45PM +0900, FUJITA Tomonori wrote:
> Remove workaround impl_atomic_only_load_and_store_ops macro and use
> declare_and_impl_atomic_methods to add AtomicBasicOps support for
> i8/i16.
> 

I did the following so that we can drop this ;-)

1. Change function names of [1] and [2] from *{load,store}* to
   *{read,set}*.

2. Reorder [3] before [4] to avoid introduction of
   impl_atomic_only_load_and_store_ops!()

[1]: https://lore.kernel.org/all/20251211113826.1299077-3-fujita.tomonori@gmail.com/
[2]: https://lore.kernel.org/all/20251211113826.1299077-2-fujita.tomonori@gmail.com/
[3]: https://lore.kernel.org/all/20251228120546.1602275-2-fujita.tomonori@gmail.com/
[4]: https://lore.kernel.org/all/20251211113826.1299077-4-fujita.tomonori@gmail.com/

I also reorder a bit to make the introduction of helpers are grouped
together, please see at

	https://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git/log/?h=rust-sync.20251229

I feel this way we have a cleaner history of changes.

Thanks!

Regards,
Boqun

> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/helpers/atomic_ext.c           | 16 +++++-----
>  rust/kernel/sync/atomic/internal.rs | 48 +----------------------------
>  2 files changed, 9 insertions(+), 55 deletions(-)
> 
> diff --git a/rust/helpers/atomic_ext.c b/rust/helpers/atomic_ext.c
> index 3a5ef6bb2776..10733bb4a75e 100644
> --- a/rust/helpers/atomic_ext.c
> +++ b/rust/helpers/atomic_ext.c
> @@ -4,42 +4,42 @@
>  #include <asm/rwonce.h>
>  #include <linux/atomic.h>
>  
> -__rust_helper s8 rust_helper_atomic_i8_load(s8 *ptr)
> +__rust_helper s8 rust_helper_atomic_i8_read(s8 *ptr)
>  {
>  	return READ_ONCE(*ptr);
>  }
>  
> -__rust_helper s8 rust_helper_atomic_i8_load_acquire(s8 *ptr)
> +__rust_helper s8 rust_helper_atomic_i8_read_acquire(s8 *ptr)
>  {
>  	return smp_load_acquire(ptr);
>  }
>  
> -__rust_helper s16 rust_helper_atomic_i16_load(s16 *ptr)
> +__rust_helper s16 rust_helper_atomic_i16_read(s16 *ptr)
>  {
>  	return READ_ONCE(*ptr);
>  }
>  
> -__rust_helper s16 rust_helper_atomic_i16_load_acquire(s16 *ptr)
> +__rust_helper s16 rust_helper_atomic_i16_read_acquire(s16 *ptr)
>  {
>  	return smp_load_acquire(ptr);
>  }
>  
> -__rust_helper void rust_helper_atomic_i8_store(s8 *ptr, s8 val)
> +__rust_helper void rust_helper_atomic_i8_set(s8 *ptr, s8 val)
>  {
>  	WRITE_ONCE(*ptr, val);
>  }
>  
> -__rust_helper void rust_helper_atomic_i8_store_release(s8 *ptr, s8 val)
> +__rust_helper void rust_helper_atomic_i8_set_release(s8 *ptr, s8 val)
>  {
>  	smp_store_release(ptr, val);
>  }
>  
> -__rust_helper void rust_helper_atomic_i16_store(s16 *ptr, s16 val)
> +__rust_helper void rust_helper_atomic_i16_set(s16 *ptr, s16 val)
>  {
>  	WRITE_ONCE(*ptr, val);
>  }
>  
> -__rust_helper void rust_helper_atomic_i16_store_release(s16 *ptr, s16 val)
> +__rust_helper void rust_helper_atomic_i16_set_release(s16 *ptr, s16 val)
>  {
>  	smp_store_release(ptr, val);
>  }
> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> index 0634368d10d2..1b2a7933bc14 100644
> --- a/rust/kernel/sync/atomic/internal.rs
> +++ b/rust/kernel/sync/atomic/internal.rs
> @@ -256,7 +256,7 @@ macro_rules! declare_and_impl_atomic_methods {
>  }
>  
>  declare_and_impl_atomic_methods!(
> -    [ i32 => atomic, i64 => atomic64 ]
> +    [ i8 => atomic_i8, i16 => atomic_i16, i32 => atomic, i64 => atomic64 ]
>      /// Basic atomic operations
>      pub trait AtomicBasicOps {
>          /// Atomic read (load).
> @@ -273,15 +273,6 @@ fn set[release](a: &AtomicRepr<Self>, v: Self) {
>      }
>  );
>  
> -// It is still unclear whether i8/i16 atomics will eventually support
> -// the same set of operations as i32/i64, because some architectures
> -// do not provide hardware support for the required atomic primitives.
> -// Furthermore, supporting Atomic<bool> will require even more
> -// significant structural changes.
> -//
> -// To avoid premature refactoring, a separate macro for i8 and i16 is
> -// used for now, leaving the existing macros untouched until the overall
> -// design requirements are settled.
>  declare_and_impl_atomic_methods!(
>      [ i32 => atomic, i64 => atomic64 ]
>      /// Exchange and compare-and-exchange atomic operations
> @@ -332,40 +323,3 @@ fn fetch_add[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self::Delta) ->
>          }
>      }
>  );
> -
> -macro_rules! impl_atomic_only_load_and_store_ops {
> -    ($($ty:ty),* $(,)?) => {
> -        $(
> -            impl AtomicBasicOps for $ty {
> -                paste! {
> -                    #[inline(always)]
> -                    fn atomic_read(a: &AtomicRepr<Self>) -> Self {
> -                        // SAFETY: `a.as_ptr()` is valid and properly aligned.
> -                        unsafe { bindings::[< atomic_ $ty _load >](a.as_ptr().cast()) }
> -                    }
> -
> -                    #[inline(always)]
> -                    fn atomic_read_acquire(a: &AtomicRepr<Self>) -> Self {
> -                        // SAFETY: `a.as_ptr()` is valid and properly aligned.
> -                        unsafe { bindings::[< atomic_ $ty _load_acquire >](a.as_ptr().cast()) }
> -                    }
> -
> -                    // Generate atomic_set and atomic_set_release
> -                    #[inline(always)]
> -                    fn atomic_set(a: &AtomicRepr<Self>, v: Self) {
> -                        // SAFETY: `a.as_ptr()` is valid and properly aligned.
> -                        unsafe { bindings::[< atomic_ $ty _store >](a.as_ptr().cast(), v) }
> -                    }
> -
> -                    #[inline(always)]
> -                    fn atomic_set_release(a: &AtomicRepr<Self>, v: Self) {
> -                        // SAFETY: `a.as_ptr()` is valid and properly aligned.
> -                        unsafe { bindings::[< atomic_ $ty _store_release >](a.as_ptr().cast(), v) }
> -                    }
> -                }
> -            }
> -        )*
> -    };
> -}
> -
> -impl_atomic_only_load_and_store_ops!(i8, i16);
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 3/3] rust: sync: atomic: Add i8/i16 xchg and cmpxchg support
  2025-12-28 12:05 ` [PATCH v1 3/3] rust: sync: atomic: Add i8/i16 xchg and cmpxchg support FUJITA Tomonori
@ 2025-12-29 12:27   ` Boqun Feng
  2025-12-29 12:30     ` Boqun Feng
  0 siblings, 1 reply; 16+ messages in thread
From: Boqun Feng @ 2025-12-29 12:27 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	tmgross, acourbot, rust-for-linux, linux-arch

On Sun, Dec 28, 2025 at 09:05:46PM +0900, FUJITA Tomonori wrote:
> Add atomic xchg and cmpxchg operation support for i8 and i16 types
> with tests.
> 

I think we also needs the following, otherwise architectures may
accidentally enable Rust but don't have the correct atomic
implementation for i8 and i16.

diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
index 248d26555ccf..a4e5bbd45eb2 100644
--- a/rust/kernel/sync/atomic/predefine.rs
+++ b/rust/kernel/sync/atomic/predefine.rs
@@ -5,14 +5,22 @@
 use crate::static_assert;
 use core::mem::{align_of, size_of};

+// 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.
+//
 // SAFETY: `i8` has the same size and alignment with itself, and is round-trip transmutable to
 // itself.
+#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
 unsafe impl super::AtomicType for i8 {
     type Repr = i8;
 }

+// 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.
+//
 // SAFETY: `i16` has the same size and alignment with itself, and is round-trip transmutable to
 // itself.
+#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
 unsafe impl super::AtomicType for i16 {
     type Repr = i16;
 }

I can fold it into your patch if that works.

Regards,
Boqun

> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/sync/atomic/internal.rs  | 2 +-
>  rust/kernel/sync/atomic/predefine.rs | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> index 1b2a7933bc14..55a80e22a7a0 100644
> --- a/rust/kernel/sync/atomic/internal.rs
> +++ b/rust/kernel/sync/atomic/internal.rs
> @@ -274,7 +274,7 @@ fn set[release](a: &AtomicRepr<Self>, v: Self) {
>  );
>  
>  declare_and_impl_atomic_methods!(
> -    [ i32 => atomic, i64 => atomic64 ]
> +    [ i8 => atomic_i8, i16 => atomic_i16, i32 => atomic, i64 => atomic64 ]
>      /// Exchange and compare-and-exchange atomic operations
>      pub trait AtomicExchangeOps {
>          /// Atomic exchange.
> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> index 51e9df0cf56e..248d26555ccf 100644
> --- a/rust/kernel/sync/atomic/predefine.rs
> +++ b/rust/kernel/sync/atomic/predefine.rs
> @@ -149,7 +149,7 @@ fn atomic_acquire_release_tests() {
>  
>      #[test]
>      fn atomic_xchg_tests() {
> -        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
> +        for_each_type!(42 in [i8, i16, i32, i64, u32, u64, isize, usize] |v| {
>              let x = Atomic::new(v);
>  
>              let old = v;
> @@ -162,7 +162,7 @@ fn atomic_xchg_tests() {
>  
>      #[test]
>      fn atomic_cmpxchg_tests() {
> -        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
> +        for_each_type!(42 in [i8, i16, i32, i64, u32, u64, isize, usize] |v| {
>              let x = Atomic::new(v);
>  
>              let old = v;
> -- 
> 2.43.0
> 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 3/3] rust: sync: atomic: Add i8/i16 xchg and cmpxchg support
  2025-12-29 12:27   ` Boqun Feng
@ 2025-12-29 12:30     ` Boqun Feng
  2025-12-29 13:04       ` FUJITA Tomonori
  0 siblings, 1 reply; 16+ messages in thread
From: Boqun Feng @ 2025-12-29 12:30 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	tmgross, acourbot, rust-for-linux, linux-arch

On Mon, Dec 29, 2025 at 08:27:01PM +0800, Boqun Feng wrote:
> On Sun, Dec 28, 2025 at 09:05:46PM +0900, FUJITA Tomonori wrote:
> > Add atomic xchg and cmpxchg operation support for i8 and i16 types
> > with tests.
> > 
> 
> I think we also needs the following, otherwise architectures may
> accidentally enable Rust but don't have the correct atomic
> implementation for i8 and i16.
> 
> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> index 248d26555ccf..a4e5bbd45eb2 100644
> --- a/rust/kernel/sync/atomic/predefine.rs
> +++ b/rust/kernel/sync/atomic/predefine.rs
> @@ -5,14 +5,22 @@
>  use crate::static_assert;
>  use core::mem::{align_of, size_of};
> 
> +// 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.
> +//
>  // SAFETY: `i8` has the same size and alignment with itself, and is round-trip transmutable to
>  // itself.
> +#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
>  unsafe impl super::AtomicType for i8 {
>      type Repr = i8;
>  }
> 
> +// 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.
> +//
>  // SAFETY: `i16` has the same size and alignment with itself, and is round-trip transmutable to
>  // itself.
> +#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
>  unsafe impl super::AtomicType for i16 {
>      type Repr = i16;
>  }
> 
> I can fold it into your patch if that works.
> 

OK, the right place should be at AtomicImpl instead of AtomicType:

diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
index ac689ce8ee8c..f4760e3a916e 100644
--- a/rust/kernel/sync/atomic/internal.rs
+++ b/rust/kernel/sync/atomic/internal.rs
@@ -37,10 +37,16 @@ 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 i8 {
     type Delta = Self;
 }

+// 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 i16 {
     type Delta = Self;
 }

Regards,
Boqun

> Regards,
> Boqun
> 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> > ---
> >  rust/kernel/sync/atomic/internal.rs  | 2 +-
> >  rust/kernel/sync/atomic/predefine.rs | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> > index 1b2a7933bc14..55a80e22a7a0 100644
> > --- a/rust/kernel/sync/atomic/internal.rs
> > +++ b/rust/kernel/sync/atomic/internal.rs
> > @@ -274,7 +274,7 @@ fn set[release](a: &AtomicRepr<Self>, v: Self) {
> >  );
> >  
> >  declare_and_impl_atomic_methods!(
> > -    [ i32 => atomic, i64 => atomic64 ]
> > +    [ i8 => atomic_i8, i16 => atomic_i16, i32 => atomic, i64 => atomic64 ]
> >      /// Exchange and compare-and-exchange atomic operations
> >      pub trait AtomicExchangeOps {
> >          /// Atomic exchange.
> > diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> > index 51e9df0cf56e..248d26555ccf 100644
> > --- a/rust/kernel/sync/atomic/predefine.rs
> > +++ b/rust/kernel/sync/atomic/predefine.rs
> > @@ -149,7 +149,7 @@ fn atomic_acquire_release_tests() {
> >  
> >      #[test]
> >      fn atomic_xchg_tests() {
> > -        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
> > +        for_each_type!(42 in [i8, i16, i32, i64, u32, u64, isize, usize] |v| {
> >              let x = Atomic::new(v);
> >  
> >              let old = v;
> > @@ -162,7 +162,7 @@ fn atomic_xchg_tests() {
> >  
> >      #[test]
> >      fn atomic_cmpxchg_tests() {
> > -        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
> > +        for_each_type!(42 in [i8, i16, i32, i64, u32, u64, isize, usize] |v| {
> >              let x = Atomic::new(v);
> >  
> >              let old = v;
> > -- 
> > 2.43.0
> > 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/3] rust: sync: atomic: Remove workaround macro for i8/i16 BasicOps
  2025-12-29 11:58   ` Boqun Feng
@ 2025-12-29 12:55     ` FUJITA Tomonori
  2025-12-29 13:19       ` Boqun Feng
  0 siblings, 1 reply; 16+ messages in thread
From: FUJITA Tomonori @ 2025-12-29 12:55 UTC (permalink / raw)
  To: boqun.feng
  Cc: fujita.tomonori, ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr,
	gary, lossin, tmgross, acourbot, rust-for-linux, linux-arch

On Mon, 29 Dec 2025 19:58:48 +0800
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Sun, Dec 28, 2025 at 09:05:45PM +0900, FUJITA Tomonori wrote:
>> Remove workaround impl_atomic_only_load_and_store_ops macro and use
>> declare_and_impl_atomic_methods to add AtomicBasicOps support for
>> i8/i16.
>> 
> 
> I did the following so that we can drop this ;-)
> 
> 1. Change function names of [1] and [2] from *{load,store}* to
>    *{read,set}*.
> 
> 2. Reorder [3] before [4] to avoid introduction of
>    impl_atomic_only_load_and_store_ops!()
> 
> [1]: https://lore.kernel.org/all/20251211113826.1299077-3-fujita.tomonori@gmail.com/
> [2]: https://lore.kernel.org/all/20251211113826.1299077-2-fujita.tomonori@gmail.com/
> [3]: https://lore.kernel.org/all/20251228120546.1602275-2-fujita.tomonori@gmail.com/
> [4]: https://lore.kernel.org/all/20251211113826.1299077-4-fujita.tomonori@gmail.com/
> 
> I also reorder a bit to make the introduction of helpers are grouped
> together, please see at
> 
> 	https://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git/log/?h=rust-sync.20251229
> 
> I feel this way we have a cleaner history of changes.

It looks much cleaner now, thanks!

Maybe, you could consider chan ging the following two points:

- change the first patch subject

rust: helpers: Add i8/i16 atomic_read_acquire/atomic_set_release helpers 

- drop the following comment in 12th patch:

+// It is still unclear whether i8/i16 atomics will eventually support
+// the same set of operations as i32/i64, because some architectures
+// do not provide hardware support for the required atomic primitives.
+// Furthermore, supporting Atomic<bool> will require even more
+// significant structural changes.
+//
+// To avoid premature refactoring, a separate macro for i8 and i16 is
+// used for now, leaving the existing macros untouched until the overall
+// design requirements are settled.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 3/3] rust: sync: atomic: Add i8/i16 xchg and cmpxchg support
  2025-12-29 12:30     ` Boqun Feng
@ 2025-12-29 13:04       ` FUJITA Tomonori
  2025-12-29 13:13         ` Boqun Feng
  0 siblings, 1 reply; 16+ messages in thread
From: FUJITA Tomonori @ 2025-12-29 13:04 UTC (permalink / raw)
  To: boqun.feng
  Cc: fujita.tomonori, ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr,
	gary, lossin, tmgross, acourbot, rust-for-linux, linux-arch

On Mon, 29 Dec 2025 20:30:58 +0800
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Mon, Dec 29, 2025 at 08:27:01PM +0800, Boqun Feng wrote:
>> On Sun, Dec 28, 2025 at 09:05:46PM +0900, FUJITA Tomonori wrote:
>> > Add atomic xchg and cmpxchg operation support for i8 and i16 types
>> > with tests.
>> > 
>> 
>> I think we also needs the following, otherwise architectures may
>> accidentally enable Rust but don't have the correct atomic
>> implementation for i8 and i16.
>> 
>> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
>> index 248d26555ccf..a4e5bbd45eb2 100644
>> --- a/rust/kernel/sync/atomic/predefine.rs
>> +++ b/rust/kernel/sync/atomic/predefine.rs
>> @@ -5,14 +5,22 @@
>>  use crate::static_assert;
>>  use core::mem::{align_of, size_of};
>> 
>> +// 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.
>> +//
>>  // SAFETY: `i8` has the same size and alignment with itself, and is round-trip transmutable to
>>  // itself.
>> +#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
>>  unsafe impl super::AtomicType for i8 {
>>      type Repr = i8;
>>  }
>> 
>> +// 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.
>> +//
>>  // SAFETY: `i16` has the same size and alignment with itself, and is round-trip transmutable to
>>  // itself.
>> +#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
>>  unsafe impl super::AtomicType for i16 {
>>      type Repr = i16;
>>  }
>> 
>> I can fold it into your patch if that works.
>> 
> 
> OK, the right place should be at AtomicImpl instead of AtomicType:
> 
> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> index ac689ce8ee8c..f4760e3a916e 100644
> --- a/rust/kernel/sync/atomic/internal.rs
> +++ b/rust/kernel/sync/atomic/internal.rs
> @@ -37,10 +37,16 @@ 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 i8 {
>      type Delta = Self;
>  }
> 
> +// 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 i16 {
>      type Delta = Self;
>  }

With the above change, won't it cause a compile error on architectures
where CONFIG_ARCH_SUPPORTS_ATOMIC_RMW is disabled?

If that is intended, I'm fine with it.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 3/3] rust: sync: atomic: Add i8/i16 xchg and cmpxchg support
  2025-12-29 13:04       ` FUJITA Tomonori
@ 2025-12-29 13:13         ` Boqun Feng
  0 siblings, 0 replies; 16+ messages in thread
From: Boqun Feng @ 2025-12-29 13:13 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	tmgross, acourbot, rust-for-linux, linux-arch

On Mon, Dec 29, 2025 at 10:04:39PM +0900, FUJITA Tomonori wrote:
> On Mon, 29 Dec 2025 20:30:58 +0800
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> > On Mon, Dec 29, 2025 at 08:27:01PM +0800, Boqun Feng wrote:
> >> On Sun, Dec 28, 2025 at 09:05:46PM +0900, FUJITA Tomonori wrote:
> >> > Add atomic xchg and cmpxchg operation support for i8 and i16 types
> >> > with tests.
> >> > 
> >> 
> >> I think we also needs the following, otherwise architectures may
> >> accidentally enable Rust but don't have the correct atomic
> >> implementation for i8 and i16.
> >> 
> >> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> >> index 248d26555ccf..a4e5bbd45eb2 100644
> >> --- a/rust/kernel/sync/atomic/predefine.rs
> >> +++ b/rust/kernel/sync/atomic/predefine.rs
> >> @@ -5,14 +5,22 @@
> >>  use crate::static_assert;
> >>  use core::mem::{align_of, size_of};
> >> 
> >> +// 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.
> >> +//
> >>  // SAFETY: `i8` has the same size and alignment with itself, and is round-trip transmutable to
> >>  // itself.
> >> +#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
> >>  unsafe impl super::AtomicType for i8 {
> >>      type Repr = i8;
> >>  }
> >> 
> >> +// 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.
> >> +//
> >>  // SAFETY: `i16` has the same size and alignment with itself, and is round-trip transmutable to
> >>  // itself.
> >> +#[cfg(CONFIG_ARCH_SUPPORTS_ATOMIC_RMW)]
> >>  unsafe impl super::AtomicType for i16 {
> >>      type Repr = i16;
> >>  }
> >> 
> >> I can fold it into your patch if that works.
> >> 
> > 
> > OK, the right place should be at AtomicImpl instead of AtomicType:
> > 
> > diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> > index ac689ce8ee8c..f4760e3a916e 100644
> > --- a/rust/kernel/sync/atomic/internal.rs
> > +++ b/rust/kernel/sync/atomic/internal.rs
> > @@ -37,10 +37,16 @@ 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 i8 {
> >      type Delta = Self;
> >  }
> > 
> > +// 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 i16 {
> >      type Delta = Self;
> >  }
> 
> With the above change, won't it cause a compile error on architectures
> where CONFIG_ARCH_SUPPORTS_ATOMIC_RMW is disabled?
> 
> If that is intended, I'm fine with it.
> 

Right, the intention is to cause build errors because then we need to
add lock-based atomic_{i8,i16}_read() and atomic_{i8,i16}_set() when
those archs begin to support Rust.

Regards,
Boqun

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 2/3] rust: sync: atomic: Remove workaround macro for i8/i16 BasicOps
  2025-12-29 12:55     ` FUJITA Tomonori
@ 2025-12-29 13:19       ` Boqun Feng
  0 siblings, 0 replies; 16+ messages in thread
From: Boqun Feng @ 2025-12-29 13:19 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
	tmgross, acourbot, rust-for-linux, linux-arch

On Mon, Dec 29, 2025 at 09:55:58PM +0900, FUJITA Tomonori wrote:
> On Mon, 29 Dec 2025 19:58:48 +0800
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> > On Sun, Dec 28, 2025 at 09:05:45PM +0900, FUJITA Tomonori wrote:
> >> Remove workaround impl_atomic_only_load_and_store_ops macro and use
> >> declare_and_impl_atomic_methods to add AtomicBasicOps support for
> >> i8/i16.
> >> 
> > 
> > I did the following so that we can drop this ;-)
> > 
> > 1. Change function names of [1] and [2] from *{load,store}* to
> >    *{read,set}*.
> > 
> > 2. Reorder [3] before [4] to avoid introduction of
> >    impl_atomic_only_load_and_store_ops!()
> > 
> > [1]: https://lore.kernel.org/all/20251211113826.1299077-3-fujita.tomonori@gmail.com/
> > [2]: https://lore.kernel.org/all/20251211113826.1299077-2-fujita.tomonori@gmail.com/
> > [3]: https://lore.kernel.org/all/20251228120546.1602275-2-fujita.tomonori@gmail.com/
> > [4]: https://lore.kernel.org/all/20251211113826.1299077-4-fujita.tomonori@gmail.com/
> > 
> > I also reorder a bit to make the introduction of helpers are grouped
> > together, please see at
> > 
> > 	https://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git/log/?h=rust-sync.20251229
> > 
> > I feel this way we have a cleaner history of changes.
> 
> It looks much cleaner now, thanks!
> 
> Maybe, you could consider chan ging the following two points:
> 
> - change the first patch subject
> 
> rust: helpers: Add i8/i16 atomic_read_acquire/atomic_set_release helpers 
> 
> - drop the following comment in 12th patch:
> 
> +// It is still unclear whether i8/i16 atomics will eventually support
> +// the same set of operations as i32/i64, because some architectures
> +// do not provide hardware support for the required atomic primitives.
> +// Furthermore, supporting Atomic<bool> will require even more
> +// significant structural changes.
> +//
> +// To avoid premature refactoring, a separate macro for i8 and i16 is
> +// used for now, leaving the existing macros untouched until the overall
> +// design requirements are settled.
> 
Good eyes!

Done (along with other fixes):

	https://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git/log/?h=rust-sync.20251229b

Regards,
Boqun

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support
  2025-12-29 11:13   ` Boqun Feng
  2025-12-29 11:54     ` FUJITA Tomonori
@ 2025-12-29 16:36     ` Gary Guo
  2025-12-30  0:17       ` Boqun Feng
  1 sibling, 1 reply; 16+ messages in thread
From: Gary Guo @ 2025-12-29 16:36 UTC (permalink / raw)
  To: Boqun Feng
  Cc: FUJITA Tomonori, ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr,
	lossin, tmgross, acourbot, rust-for-linux, linux-arch

On Mon, 29 Dec 2025 19:13:11 +0800
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Sun, Dec 28, 2025 at 09:05:44PM +0900, FUJITA Tomonori wrote:
> > Rework the internal AtomicOps macro plumbing to generate per-type
> > implementations from a mapping list.
> > 
> > Capture the trait definition once and reuse it for both declaration
> > and per-type impl expansion to reduce duplication and keep future
> > extensions simple.
> > 
> > This is a preparatory refactor for enabling i8/i16 atomics cleanly.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>  
> 
> Thanks! I have an idea that uses proc-macro to generate the Atomic*Ops
> impls, e.g.
> 
>     #[atomic_ops(i8, i16, i32, i64)]
>     pub trait AtomicBasicOps {
>         #[variant(acquire)]
>         fn read(a: &AtomicRepr<Self>) -> Self {
> 	    unsafe { binding_call!(a.as_ptr().cast()) }
> 	}
>     }

Unless the proc macro is generally applicable to a wide range of subsystem
abstractions, I would still prefer to use declarative macros.

Best,
Gary

> 
> But I think the current solution in your patch suffices as a temporary
> solution at least.
> 
> Regards,
> Boqun

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support
  2025-12-29 16:36     ` Gary Guo
@ 2025-12-30  0:17       ` Boqun Feng
  2026-01-02 11:25         ` Gary Guo
  0 siblings, 1 reply; 16+ messages in thread
From: Boqun Feng @ 2025-12-30  0:17 UTC (permalink / raw)
  To: Gary Guo
  Cc: FUJITA Tomonori, ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr,
	lossin, tmgross, acourbot, rust-for-linux, linux-arch

On Mon, Dec 29, 2025 at 04:36:16PM +0000, Gary Guo wrote:
> On Mon, 29 Dec 2025 19:13:11 +0800
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> > On Sun, Dec 28, 2025 at 09:05:44PM +0900, FUJITA Tomonori wrote:
> > > Rework the internal AtomicOps macro plumbing to generate per-type
> > > implementations from a mapping list.
> > > 
> > > Capture the trait definition once and reuse it for both declaration
> > > and per-type impl expansion to reduce duplication and keep future
> > > extensions simple.
> > > 
> > > This is a preparatory refactor for enabling i8/i16 atomics cleanly.
> > > 
> > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>  
> > 
> > Thanks! I have an idea that uses proc-macro to generate the Atomic*Ops
> > impls, e.g.
> > 
> >     #[atomic_ops(i8, i16, i32, i64)]
> >     pub trait AtomicBasicOps {
> >         #[variant(acquire)]
> >         fn read(a: &AtomicRepr<Self>) -> Self {
> > 	    unsafe { binding_call!(a.as_ptr().cast()) }
> > 	}
> >     }
> 
> Unless the proc macro is generally applicable to a wide range of subsystem
> abstractions, I would still prefer to use declarative macros.
> 

But the tt-muncher style is quite challenging for a boarder audience to
understand and change (if necessary) the declarative macros. IMO, if we
can have mod-specific proc macros (i.e. macros that are only usable in
atomic mod) then it should be fine. Thoughts?

Regards,
Boqun

> Best,
> Gary
> 
> > 
> > But I think the current solution in your patch suffices as a temporary
> > solution at least.
> > 
> > Regards,
> > Boqun

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support
  2025-12-30  0:17       ` Boqun Feng
@ 2026-01-02 11:25         ` Gary Guo
  0 siblings, 0 replies; 16+ messages in thread
From: Gary Guo @ 2026-01-02 11:25 UTC (permalink / raw)
  To: Boqun Feng
  Cc: FUJITA Tomonori, ojeda, a.hindborg, aliceryhl, bjorn3_gh, dakr,
	lossin, tmgross, acourbot, rust-for-linux, linux-arch

On Tue, 30 Dec 2025 08:17:13 +0800
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Mon, Dec 29, 2025 at 04:36:16PM +0000, Gary Guo wrote:
> > On Mon, 29 Dec 2025 19:13:11 +0800
> > Boqun Feng <boqun.feng@gmail.com> wrote:
> >   
> > > On Sun, Dec 28, 2025 at 09:05:44PM +0900, FUJITA Tomonori wrote:  
> > > > Rework the internal AtomicOps macro plumbing to generate per-type
> > > > implementations from a mapping list.
> > > > 
> > > > Capture the trait definition once and reuse it for both declaration
> > > > and per-type impl expansion to reduce duplication and keep future
> > > > extensions simple.
> > > > 
> > > > This is a preparatory refactor for enabling i8/i16 atomics cleanly.
> > > > 
> > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>    
> > > 
> > > Thanks! I have an idea that uses proc-macro to generate the Atomic*Ops
> > > impls, e.g.
> > > 
> > >     #[atomic_ops(i8, i16, i32, i64)]
> > >     pub trait AtomicBasicOps {
> > >         #[variant(acquire)]
> > >         fn read(a: &AtomicRepr<Self>) -> Self {
> > > 	    unsafe { binding_call!(a.as_ptr().cast()) }
> > > 	}
> > >     }  
> > 
> > Unless the proc macro is generally applicable to a wide range of subsystem
> > abstractions, I would still prefer to use declarative macros.
> >   
> 
> But the tt-muncher style is quite challenging for a boarder audience to
> understand and change (if necessary) the declarative macros. IMO, if we
> can have mod-specific proc macros (i.e. macros that are only usable in
> atomic mod) then it should be fine. Thoughts?

I don't think our current macro impl is too bad (not really tt-muncher I
think?), and it's still very readable to me.  Proc macros can also be
challenging for a broader audience, as it requires you to read parser code
:)

Unless the macro is complex enough (e.g. pin-init) or has very broad usage
in the kernel, I think it's still best to limit to declarative macros.

Best,
Gary

> 
> Regards,
> Boqun
> 
> > Best,
> > Gary
> >   
> > > 
> > > But I think the current solution in your patch suffices as a temporary
> > > solution at least.
> > > 
> > > Regards,
> > > Boqun  


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-01-02 11:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-28 12:05 [PATCH v1 0/3] rust: Add xchg and cmpxchg support on i8/i16 FUJITA Tomonori
2025-12-28 12:05 ` [PATCH v1 1/3] rust: sync: atomic: Prepare AtomicOps macros for i8/i16 support FUJITA Tomonori
2025-12-29 11:13   ` Boqun Feng
2025-12-29 11:54     ` FUJITA Tomonori
2025-12-29 16:36     ` Gary Guo
2025-12-30  0:17       ` Boqun Feng
2026-01-02 11:25         ` Gary Guo
2025-12-28 12:05 ` [PATCH v1 2/3] rust: sync: atomic: Remove workaround macro for i8/i16 BasicOps FUJITA Tomonori
2025-12-29 11:58   ` Boqun Feng
2025-12-29 12:55     ` FUJITA Tomonori
2025-12-29 13:19       ` Boqun Feng
2025-12-28 12:05 ` [PATCH v1 3/3] rust: sync: atomic: Add i8/i16 xchg and cmpxchg support FUJITA Tomonori
2025-12-29 12:27   ` Boqun Feng
2025-12-29 12:30     ` Boqun Feng
2025-12-29 13:04       ` FUJITA Tomonori
2025-12-29 13:13         ` Boqun Feng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox