* [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