* [PATCH v2 1/4] rust: sync: Add i8/i16 atomic_load_acquire/atomic_store_release helpers
2025-11-17 0:10 [PATCH v2 0/4] rust: Add i8 and i16 atomic support FUJITA Tomonori
@ 2025-11-17 0:10 ` FUJITA Tomonori
2025-12-08 3:01 ` Alexandre Courbot
2025-11-17 0:10 ` [PATCH v2 2/4] rust: helpers: Add i8/i16 relaxed atomic helpers FUJITA Tomonori
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: FUJITA Tomonori @ 2025-11-17 0:10 UTC (permalink / raw)
To: alex.gaynor, boqun.feng, ojeda, peterz, will
Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
mark.rutland, rust-for-linux, tmgross
Add helper functions to expose smp_load_acquire() and
smp_store_release() for i8 and i16 types.
The smp_load_acquire() and smp_store_release() macros require type
information (sizeof) to generate appropriate architecture-specific
memory ordering instructions. Therefore, separate helper functions are
needed for each type size.
These helpers will be used by the upcoming Atomic<i8> and Atomic<i16>
implementation to provide proper Acquire/Release semantics across all
architectures.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/helpers/atomic_ext.c | 23 +++++++++++++++++++++++
rust/helpers/helpers.c | 1 +
2 files changed, 24 insertions(+)
create mode 100644 rust/helpers/atomic_ext.c
diff --git a/rust/helpers/atomic_ext.c b/rust/helpers/atomic_ext.c
new file mode 100644
index 000000000000..d05f6f7ba10c
--- /dev/null
+++ b/rust/helpers/atomic_ext.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <asm/barrier.h>
+
+s8 rust_helper_atomic_i8_load_acquire(s8 *ptr)
+{
+ return smp_load_acquire(ptr);
+}
+
+s16 rust_helper_atomic_i16_load_acquire(s16 *ptr)
+{
+ return smp_load_acquire(ptr);
+}
+
+void rust_helper_atomic_i8_store_release(s8 *ptr, s8 val)
+{
+ smp_store_release(ptr, val);
+}
+
+void rust_helper_atomic_i16_store_release(s16 *ptr, s16 val)
+{
+ smp_store_release(ptr, val);
+}
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 551da6c9b506..6f1c4e2faed4 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -8,6 +8,7 @@
*/
#include "atomic.c"
+#include "atomic_ext.c"
#include "auxiliary.c"
#include "barrier.c"
#include "binder.c"
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 1/4] rust: sync: Add i8/i16 atomic_load_acquire/atomic_store_release helpers
2025-11-17 0:10 ` [PATCH v2 1/4] rust: sync: Add i8/i16 atomic_load_acquire/atomic_store_release helpers FUJITA Tomonori
@ 2025-12-08 3:01 ` Alexandre Courbot
2025-12-08 21:56 ` FUJITA Tomonori
0 siblings, 1 reply; 18+ messages in thread
From: Alexandre Courbot @ 2025-12-08 3:01 UTC (permalink / raw)
To: FUJITA Tomonori, alex.gaynor, boqun.feng, ojeda, peterz, will
Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
mark.rutland, rust-for-linux, tmgross
On Mon Nov 17, 2025 at 9:10 AM JST, FUJITA Tomonori wrote:
> Add helper functions to expose smp_load_acquire() and
> smp_store_release() for i8 and i16 types.
>
> The smp_load_acquire() and smp_store_release() macros require type
> information (sizeof) to generate appropriate architecture-specific
> memory ordering instructions. Therefore, separate helper functions are
> needed for each type size.
>
> These helpers will be used by the upcoming Atomic<i8> and Atomic<i16>
> implementation to provide proper Acquire/Release semantics across all
> architectures.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
> rust/helpers/atomic_ext.c | 23 +++++++++++++++++++++++
> rust/helpers/helpers.c | 1 +
> 2 files changed, 24 insertions(+)
> create mode 100644 rust/helpers/atomic_ext.c
Why not add these to `rust/helpers/atomic.c`?
>
> diff --git a/rust/helpers/atomic_ext.c b/rust/helpers/atomic_ext.c
> new file mode 100644
> index 000000000000..d05f6f7ba10c
> --- /dev/null
> +++ b/rust/helpers/atomic_ext.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <asm/barrier.h>
> +
> +s8 rust_helper_atomic_i8_load_acquire(s8 *ptr)
I guess you will want to prefix these with `__rust_helper` in the next
revision.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/4] rust: sync: Add i8/i16 atomic_load_acquire/atomic_store_release helpers
2025-12-08 3:01 ` Alexandre Courbot
@ 2025-12-08 21:56 ` FUJITA Tomonori
0 siblings, 0 replies; 18+ messages in thread
From: FUJITA Tomonori @ 2025-12-08 21:56 UTC (permalink / raw)
To: acourbot, boqun.feng
Cc: fujita.tomonori, alex.gaynor, ojeda, peterz, will, a.hindborg,
aliceryhl, bjorn3_gh, dakr, gary, lossin, mark.rutland,
rust-for-linux, tmgross
Thanks for the review!
On Mon, 08 Dec 2025 12:01:12 +0900
"Alexandre Courbot" <acourbot@nvidia.com> wrote:
> On Mon Nov 17, 2025 at 9:10 AM JST, FUJITA Tomonori wrote:
>> Add helper functions to expose smp_load_acquire() and
>> smp_store_release() for i8 and i16 types.
>>
>> The smp_load_acquire() and smp_store_release() macros require type
>> information (sizeof) to generate appropriate architecture-specific
>> memory ordering instructions. Therefore, separate helper functions are
>> needed for each type size.
>>
>> These helpers will be used by the upcoming Atomic<i8> and Atomic<i16>
>> implementation to provide proper Acquire/Release semantics across all
>> architectures.
>>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> ---
>> rust/helpers/atomic_ext.c | 23 +++++++++++++++++++++++
>> rust/helpers/helpers.c | 1 +
>> 2 files changed, 24 insertions(+)
>> create mode 100644 rust/helpers/atomic_ext.c
>
> Why not add these to `rust/helpers/atomic.c`?
https://lore.kernel.org/rust-for-linux/20251115050305.3872412-1-fujita.tomonori@gmail.com/
I split them into atomic_ext.c because Boqun specifically asked me to
keep the new helpers separate. My understanding is that these helpers
expose different symbol names than their C counterparts, so he
preferred to isolate them from the existing ones.
>> diff --git a/rust/helpers/atomic_ext.c b/rust/helpers/atomic_ext.c
>> new file mode 100644
>> index 000000000000..d05f6f7ba10c
>> --- /dev/null
>> +++ b/rust/helpers/atomic_ext.c
>> @@ -0,0 +1,23 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <asm/barrier.h>
>> +
>> +s8 rust_helper_atomic_i8_load_acquire(s8 *ptr)
>
> I guess you will want to prefix these with `__rust_helper` in the next
> revision.
Ah, you are right. I will add it in v3.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 2/4] rust: helpers: Add i8/i16 relaxed atomic helpers
2025-11-17 0:10 [PATCH v2 0/4] rust: Add i8 and i16 atomic support FUJITA Tomonori
2025-11-17 0:10 ` [PATCH v2 1/4] rust: sync: Add i8/i16 atomic_load_acquire/atomic_store_release helpers FUJITA Tomonori
@ 2025-11-17 0:10 ` FUJITA Tomonori
2025-11-17 0:10 ` [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support FUJITA Tomonori
` (2 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: FUJITA Tomonori @ 2025-11-17 0:10 UTC (permalink / raw)
To: alex.gaynor, boqun.feng, ojeda, peterz, will
Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
mark.rutland, rust-for-linux, tmgross
Add READ_ONCE/WRITE_ONCE based helpers for i8 and i16 types to support
relaxed atomic operations in Rust.
While relaxed operations could be implemented purely in Rust using
read_volatile() and write_volatile(), using C's READ_ONCE() and
WRITE_ONCE() macros ensures complete consistency with the kernel
memory model.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/helpers/atomic_ext.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/rust/helpers/atomic_ext.c b/rust/helpers/atomic_ext.c
index d05f6f7ba10c..ca22f57f37a4 100644
--- a/rust/helpers/atomic_ext.c
+++ b/rust/helpers/atomic_ext.c
@@ -1,22 +1,43 @@
// SPDX-License-Identifier: GPL-2.0
#include <asm/barrier.h>
+#include <asm/rwonce.h>
+
+s8 rust_helper_atomic_i8_load(s8 *ptr)
+{
+ return READ_ONCE(*ptr);
+}
s8 rust_helper_atomic_i8_load_acquire(s8 *ptr)
{
return smp_load_acquire(ptr);
}
+s16 rust_helper_atomic_i16_load(s16 *ptr)
+{
+ return READ_ONCE(*ptr);
+}
+
s16 rust_helper_atomic_i16_load_acquire(s16 *ptr)
{
return smp_load_acquire(ptr);
}
+void rust_helper_atomic_i8_store(s8 *ptr, s8 val)
+{
+ WRITE_ONCE(*ptr, val);
+}
+
void rust_helper_atomic_i8_store_release(s8 *ptr, s8 val)
{
smp_store_release(ptr, val);
}
+void rust_helper_atomic_i16_store(s16 *ptr, s16 val)
+{
+ WRITE_ONCE(*ptr, val);
+}
+
void rust_helper_atomic_i16_store_release(s16 *ptr, s16 val)
{
smp_store_release(ptr, val);
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support
2025-11-17 0:10 [PATCH v2 0/4] rust: Add i8 and i16 atomic support FUJITA Tomonori
2025-11-17 0:10 ` [PATCH v2 1/4] rust: sync: Add i8/i16 atomic_load_acquire/atomic_store_release helpers FUJITA Tomonori
2025-11-17 0:10 ` [PATCH v2 2/4] rust: helpers: Add i8/i16 relaxed atomic helpers FUJITA Tomonori
@ 2025-11-17 0:10 ` FUJITA Tomonori
2025-12-08 3:08 ` Alexandre Courbot
2025-11-17 0:10 ` [PATCH v2 4/4] rust: sync: atomic: Add store_release/load_acquire tests FUJITA Tomonori
2025-12-03 3:41 ` [PATCH v2 0/4] rust: Add i8 and i16 atomic support FUJITA Tomonori
4 siblings, 1 reply; 18+ messages in thread
From: FUJITA Tomonori @ 2025-11-17 0:10 UTC (permalink / raw)
To: alex.gaynor, boqun.feng, ojeda, peterz, will
Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
mark.rutland, rust-for-linux, tmgross
Add atomic operation support for i8 and i16 types using volatile
read/write and smp_load_acquire/smp_store_release helpers.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/sync/atomic/internal.rs | 48 ++++++++++++++++++++++++++++
rust/kernel/sync/atomic/predefine.rs | 14 +++++++-
2 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
index 6fdd8e59f45b..4c9f4f76dbdf 100644
--- a/rust/kernel/sync/atomic/internal.rs
+++ b/rust/kernel/sync/atomic/internal.rs
@@ -263,3 +263,51 @@ fn fetch_add[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self::Delta) ->
}
}
);
+
+impl private::Sealed for i8 {}
+impl private::Sealed for i16 {}
+
+impl AtomicImpl for i8 {
+ type Delta = Self;
+}
+
+impl AtomicImpl for i16 {
+ type Delta = Self;
+}
+
+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);
diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
index 45a17985cda4..09b357be59b8 100644
--- a/rust/kernel/sync/atomic/predefine.rs
+++ b/rust/kernel/sync/atomic/predefine.rs
@@ -5,6 +5,18 @@
use crate::static_assert;
use core::mem::{align_of, size_of};
+// SAFETY: `i8` has the same size and alignment with itself, and is round-trip transmutable to
+// itself.
+unsafe impl super::AtomicType for i8 {
+ type Repr = i8;
+}
+
+// SAFETY: `i16` has the same size and alignment with itself, and is round-trip transmutable to
+// itself.
+unsafe impl super::AtomicType for i16 {
+ type Repr = i16;
+}
+
// SAFETY: `i32` has the same size and alignment with itself, and is round-trip transmutable to
// itself.
unsafe impl super::AtomicType for i32 {
@@ -118,7 +130,7 @@ macro_rules! for_each_type {
#[test]
fn atomic_basic_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);
assert_eq!(v, x.load(Relaxed));
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support
2025-11-17 0:10 ` [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support FUJITA Tomonori
@ 2025-12-08 3:08 ` Alexandre Courbot
2025-12-08 23:14 ` FUJITA Tomonori
0 siblings, 1 reply; 18+ messages in thread
From: Alexandre Courbot @ 2025-12-08 3:08 UTC (permalink / raw)
To: FUJITA Tomonori, alex.gaynor, boqun.feng, ojeda, peterz, will
Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
mark.rutland, rust-for-linux, tmgross
On Mon Nov 17, 2025 at 9:10 AM JST, FUJITA Tomonori wrote:
> Add atomic operation support for i8 and i16 types using volatile
> read/write and smp_load_acquire/smp_store_release helpers.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
> rust/kernel/sync/atomic/internal.rs | 48 ++++++++++++++++++++++++++++
> rust/kernel/sync/atomic/predefine.rs | 14 +++++++-
> 2 files changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> index 6fdd8e59f45b..4c9f4f76dbdf 100644
> --- a/rust/kernel/sync/atomic/internal.rs
> +++ b/rust/kernel/sync/atomic/internal.rs
> @@ -263,3 +263,51 @@ fn fetch_add[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self::Delta) ->
> }
> }
> );
> +
> +impl private::Sealed for i8 {}
> +impl private::Sealed for i16 {}
> +
> +impl AtomicImpl for i8 {
> + type Delta = Self;
> +}
> +
> +impl AtomicImpl for i16 {
> + type Delta = Self;
> +}
I'd suggest putting these next to the impls for `i32` and `i64`, for clarity.
> +
> +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) }
> + }
> + }
> + }
> + )*
> + };
> +}
Can you document this macro a bit, in particular the motivations for not
leveraging the existing ones (I guess this has to be with the new
helpers, but following the code through is a bit difficult without
comments).
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support
2025-12-08 3:08 ` Alexandre Courbot
@ 2025-12-08 23:14 ` FUJITA Tomonori
2025-12-09 0:27 ` Alexandre Courbot
2025-12-10 23:02 ` Boqun Feng
0 siblings, 2 replies; 18+ messages in thread
From: FUJITA Tomonori @ 2025-12-08 23:14 UTC (permalink / raw)
To: acourbot, boqun.feng
Cc: fujita.tomonori, alex.gaynor, ojeda, peterz, will, a.hindborg,
aliceryhl, bjorn3_gh, dakr, gary, lossin, mark.rutland,
rust-for-linux, tmgross
On Mon, 08 Dec 2025 12:08:23 +0900
"Alexandre Courbot" <acourbot@nvidia.com> wrote:
> On Mon Nov 17, 2025 at 9:10 AM JST, FUJITA Tomonori wrote:
>> Add atomic operation support for i8 and i16 types using volatile
>> read/write and smp_load_acquire/smp_store_release helpers.
>>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> ---
>> rust/kernel/sync/atomic/internal.rs | 48 ++++++++++++++++++++++++++++
>> rust/kernel/sync/atomic/predefine.rs | 14 +++++++-
>> 2 files changed, 61 insertions(+), 1 deletion(-)
>>
>> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
>> index 6fdd8e59f45b..4c9f4f76dbdf 100644
>> --- a/rust/kernel/sync/atomic/internal.rs
>> +++ b/rust/kernel/sync/atomic/internal.rs
>> @@ -263,3 +263,51 @@ fn fetch_add[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self::Delta) ->
>> }
>> }
>> );
>> +
>> +impl private::Sealed for i8 {}
>> +impl private::Sealed for i16 {}
>> +
>> +impl AtomicImpl for i8 {
>> + type Delta = Self;
>> +}
>> +
>> +impl AtomicImpl for i16 {
>> + type Delta = Self;
>> +}
>
> I'd suggest putting these next to the impls for `i32` and `i64`, for clarity.
I'm fine with either. Boqun, what do you think?
The reason I didn't put i8/i16 next to i32/i64 is that I assume that
i8/i16 won't support all the methods supported by i32/i64. I thought
keeping them separate would make that distinction clearer.
>> +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) }
>> + }
>> + }
>> + }
>> + )*
>> + };
>> +}
>
> Can you document this macro a bit, in particular the motivations for not
> leveraging the existing ones (I guess this has to be with the new
> helpers, but following the code through is a bit difficult without
> comments).
// Since i8 and i16 are not expected to support the full feature set
// of i32 and i64, using the current declare_and_impl_atomic_methods!
// would require refactoring it to handle specific types or splitting
// the definitions. 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.
makes sense?
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support
2025-12-08 23:14 ` FUJITA Tomonori
@ 2025-12-09 0:27 ` Alexandre Courbot
2025-12-09 23:31 ` FUJITA Tomonori
2025-12-10 23:02 ` Boqun Feng
1 sibling, 1 reply; 18+ messages in thread
From: Alexandre Courbot @ 2025-12-09 0:27 UTC (permalink / raw)
To: FUJITA Tomonori, acourbot, boqun.feng
Cc: alex.gaynor, ojeda, peterz, will, a.hindborg, aliceryhl,
bjorn3_gh, dakr, gary, lossin, mark.rutland, rust-for-linux,
tmgross
On Tue Dec 9, 2025 at 8:14 AM JST, FUJITA Tomonori wrote:
> On Mon, 08 Dec 2025 12:08:23 +0900
> "Alexandre Courbot" <acourbot@nvidia.com> wrote:
>
>> On Mon Nov 17, 2025 at 9:10 AM JST, FUJITA Tomonori wrote:
>>> Add atomic operation support for i8 and i16 types using volatile
>>> read/write and smp_load_acquire/smp_store_release helpers.
>>>
>>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>> ---
>>> rust/kernel/sync/atomic/internal.rs | 48 ++++++++++++++++++++++++++++
>>> rust/kernel/sync/atomic/predefine.rs | 14 +++++++-
>>> 2 files changed, 61 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
>>> index 6fdd8e59f45b..4c9f4f76dbdf 100644
>>> --- a/rust/kernel/sync/atomic/internal.rs
>>> +++ b/rust/kernel/sync/atomic/internal.rs
>>> @@ -263,3 +263,51 @@ fn fetch_add[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self::Delta) ->
>>> }
>>> }
>>> );
>>> +
>>> +impl private::Sealed for i8 {}
>>> +impl private::Sealed for i16 {}
>>> +
>>> +impl AtomicImpl for i8 {
>>> + type Delta = Self;
>>> +}
>>> +
>>> +impl AtomicImpl for i16 {
>>> + type Delta = Self;
>>> +}
>>
>> I'd suggest putting these next to the impls for `i32` and `i64`, for clarity.
>
> I'm fine with either. Boqun, what do you think?
>
> The reason I didn't put i8/i16 next to i32/i64 is that I assume that
> i8/i16 won't support all the methods supported by i32/i64. I thought
> keeping them separate would make that distinction clearer.
The `AtomicImpl` blocks are the same, it is the macro-implemented blocks
that differ and these can come later. But having all the types and their
basic implementations at the same place helps understanding the basics
of what this module provides IMHO.
>
>>> +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) }
>>> + }
>>> + }
>>> + }
>>> + )*
>>> + };
>>> +}
>>
>> Can you document this macro a bit, in particular the motivations for not
>> leveraging the existing ones (I guess this has to be with the new
>> helpers, but following the code through is a bit difficult without
>> comments).
>
> // Since i8 and i16 are not expected to support the full feature set
> // of i32 and i64, using the current declare_and_impl_atomic_methods!
> // would require refactoring it to handle specific types or splitting
> // the definitions. 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.
>
> makes sense?
Absolutely, thanks! Maybe also give a short explanation for *why* i8 and
i16 won't support the same feature set as i32 and i64.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support
2025-12-09 0:27 ` Alexandre Courbot
@ 2025-12-09 23:31 ` FUJITA Tomonori
2025-12-10 23:16 ` Boqun Feng
0 siblings, 1 reply; 18+ messages in thread
From: FUJITA Tomonori @ 2025-12-09 23:31 UTC (permalink / raw)
To: acourbot, boqun.feng
Cc: fujita.tomonori, alex.gaynor, ojeda, peterz, will, a.hindborg,
aliceryhl, bjorn3_gh, dakr, gary, lossin, mark.rutland,
rust-for-linux, tmgross
On Tue, 09 Dec 2025 09:27:25 +0900
"Alexandre Courbot" <acourbot@nvidia.com> wrote:
>>> Can you document this macro a bit, in particular the motivations for not
>>> leveraging the existing ones (I guess this has to be with the new
>>> helpers, but following the code through is a bit difficult without
>>> comments).
>>
>> // Since i8 and i16 are not expected to support the full feature set
>> // of i32 and i64, using the current declare_and_impl_atomic_methods!
>> // would require refactoring it to handle specific types or splitting
>> // the definitions. 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.
>>
>> makes sense?
>
> Absolutely, thanks! Maybe also give a short explanation for *why* i8 and
> i16 won't support the same feature set as i32 and i64.
I could add, though Boqun might have a different perspective.
In my opinion, the reason is that the C side doesn't fully support
atomic operations for i8 and i16. While the C side doesn't support
load and store for i8/i16, we can easily support them with simple
wrappers (as this patchset does). I believe that xchg and cmpxchg
could also be supported easily.
However, supporting AtomicArithmeticOps (like fetch_add) for i8/i16
would likely require more than wrappers; architecture-dependent code
(likely with assembly). I don't think that it's a good idea to
implement such.
My plan is to add supoprt for limited atomic operations for i8/i16,
and then use the i8 support to implement Atomic<bool>.
Once that is in place, I intend to replace the Atomic<i32> usage in
SetOnce and OnceLite [1]. I believe the AtomicBool usage in the
binder driver could be replaced too.
[1] https://lore.kernel.org/rust-for-linux/20251117002452.4068692-1-fujita.tomonori@gmail.com/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support
2025-12-09 23:31 ` FUJITA Tomonori
@ 2025-12-10 23:16 ` Boqun Feng
2025-12-11 5:17 ` FUJITA Tomonori
0 siblings, 1 reply; 18+ messages in thread
From: Boqun Feng @ 2025-12-10 23:16 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: acourbot, alex.gaynor, ojeda, peterz, will, a.hindborg, aliceryhl,
bjorn3_gh, dakr, gary, lossin, mark.rutland, rust-for-linux,
tmgross
On Wed, Dec 10, 2025 at 08:31:47AM +0900, FUJITA Tomonori wrote:
> On Tue, 09 Dec 2025 09:27:25 +0900
> "Alexandre Courbot" <acourbot@nvidia.com> wrote:
>
> >>> Can you document this macro a bit, in particular the motivations for not
> >>> leveraging the existing ones (I guess this has to be with the new
> >>> helpers, but following the code through is a bit difficult without
> >>> comments).
> >>
> >> // Since i8 and i16 are not expected to support the full feature set
> >> // of i32 and i64, using the current declare_and_impl_atomic_methods!
> >> // would require refactoring it to handle specific types or splitting
> >> // the definitions. 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.
> >>
> >> makes sense?
> >
> > Absolutely, thanks! Maybe also give a short explanation for *why* i8 and
> > i16 won't support the same feature set as i32 and i64.
>
> I could add, though Boqun might have a different perspective.
>
Yeah, I think it's more of an "undecided" stage. The reasons that why we
want to avoid the same feature as i32 and i64 for i8/i16 are:
1. For some architectures, 1-byte or 2-byte atomic RmW
(read-modify-write) operations are not supported, they would generate
worse binary than designing the algorithm with Atomic<i32/i64>.
2. For some architectures, atomics RmW are implemented by lock-based
operations on C side, in that case READ_ONCE()/WRITE_ONCE() based
load/store implementations are incorrect, because it would race with
lock-based RmW for example.
atomic_add_unless(v, 1, 0);
lock();
ret = READ_ONCE(v->counter); // == 1
atomic_set(v, 0);
if (ret != u) WRITE_ONCE(v->counter, 0);
WRITE_ONCE(v->counter, ret + 1);
unlock();
Hence, we need to make a design decision here, either 1) we make
Atomic<i8> support RmW and Atomic::<i8>::load() may contain locking
on certain architecture (that also means you cannot use it as
READ_ONCE()), or 2) we disallow RmW for Atomic<i8> if they use
lock-based atomic.
I feel we are not having the enough data to make the call here,
therefore I would avoid premature conclusion in the documentation.
Thanks!
Regards,
Boqun
> In my opinion, the reason is that the C side doesn't fully support
> atomic operations for i8 and i16. While the C side doesn't support
> load and store for i8/i16, we can easily support them with simple
> wrappers (as this patchset does). I believe that xchg and cmpxchg
> could also be supported easily.
>
> However, supporting AtomicArithmeticOps (like fetch_add) for i8/i16
> would likely require more than wrappers; architecture-dependent code
> (likely with assembly). I don't think that it's a good idea to
> implement such.
>
>
> My plan is to add supoprt for limited atomic operations for i8/i16,
> and then use the i8 support to implement Atomic<bool>.
>
> Once that is in place, I intend to replace the Atomic<i32> usage in
> SetOnce and OnceLite [1]. I believe the AtomicBool usage in the
> binder driver could be replaced too.
>
> [1] https://lore.kernel.org/rust-for-linux/20251117002452.4068692-1-fujita.tomonori@gmail.com/
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support
2025-12-10 23:16 ` Boqun Feng
@ 2025-12-11 5:17 ` FUJITA Tomonori
0 siblings, 0 replies; 18+ messages in thread
From: FUJITA Tomonori @ 2025-12-11 5:17 UTC (permalink / raw)
To: boqun.feng
Cc: fujita.tomonori, acourbot, alex.gaynor, ojeda, peterz, will,
a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
mark.rutland, rust-for-linux, tmgross
On Thu, 11 Dec 2025 08:16:40 +0900
Boqun Feng <boqun.feng@gmail.com> wrote:
> On Wed, Dec 10, 2025 at 08:31:47AM +0900, FUJITA Tomonori wrote:
>> On Tue, 09 Dec 2025 09:27:25 +0900
>> "Alexandre Courbot" <acourbot@nvidia.com> wrote:
>>
>> >>> Can you document this macro a bit, in particular the motivations for not
>> >>> leveraging the existing ones (I guess this has to be with the new
>> >>> helpers, but following the code through is a bit difficult without
>> >>> comments).
>> >>
>> >> // Since i8 and i16 are not expected to support the full feature set
>> >> // of i32 and i64, using the current declare_and_impl_atomic_methods!
>> >> // would require refactoring it to handle specific types or splitting
>> >> // the definitions. 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.
>> >>
>> >> makes sense?
>> >
>> > Absolutely, thanks! Maybe also give a short explanation for *why* i8 and
>> > i16 won't support the same feature set as i32 and i64.
>>
>> I could add, though Boqun might have a different perspective.
>>
>
> Yeah, I think it's more of an "undecided" stage. The reasons that why we
> want to avoid the same feature as i32 and i64 for i8/i16 are:
>
> 1. For some architectures, 1-byte or 2-byte atomic RmW
> (read-modify-write) operations are not supported, they would generate
> worse binary than designing the algorithm with Atomic<i32/i64>.
>
> 2. For some architectures, atomics RmW are implemented by lock-based
> operations on C side, in that case READ_ONCE()/WRITE_ONCE() based
> load/store implementations are incorrect, because it would race with
> lock-based RmW for example.
>
> atomic_add_unless(v, 1, 0);
> lock();
> ret = READ_ONCE(v->counter); // == 1
> atomic_set(v, 0);
> if (ret != u) WRITE_ONCE(v->counter, 0);
> WRITE_ONCE(v->counter, ret + 1);
> unlock();
>
> Hence, we need to make a design decision here, either 1) we make
> Atomic<i8> support RmW and Atomic::<i8>::load() may contain locking
> on certain architecture (that also means you cannot use it as
> READ_ONCE()), or 2) we disallow RmW for Atomic<i8> if they use
> lock-based atomic.
I was assuming option 2. As far as I know, all architectures that
support Rust today have hardware support for RmW operations, and I
don't expect we would add Rust support for older architectures like
PARISC in the future.
> I feel we are not having the enough data to make the call here,
> therefore I would avoid premature conclusion in the documentation.
I agree. I will update the comment to reflect that it is still unclear
whether i8/i16 atomics will support the same functionality as i32/i64
in the future.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support
2025-12-08 23:14 ` FUJITA Tomonori
2025-12-09 0:27 ` Alexandre Courbot
@ 2025-12-10 23:02 ` Boqun Feng
2025-12-11 4:46 ` FUJITA Tomonori
1 sibling, 1 reply; 18+ messages in thread
From: Boqun Feng @ 2025-12-10 23:02 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: acourbot, alex.gaynor, ojeda, peterz, will, a.hindborg, aliceryhl,
bjorn3_gh, dakr, gary, lossin, mark.rutland, rust-for-linux,
tmgross
On Tue, Dec 09, 2025 at 08:14:56AM +0900, FUJITA Tomonori wrote:
> On Mon, 08 Dec 2025 12:08:23 +0900
> "Alexandre Courbot" <acourbot@nvidia.com> wrote:
>
> > On Mon Nov 17, 2025 at 9:10 AM JST, FUJITA Tomonori wrote:
> >> Add atomic operation support for i8 and i16 types using volatile
> >> read/write and smp_load_acquire/smp_store_release helpers.
> >>
> >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> >> ---
> >> rust/kernel/sync/atomic/internal.rs | 48 ++++++++++++++++++++++++++++
> >> rust/kernel/sync/atomic/predefine.rs | 14 +++++++-
> >> 2 files changed, 61 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
> >> index 6fdd8e59f45b..4c9f4f76dbdf 100644
> >> --- a/rust/kernel/sync/atomic/internal.rs
> >> +++ b/rust/kernel/sync/atomic/internal.rs
> >> @@ -263,3 +263,51 @@ fn fetch_add[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self::Delta) ->
> >> }
> >> }
> >> );
> >> +
> >> +impl private::Sealed for i8 {}
> >> +impl private::Sealed for i16 {}
> >> +
> >> +impl AtomicImpl for i8 {
> >> + type Delta = Self;
> >> +}
> >> +
> >> +impl AtomicImpl for i16 {
> >> + type Delta = Self;
> >> +}
> >
> > I'd suggest putting these next to the impls for `i32` and `i64`, for clarity.
>
> I'm fine with either. Boqun, what do you think?
>
I think we should put them together.
> The reason I didn't put i8/i16 next to i32/i64 is that I assume that
> i8/i16 won't support all the methods supported by i32/i64. I thought
> keeping them separate would make that distinction clearer.
>
Even if that's true, it still make sense to put the impl blocks with
i32 and i64.
Regards,
Boqun
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support
2025-12-10 23:02 ` Boqun Feng
@ 2025-12-11 4:46 ` FUJITA Tomonori
0 siblings, 0 replies; 18+ messages in thread
From: FUJITA Tomonori @ 2025-12-11 4:46 UTC (permalink / raw)
To: boqun.feng
Cc: fujita.tomonori, acourbot, alex.gaynor, ojeda, peterz, will,
a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
mark.rutland, rust-for-linux, tmgross
On Thu, 11 Dec 2025 08:02:15 +0900
Boqun Feng <boqun.feng@gmail.com> wrote:
> On Tue, Dec 09, 2025 at 08:14:56AM +0900, FUJITA Tomonori wrote:
>> On Mon, 08 Dec 2025 12:08:23 +0900
>> "Alexandre Courbot" <acourbot@nvidia.com> wrote:
>>
>> > On Mon Nov 17, 2025 at 9:10 AM JST, FUJITA Tomonori wrote:
>> >> Add atomic operation support for i8 and i16 types using volatile
>> >> read/write and smp_load_acquire/smp_store_release helpers.
>> >>
>> >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> >> ---
>> >> rust/kernel/sync/atomic/internal.rs | 48 ++++++++++++++++++++++++++++
>> >> rust/kernel/sync/atomic/predefine.rs | 14 +++++++-
>> >> 2 files changed, 61 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/rust/kernel/sync/atomic/internal.rs b/rust/kernel/sync/atomic/internal.rs
>> >> index 6fdd8e59f45b..4c9f4f76dbdf 100644
>> >> --- a/rust/kernel/sync/atomic/internal.rs
>> >> +++ b/rust/kernel/sync/atomic/internal.rs
>> >> @@ -263,3 +263,51 @@ fn fetch_add[acquire, release, relaxed](a: &AtomicRepr<Self>, v: Self::Delta) ->
>> >> }
>> >> }
>> >> );
>> >> +
>> >> +impl private::Sealed for i8 {}
>> >> +impl private::Sealed for i16 {}
>> >> +
>> >> +impl AtomicImpl for i8 {
>> >> + type Delta = Self;
>> >> +}
>> >> +
>> >> +impl AtomicImpl for i16 {
>> >> + type Delta = Self;
>> >> +}
>> >
>> > I'd suggest putting these next to the impls for `i32` and `i64`, for clarity.
>>
>> I'm fine with either. Boqun, what do you think?
>>
>
> I think we should put them together.
Thanks for the confirming. I'll move them in v3.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 4/4] rust: sync: atomic: Add store_release/load_acquire tests
2025-11-17 0:10 [PATCH v2 0/4] rust: Add i8 and i16 atomic support FUJITA Tomonori
` (2 preceding siblings ...)
2025-11-17 0:10 ` [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support FUJITA Tomonori
@ 2025-11-17 0:10 ` FUJITA Tomonori
2025-12-03 3:41 ` [PATCH v2 0/4] rust: Add i8 and i16 atomic support FUJITA Tomonori
4 siblings, 0 replies; 18+ messages in thread
From: FUJITA Tomonori @ 2025-11-17 0:10 UTC (permalink / raw)
To: alex.gaynor, boqun.feng, ojeda, peterz, will
Cc: a.hindborg, aliceryhl, bjorn3_gh, dakr, gary, lossin,
mark.rutland, rust-for-linux, tmgross
Add minimum store_release/load_acquire tests.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/sync/atomic/predefine.rs | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
index 09b357be59b8..51e9df0cf56e 100644
--- a/rust/kernel/sync/atomic/predefine.rs
+++ b/rust/kernel/sync/atomic/predefine.rs
@@ -137,6 +137,16 @@ fn atomic_basic_tests() {
});
}
+ #[test]
+ fn atomic_acquire_release_tests() {
+ for_each_type!(42 in [i8, i16, i32, i64, u32, u64, isize, usize] |v| {
+ let x = Atomic::new(0);
+
+ x.store(v, Release);
+ assert_eq!(v, x.load(Acquire));
+ });
+ }
+
#[test]
fn atomic_xchg_tests() {
for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] rust: Add i8 and i16 atomic support
2025-11-17 0:10 [PATCH v2 0/4] rust: Add i8 and i16 atomic support FUJITA Tomonori
` (3 preceding siblings ...)
2025-11-17 0:10 ` [PATCH v2 4/4] rust: sync: atomic: Add store_release/load_acquire tests FUJITA Tomonori
@ 2025-12-03 3:41 ` FUJITA Tomonori
2025-12-04 3:40 ` Boqun Feng
4 siblings, 1 reply; 18+ messages in thread
From: FUJITA Tomonori @ 2025-12-03 3:41 UTC (permalink / raw)
To: boqun.feng
Cc: ojeda, peterz, will, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary,
lossin, mark.rutland, rust-for-linux, tmgross
On Mon, 17 Nov 2025 09:10:31 +0900
FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
> This adds Atomic<i8> and Atomic<i16> support; including
> load/store(Relaxed) and load(Acquire)/store(Release) operations.
>
> Relaxed operations are implemented in C's READ_ONCE() and
> WRITE_ONCE() macros.
>
> load(Acquire)/store(Release) use C's smp_load_acquire() and
> smp_store_release() macros. They internally use the appropriate
> architecture-specific instructions.
>
> v2:
> - Implement relaxed operations by using C's READ_ONCE/WRITE_ONCE
> - Rename smp_load_acquire() and smp_store_release() helpers
> - Simplify the macro to generate atomic methods
> v1: https://lore.kernel.org/rust-for-linux/20251115050305.3872412-1-fujita.tomonori@gmail.com/
>
> FUJITA Tomonori (4):
> rust: sync: Add i8/i16 atomic_load_acquire/atomic_store_release
> helpers
> rust: helpers: Add i8/i16 relaxed atomic helpers
> rust: sync: atomic: Add i8/i16 load and store support
> rust: sync: atomic: Add store_release/load_acquire tests
>
> rust/helpers/atomic_ext.c | 44 +++++++++++++++++++++++++
> rust/helpers/helpers.c | 1 +
> rust/kernel/sync/atomic/internal.rs | 48 ++++++++++++++++++++++++++++
> rust/kernel/sync/atomic/predefine.rs | 24 +++++++++++++-
> 4 files changed, 116 insertions(+), 1 deletion(-)
> create mode 100644 rust/helpers/atomic_ext.c
Boqun, any thoughts on this?
If we build Atomic<bool> on top of this patch's Atomic<u8> support, it
seems we could replace all of the Rust native atomic types currently
in use.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/4] rust: Add i8 and i16 atomic support
2025-12-03 3:41 ` [PATCH v2 0/4] rust: Add i8 and i16 atomic support FUJITA Tomonori
@ 2025-12-04 3:40 ` Boqun Feng
2025-12-04 13:12 ` FUJITA Tomonori
0 siblings, 1 reply; 18+ messages in thread
From: Boqun Feng @ 2025-12-04 3:40 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: ojeda, peterz, will, a.hindborg, aliceryhl, bjorn3_gh, dakr, gary,
lossin, mark.rutland, rust-for-linux, tmgross
On Wed, Dec 03, 2025 at 12:41:48PM +0900, FUJITA Tomonori wrote:
> On Mon, 17 Nov 2025 09:10:31 +0900
> FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>
> > This adds Atomic<i8> and Atomic<i16> support; including
> > load/store(Relaxed) and load(Acquire)/store(Release) operations.
> >
> > Relaxed operations are implemented in C's READ_ONCE() and
> > WRITE_ONCE() macros.
> >
> > load(Acquire)/store(Release) use C's smp_load_acquire() and
> > smp_store_release() macros. They internally use the appropriate
> > architecture-specific instructions.
> >
> > v2:
> > - Implement relaxed operations by using C's READ_ONCE/WRITE_ONCE
> > - Rename smp_load_acquire() and smp_store_release() helpers
> > - Simplify the macro to generate atomic methods
> > v1: https://lore.kernel.org/rust-for-linux/20251115050305.3872412-1-fujita.tomonori@gmail.com/
> >
> > FUJITA Tomonori (4):
> > rust: sync: Add i8/i16 atomic_load_acquire/atomic_store_release
> > helpers
> > rust: helpers: Add i8/i16 relaxed atomic helpers
> > rust: sync: atomic: Add i8/i16 load and store support
> > rust: sync: atomic: Add store_release/load_acquire tests
> >
> > rust/helpers/atomic_ext.c | 44 +++++++++++++++++++++++++
> > rust/helpers/helpers.c | 1 +
> > rust/kernel/sync/atomic/internal.rs | 48 ++++++++++++++++++++++++++++
> > rust/kernel/sync/atomic/predefine.rs | 24 +++++++++++++-
> > 4 files changed, 116 insertions(+), 1 deletion(-)
> > create mode 100644 rust/helpers/atomic_ext.c
>
> Boqun, any thoughts on this?
>
Sorry for the late response, the patchset looks good to me, I'm just
waiting for more reviews and some free time to take it.
Regards,
Boqun
> If we build Atomic<bool> on top of this patch's Atomic<u8> support, it
> seems we could replace all of the Rust native atomic types currently
> in use.
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/4] rust: Add i8 and i16 atomic support
2025-12-04 3:40 ` Boqun Feng
@ 2025-12-04 13:12 ` FUJITA Tomonori
0 siblings, 0 replies; 18+ messages in thread
From: FUJITA Tomonori @ 2025-12-04 13:12 UTC (permalink / raw)
To: boqun.feng
Cc: fujita.tomonori, ojeda, peterz, will, a.hindborg, aliceryhl,
bjorn3_gh, dakr, gary, lossin, mark.rutland, rust-for-linux,
tmgross
On Wed, 3 Dec 2025 19:40:59 -0800
Boqun Feng <boqun.feng@gmail.com> wrote:
>> > This adds Atomic<i8> and Atomic<i16> support; including
>> > load/store(Relaxed) and load(Acquire)/store(Release) operations.
>> >
>> > Relaxed operations are implemented in C's READ_ONCE() and
>> > WRITE_ONCE() macros.
>> >
>> > load(Acquire)/store(Release) use C's smp_load_acquire() and
>> > smp_store_release() macros. They internally use the appropriate
>> > architecture-specific instructions.
>> >
>> > v2:
>> > - Implement relaxed operations by using C's READ_ONCE/WRITE_ONCE
>> > - Rename smp_load_acquire() and smp_store_release() helpers
>> > - Simplify the macro to generate atomic methods
>> > v1: https://lore.kernel.org/rust-for-linux/20251115050305.3872412-1-fujita.tomonori@gmail.com/
>> >
>> > FUJITA Tomonori (4):
>> > rust: sync: Add i8/i16 atomic_load_acquire/atomic_store_release
>> > helpers
>> > rust: helpers: Add i8/i16 relaxed atomic helpers
>> > rust: sync: atomic: Add i8/i16 load and store support
>> > rust: sync: atomic: Add store_release/load_acquire tests
>> >
>> > rust/helpers/atomic_ext.c | 44 +++++++++++++++++++++++++
>> > rust/helpers/helpers.c | 1 +
>> > rust/kernel/sync/atomic/internal.rs | 48 ++++++++++++++++++++++++++++
>> > rust/kernel/sync/atomic/predefine.rs | 24 +++++++++++++-
>> > 4 files changed, 116 insertions(+), 1 deletion(-)
>> > create mode 100644 rust/helpers/atomic_ext.c
>>
>> Boqun, any thoughts on this?
>>
>
> Sorry for the late response, the patchset looks good to me, I'm just
> waiting for more reviews and some free time to take it.
Thanks for the update, and no problem about the late response. Please
take your time. I just wanted to make sure if there was anything I
needed to do on my side.
^ permalink raw reply [flat|nested] 18+ messages in thread