[PATCH v6 9/9] rust: sync: atomic: Add Atomic<{usize,isize}>

Boqun Feng posted 9 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH v6 9/9] rust: sync: atomic: Add Atomic<{usize,isize}>
Posted by Boqun Feng 2 months, 4 weeks ago
Add generic atomic support for `usize` and `isize`. Note that instead of
mapping directly to `atomic_long_t`, the represention type
(`AllowAtomic::Repr`) is selected based on CONFIG_64BIT. This reduces
the necessity of creating `atomic_long_*` helpers, which could save
the binary size of kernel if inline helpers are not available.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic.rs | 48 ++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index e676bc7d9275..e1e40757d7b5 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -53,6 +53,26 @@ fn delta_into_repr(d: Self::Delta) -> Self::Repr {
     }
 }
 
+// SAFETY: For 32bit kernel, `isize` has the same size and alignment with `i32` and is round-trip
+// transmutable to it, for 64bit kernel `isize` has the same size and alignment with `i64` and is
+// round-trip transmutable to it.
+unsafe impl generic::AllowAtomic for isize {
+    #[cfg(not(CONFIG_64BIT))]
+    type Repr = i32;
+    #[cfg(CONFIG_64BIT)]
+    type Repr = i64;
+}
+
+// SAFETY: `isize` is always sound to transmute back from `i32` or `i64` when their sizes are the
+// same.
+unsafe impl generic::AllowAtomicArithmetic for isize {
+    type Delta = Self;
+
+    fn delta_into_repr(d: Self::Delta) -> Self::Repr {
+        d as Self::Repr
+    }
+}
+
 // SAFETY: `u32` and `i32` has the same size and alignment, and `u32` is round-trip transmutable to
 // `i32`.
 unsafe impl generic::AllowAtomic for u32 {
@@ -83,6 +103,26 @@ fn delta_into_repr(d: Self::Delta) -> Self::Repr {
     }
 }
 
+// SAFETY: For 32bit kernel, `usize` has the same size and alignment with `i32` and is round-trip
+// transmutable to it, for 64bit kernel `usize` has the same size and alignment with `i64` and is
+// round-trip transmutable to it.
+unsafe impl generic::AllowAtomic for usize {
+    #[cfg(not(CONFIG_64BIT))]
+    type Repr = i32;
+    #[cfg(CONFIG_64BIT)]
+    type Repr = i64;
+}
+
+// SAFETY: `usize` is always sound to transmute back from `i32` or `i64` when their sizes are the
+// same.
+unsafe impl generic::AllowAtomicArithmetic for usize {
+    type Delta = Self;
+
+    fn delta_into_repr(d: Self::Delta) -> Self::Repr {
+        d as Self::Repr
+    }
+}
+
 use crate::macros::kunit_tests;
 
 #[kunit_tests(rust_atomics)]
