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