@@ -102,7 +142,7 @@ macro_rules! for_each_type {
 
     #[test]
     fn atomic_basic_tests() {
-        for_each_type!(42 in [i32, i64, u32, u64] |v| {
+        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
             let x = Atomic::new(v);
 
             assert_eq!(v, x.load(Relaxed));
@@ -111,7 +151,7 @@ fn atomic_basic_tests() {
 
     #[test]
     fn atomic_xchg_tests() {
-        for_each_type!(42 in [i32, i64, u32, u64] |v| {
+        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
             let x = Atomic::new(v);
 
             let old = v;
@@ -124,7 +164,7 @@ fn atomic_xchg_tests() {
 
     #[test]
     fn atomic_cmpxchg_tests() {
-        for_each_type!(42 in [i32, i64, u32, u64] |v| {
+        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
             let x = Atomic::new(v);
 
             let old = v;
@@ -139,7 +179,7 @@ fn atomic_cmpxchg_tests() {
 
     #[test]
     fn atomic_arithmetic_tests() {
-        for_each_type!(42 in [i32, i64, u32, u64] |v| {
+        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
             let x = Atomic::new(v);
 
             assert_eq!(v, x.fetch_add(12, Full));
-- 
2.39.5 (Apple Git-154)
Re: [PATCH v6 9/9] rust: sync: atomic: Add Atomic<{usize,isize}>
Posted by Benno Lossin 2 months, 4 weeks ago
On Thu Jul 10, 2025 at 8:00 AM CEST, Boqun Feng wrote:
> Add generic atomic support for `usize` and `isize`. Note that instead of
> mapping directly to `atomic_long_t`, the represention type
> (`AllowAtomic::Repr`) is selected based on CONFIG_64BIT. This reduces
> the necessity of creating `atomic_long_*` helpers, which could save
> the binary size of kernel if inline helpers are not available.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>

Reviewed-by: Benno Lossin <lossin@kernel.org>

> ---
>  rust/kernel/sync/atomic.rs | 48 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> index e676bc7d9275..e1e40757d7b5 100644
> --- a/rust/kernel/sync/atomic.rs
> +++ b/rust/kernel/sync/atomic.rs
> @@ -53,6 +53,26 @@ fn delta_into_repr(d: Self::Delta) -> Self::Repr {
>      }
>  }
>  
> +// SAFETY: For 32bit kernel, `isize` has the same size and alignment with `i32` and is round-trip
> +// transmutable to it, for 64bit kernel `isize` has the same size and alignment with `i64` and is
> +// round-trip transmutable to it.
> +unsafe impl generic::AllowAtomic for isize {
> +    #[cfg(not(CONFIG_64BIT))]
> +    type Repr = i32;
> +    #[cfg(CONFIG_64BIT)]
> +    type Repr = i64;

Do we have a static assert with these cfgs that `isize` has the same
size as these?

If not, then it would probably make sense to add them now.

---
Cheers,
Benno

> +}
Re: [PATCH v6 9/9] rust: sync: atomic: Add Atomic<{usize,isize}>
Posted by Miguel Ojeda 2 months, 3 weeks ago
On Fri, Jul 11, 2025 at 11:00 AM Benno Lossin <lossin@kernel.org> wrote:
>
> Do we have a static assert with these cfgs that `isize` has the same
> size as these?
>
> If not, then it would probably make sense to add them now.

Yeah, according to e.g. Matthew et al., we may end up with 128-bit
pointers in the kernel fairly soon (e.g. a decade):

    https://lwn.net/Articles/908026/

I rescued part of what I wrote in the old `mod assumptions` which I
never got to send back then -- most of the `static_asserts` are
redundant now that we define directly the types in the `ffi` crate (I
mean, we could still assert that `size_of::<c_char>() == 1` and so on,
but they are essentially a tautology now), so I adapted the comments.
Please see below (draft).

Cheers,
Miguel
Re: [PATCH v6 9/9] rust: sync: atomic: Add Atomic<{usize,isize}>
Posted by Benno Lossin 2 months, 3 weeks ago
On Fri Jul 11, 2025 at 3:45 PM CEST, Miguel Ojeda wrote:
> On Fri, Jul 11, 2025 at 11:00 AM Benno Lossin <lossin@kernel.org> wrote:
>>
>> Do we have a static assert with these cfgs that `isize` has the same
>> size as these?
>>
>> If not, then it would probably make sense to add them now.
>
> Yeah, according to e.g. Matthew et al., we may end up with 128-bit
> pointers in the kernel fairly soon (e.g. a decade):
>
>     https://lwn.net/Articles/908026/
>
> I rescued part of what I wrote in the old `mod assumptions` which I
> never got to send back then -- most of the `static_asserts` are
> redundant now that we define directly the types in the `ffi` crate (I
> mean, we could still assert that `size_of::<c_char>() == 1` and so on,
> but they are essentially a tautology now), so I adapted the comments.
> Please see below (draft).

Ahhh yes the `mod assumptions` was the thing I remembered! I think it's
still useful as a file where we can put other global assumptions as
well.

---
Cheers,
Benno
Re: [PATCH v6 9/9] rust: sync: atomic: Add Atomic<{usize,isize}>
Posted by Boqun Feng 2 months, 3 weeks ago
On Fri, Jul 11, 2025 at 03:45:32PM +0200, Miguel Ojeda wrote:
> On Fri, Jul 11, 2025 at 11:00 AM Benno Lossin <lossin@kernel.org> wrote:
> >
> > Do we have a static assert with these cfgs that `isize` has the same
> > size as these?
> >
> > If not, then it would probably make sense to add them now.
> 
> Yeah, according to e.g. Matthew et al., we may end up with 128-bit
> pointers in the kernel fairly soon (e.g. a decade):
> 
>     https://lwn.net/Articles/908026/
> 
> I rescued part of what I wrote in the old `mod assumptions` which I
> never got to send back then -- most of the `static_asserts` are
> redundant now that we define directly the types in the `ffi` crate (I
> mean, we could still assert that `size_of::<c_char>() == 1` and so on,
> but they are essentially a tautology now), so I adapted the comments.
> Please see below (draft).
> 

Thanks Miguel.

Maybe we can do even better: having a type alias mapping to the exact
i{32,64,128} based on kernel configs? Like

(in kernel/lib.rs or ffi.rs)

// Want to buy a better name ;-)
#[cfg(CONFIG_128BIT)]
type isize_mapping = i128;
#[cfg(CONFIG_64BIT)]
type isize_mapping = i64;
#[cfg(not(any(CONFIG_128BIT, CONFIG_64BIT)))]
type isize_mapping = i32;

similar for usize.

Thoughts?

Regards,
Boqun

> Cheers,
> Miguel

> From afd58f3808bd41cfb92bf1acdf2a19081a439ca3 Mon Sep 17 00:00:00 2001
> From: Miguel Ojeda <ojeda@kernel.org>
> Date: Fri, 11 Jul 2025 15:27:27 +0200
> Subject: [PATCH] rust: ffi: assert sizes and clarify 128-bit situation
> 
> Link: https://lwn.net/Articles/908026/
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  rust/ffi.rs | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/rust/ffi.rs b/rust/ffi.rs
> index d60aad792af4..bbda56c28ca8 100644
> --- a/rust/ffi.rs
> +++ b/rust/ffi.rs
> @@ -38,11 +38,43 @@ macro_rules! alias {
>  
>      // In the kernel, `intptr_t` is defined to be `long` in all platforms, so we can map the type to
>      // `isize`.
> +    //
> +    // It is likely that the assumption that `long` is the size of a CPU register/pointer will stay
> +    // when support for 128-bit architectures is added, thus these will still mapped to `{i,u}size`.
>      c_long = isize;
>      c_ulong = usize;
>  
> +    // Since `long` will likely be 128-bit for 128-bit architectures, `long long` will likely be
> +    // increased. Thus these may happen to be either 64-bit or 128-bit in the future, and thus new
> +    // code should avoid relying on them being 64-bit.
>      c_longlong = i64;
>      c_ulonglong = u64;
>  }
>  
> +// Thus, `long` may be 32-bit, 64-bit or 128-bit.
> +const _: () = {
> +    assert!(
> +        core::mem::size_of::<c_long>()
> +            == if cfg!(CONFIG_128BIT) {
> +                core::mem::size_of::<u128>()
> +            } else if cfg!(CONFIG_64BIT) {
> +                core::mem::size_of::<u64>()
> +            } else {
> +                core::mem::size_of::<u32>()
> +            }
> +    );
> +};
> +
> +// And `long long` may be 64-bit or 128-bit.
> +const _: () = {
> +    assert!(
> +        core::mem::size_of::<c_longlong>()
> +            == if cfg!(CONFIG_64BIT) {
> +                core::mem::size_of::<u64>()
> +            } else {
> +                core::mem::size_of::<u128>()
> +            }
> +    );
> +};
> +
>  pub use core::ffi::c_void;
> 
> base-commit: d7b8f8e20813f0179d8ef519541a3527e7661d3a
> -- 
> 2.50.1
> 

Re: [PATCH v6 9/9] rust: sync: atomic: Add Atomic<{usize,isize}>
Posted by Miguel Ojeda 2 months, 3 weeks ago
On Fri, Jul 11, 2025 at 4:07 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Thanks Miguel.
>
> Maybe we can do even better: having a type alias mapping to the exact
> i{32,64,128} based on kernel configs? Like
>
> (in kernel/lib.rs or ffi.rs)
>
> // Want to buy a better name ;-)
> #[cfg(CONFIG_128BIT)]
> type isize_mapping = i128;
> #[cfg(CONFIG_64BIT)]
> type isize_mapping = i64;
> #[cfg(not(any(CONFIG_128BIT, CONFIG_64BIT)))]
> type isize_mapping = i32;
>
> similar for usize.
>
> Thoughts?

Yeah, I wondered about that too, but I don't know how common it will
be (so you may want to keep it local anyway), and I wasn't sure what
to call it either, because e.g. something like `isize_mapping` sounds
like we are talking about `c_long`.

What we want is a Rust fixed-width integer of the same size of `isize`
-- so I think you should try to pick a word that evokes a bit that
part. Something like `fixed_isize` or words like `underlying` or
`repr` perhaps?

Cheers,
Miguel
Re: [PATCH v6 9/9] rust: sync: atomic: Add Atomic<{usize,isize}>
Posted by Boqun Feng 2 months, 3 weeks ago
On Fri, Jul 11, 2025 at 04:40:15PM +0200, Miguel Ojeda wrote:
> On Fri, Jul 11, 2025 at 4:07 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Thanks Miguel.
> >
> > Maybe we can do even better: having a type alias mapping to the exact
> > i{32,64,128} based on kernel configs? Like
> >
> > (in kernel/lib.rs or ffi.rs)
> >
> > // Want to buy a better name ;-)
> > #[cfg(CONFIG_128BIT)]
> > type isize_mapping = i128;
> > #[cfg(CONFIG_64BIT)]
> > type isize_mapping = i64;
> > #[cfg(not(any(CONFIG_128BIT, CONFIG_64BIT)))]
> > type isize_mapping = i32;
> >
> > similar for usize.
> >
> > Thoughts?
> 
> Yeah, I wondered about that too, but I don't know how common it will
> be (so you may want to keep it local anyway), and I wasn't sure what

Sounds good, I will put it locally.

> to call it either, because e.g. something like `isize_mapping` sounds
> like we are talking about `c_long`.
> 
> What we want is a Rust fixed-width integer of the same size of `isize`
> -- so I think you should try to pick a word that evokes a bit that
> part. Something like `fixed_isize` or words like `underlying` or
> `repr` perhaps?
> 

I end up calling it `isize_atomic_repr`, and create the diff as below:

------------>8
diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index 7ff87b2b49d6..bb0d3d49e3f7 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -53,14 +53,21 @@ fn delta_into_repr(d: Self::Delta) -> Self::Repr {
     }
 }

+#[allow(non_camel_case_types)]
+#[cfg(not(CONFIG_64BIT))]
+type isize_atomic_repr = i32;
+#[allow(non_camel_case_types)]
+#[cfg(CONFIG_64BIT)]
+type isize_atomic_repr = i64;
+
+crate::static_assert!(core::mem::size_of::<isize>() == core::mem::size_of::<isize_atomic_repr>());
+crate::static_assert!(core::mem::align_of::<isize>() == core::mem::align_of::<isize_atomic_repr>());
+
 // SAFETY: For 32bit kernel, `isize` has the same size and alignment with `i32` and is round-trip
 // transmutable to it, for 64bit kernel `isize` has the same size and alignment with `i64` and is
 // round-trip transmutable to it.
 unsafe impl generic::AllowAtomic for isize {
-    #[cfg(not(CONFIG_64BIT))]
-    type Repr = i32;
-    #[cfg(CONFIG_64BIT)]
-    type Repr = i64;
+    type Repr = isize_atomic_repr;
 }

 // SAFETY: `isize` is always sound to transmute back from `i32` or `i64` when their sizes are the


Seems good?

Regards,
Boqun

> Cheers,
> Miguel
Re: [PATCH v6 9/9] rust: sync: atomic: Add Atomic<{usize,isize}>
Posted by Miguel Ojeda 2 months, 3 weeks ago
On Fri, Jul 11, 2025 at 5:47 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Seems good?

Yeah, thanks!

I will send the other one about 128-bit independently.

Cheers,
Miguel