Hi,

This is another RFC version of LKMM atomics in Rust, you can find the
previous versions:

v1: https://lore.kernel.org/rust-for-linux/20240612223025.1158537-1-boqun.feng@gmail.com/
wip: https://lore.kernel.org/rust-for-linux/20240322233838.868874-1-boqun.feng@gmail.com/

I add a few more people Cced this time, so if you're curious about why
we choose to implement LKMM atomics instead of using the Rust ones, you
can find some explanation:

* In my Kangrejos talk: https://lwn.net/Articles/993785/
* In Linus' email: https://lore.kernel.org/rust-for-linux/CAHk-=whkQk=zq5XiMcaU3xj4v69+jyoP-y6Sywhq-TvxSSvfEA@mail.gmail.com/

This time, I try implementing a generic atomic type `Atomic<T>`, since
Benno and Gary suggested last time, and also Rust standard library is
also going to that direction [1].

Honestly, a generic atomic type is still not quite necessary for myself,
but here are a few reasons that it's useful:

*	It's useful for type alias, for example, if you have:

	type c_long = isize;

	Then `Atomic<c_clong>` and `Atomic<isize>` is the same type,
	this may make FFI code (mapping a C type to a Rust type or vice
	versa) more readable.

*	In kernel, we sometimes switch atomic to percpu for better
	scalabity, percpu is naturally a generic type, because it can
	have data that is larger than machine word size. Making atomic
	generic ease the potential switching/refactoring.

*	Generic atomics provide all the functionalities that non-generic
	atomics could have.

That said, currently "generic" is limited to a few types: the type must
be the same size as atomic_t or atomic64_t, other than basic types, only
#[repr(<basic types>)] struct can be used in a `Atomic<T>`.

Also this is not a full feature set patchset, things like different
arithmetic operations and bit operations are still missing, they can be
either future work or for future versions.

I included an RCU pointer implementation as one example of the usage, so
a patch from Danilo is added, but I will drop it once his patch merged.

This is based on today's rust-next, and I've run all kunit tests from
the doc test on x86, arm64 and riscv.

Feedbacks and comments are welcome! Thanks.

Regards,
Boqun

[1]: https://github.com/rust-lang/rust/issues/130539

Boqun Feng (12):
  rust: Introduce atomic API helpers
  rust: sync: Add basic atomic operation mapping framework
  rust: sync: atomic: Add ordering annotation types
  rust: sync: atomic: Add generic atomics
  rust: sync: atomic: Add atomic {cmp,}xchg operations
  rust: sync: atomic: Add the framework of arithmetic operations
  rust: sync: atomic: Add Atomic<u{32,64}>
  rust: sync: atomic: Add Atomic<{usize,isize}>
  rust: sync: atomic: Add Atomic<*mut T>
  rust: sync: atomic: Add arithmetic ops for Atomic<*mut T>
  rust: sync: Add memory barriers
  rust: sync: rcu: Add RCU protected pointer

Wedson Almeida Filho (1):
  rust: add rcu abstraction

 MAINTAINERS                               |    4 +-
 rust/helpers/atomic.c                     | 1038 +++++++++++++++++++++
 rust/helpers/helpers.c                    |    3 +
 rust/helpers/rcu.c                        |   13 +
 rust/kernel/sync.rs                       |    3 +
 rust/kernel/sync/atomic.rs                |  228 +++++
 rust/kernel/sync/atomic/generic.rs        |  516 ++++++++++
 rust/kernel/sync/atomic/ops.rs            |  199 ++++
 rust/kernel/sync/atomic/ordering.rs       |   94 ++
 rust/kernel/sync/barrier.rs               |   67 ++
 rust/kernel/sync/rcu.rs                   |  319 +++++++
 scripts/atomic/gen-atomics.sh             |    1 +
 scripts/atomic/gen-rust-atomic-helpers.sh |   65 ++
 13 files changed, 2549 insertions(+), 1 deletion(-)
 create mode 100644 rust/helpers/atomic.c
 create mode 100644 rust/helpers/rcu.c
 create mode 100644 rust/kernel/sync/atomic.rs
 create mode 100644 rust/kernel/sync/atomic/generic.rs
 create mode 100644 rust/kernel/sync/atomic/ops.rs
 create mode 100644 rust/kernel/sync/atomic/ordering.rs
 create mode 100644 rust/kernel/sync/barrier.rs
 create mode 100644 rust/kernel/sync/rcu.rs
 create mode 100755 scripts/atomic/gen-rust-atomic-helpers.sh

-- 
2.45.2

On Fri, Nov 1, 2024 at 7:03 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> This time, I try implementing a generic atomic type `Atomic<T>`, since
> Benno and Gary suggested last time, and also Rust standard library is
> also going to that direction [1].

I would like to thank Boqun for trying out this approach, even when he
wasn't (and maybe still isn't) convinced.

Cheers,
Miguel

On Fri, 1 Nov 2024 at 14:04, Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Hi,
>
> This is another RFC version of LKMM atomics in Rust, you can find the
> previous versions:
>
> v1: https://lore.kernel.org/rust-for-linux/20240612223025.1158537-1-boqun.feng@gmail.com/
> wip: https://lore.kernel.org/rust-for-linux/20240322233838.868874-1-boqun.feng@gmail.com/
>
> I add a few more people Cced this time, so if you're curious about why
> we choose to implement LKMM atomics instead of using the Rust ones, you
> can find some explanation:
>
> * In my Kangrejos talk: https://lwn.net/Articles/993785/
> * In Linus' email: https://lore.kernel.org/rust-for-linux/CAHk-=whkQk=zq5XiMcaU3xj4v69+jyoP-y6Sywhq-TvxSSvfEA@mail.gmail.com/
>
> This time, I try implementing a generic atomic type `Atomic<T>`, since
> Benno and Gary suggested last time, and also Rust standard library is
> also going to that direction [1].
>
> Honestly, a generic atomic type is still not quite necessary for myself,
> but here are a few reasons that it's useful:
>
> *       It's useful for type alias, for example, if you have:
>
>         type c_long = isize;
>
>         Then `Atomic<c_clong>` and `Atomic<isize>` is the same type,
>         this may make FFI code (mapping a C type to a Rust type or vice
>         versa) more readable.
>
> *       In kernel, we sometimes switch atomic to percpu for better
>         scalabity, percpu is naturally a generic type, because it can
>         have data that is larger than machine word size. Making atomic
>         generic ease the potential switching/refactoring.
>
> *       Generic atomics provide all the functionalities that non-generic
>         atomics could have.
>
> That said, currently "generic" is limited to a few types: the type must
> be the same size as atomic_t or atomic64_t, other than basic types, only
> #[repr(<basic types>)] struct can be used in a `Atomic<T>`.
>
> Also this is not a full feature set patchset, things like different
> arithmetic operations and bit operations are still missing, they can be
> either future work or for future versions.
>
> I included an RCU pointer implementation as one example of the usage, so
> a patch from Danilo is added, but I will drop it once his patch merged.
>
> This is based on today's rust-next, and I've run all kunit tests from
> the doc test on x86, arm64 and riscv.
>
> Feedbacks and comments are welcome! Thanks.
>
> Regards,
> Boqun
>
> [1]: https://github.com/rust-lang/rust/issues/130539
>

Thanks, Boqun.

I played around a bit with porting the blk-mq atomic code to this. As
neither an expert in Rust nor an expert in atomics, this is probably
both non-idiomatic and wrong, but unlike the `core` atomics, it
provides an Atomic::<u64> on 32-bit systems, which gets UML's 32-bit
build working again.

Diff below -- I'm not likely to have much time to work on this again
soon, so feel free to pick it up/fix it if it suits.

Thanks,
-- David

---
diff --git a/rust/kernel/block/mq/operations.rs
b/rust/kernel/block/mq/operations.rs
index 9ba7fdfeb4b2..822d64230e11 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -11,7 +11,8 @@
     error::{from_result, Result},
     types::ARef,
 };
-use core::{marker::PhantomData, sync::atomic::AtomicU64,
sync::atomic::Ordering};
+use core::marker::PhantomData;
+use kernel::sync::atomic::{Atomic, Relaxed};

 /// Implement this trait to interface blk-mq as block devices.
 ///
@@ -77,7 +78,7 @@ impl<T: Operations> OperationsVTable<T> {
         let request = unsafe { &*(*bd).rq.cast::<Request<T>>() };

         // One refcount for the ARef, one for being in flight
-        request.wrapper_ref().refcount().store(2, Ordering::Relaxed);
+        request.wrapper_ref().refcount().store(2, Relaxed);

         // SAFETY:
         //  - We own a refcount that we took above. We pass that to `ARef`.
@@ -186,7 +187,7 @@ impl<T: Operations> OperationsVTable<T> {

             // SAFETY: The refcount field is allocated but not initialized, so
             // it is valid for writes.
-            unsafe {
RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(AtomicU64::new(0))
};
+            unsafe {
RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(Atomic::<u64>::new(0))
};

             Ok(0)
         })
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 7943f43b9575..8d4060d65159 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -13,8 +13,8 @@
 use core::{
     marker::PhantomData,
     ptr::{addr_of_mut, NonNull},
-    sync::atomic::{AtomicU64, Ordering},
 };
+use kernel::sync::atomic::{Atomic, Relaxed};

 /// A wrapper around a blk-mq [`struct request`]. This represents an
IO request.
 ///
@@ -102,8 +102,7 @@ fn try_set_end(this: ARef<Self>) -> Result<*mut
bindings::request, ARef<Self>> {
         if let Err(_old) = this.wrapper_ref().refcount().compare_exchange(
             2,
             0,
-            Ordering::Relaxed,
-            Ordering::Relaxed,
+            Relaxed
         ) {
             return Err(this);
         }
@@ -168,13 +167,13 @@ pub(crate) struct RequestDataWrapper {
     /// - 0: The request is owned by C block layer.
     /// - 1: The request is owned by Rust abstractions but there are
no [`ARef`] references to it.
     /// - 2+: There are [`ARef`] references to the request.
-    refcount: AtomicU64,
+    refcount: Atomic::<u64>,
 }

 impl RequestDataWrapper {
     /// Return a reference to the refcount of the request that is embedding
     /// `self`.
-    pub(crate) fn refcount(&self) -> &AtomicU64 {
+    pub(crate) fn refcount(&self) -> &Atomic::<u64> {
         &self.refcount
     }

@@ -184,7 +183,7 @@ pub(crate) fn refcount(&self) -> &AtomicU64 {
     /// # Safety
     ///
     /// - `this` must point to a live allocation of at least the size
of `Self`.
-    pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut AtomicU64 {
+    pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut Atomic::<u64> {
         // SAFETY: Because of the safety requirements of this function, the
         // field projection is safe.
         unsafe { addr_of_mut!((*this).refcount) }
@@ -202,28 +201,22 @@ unsafe impl<T: Operations> Sync for Request<T> {}

 /// Store the result of `op(target.load())` in target, returning new value of
 /// target.
-fn atomic_relaxed_op_return(target: &AtomicU64, op: impl Fn(u64) ->
u64) -> u64 {
-    let old = target.fetch_update(Ordering::Relaxed,
Ordering::Relaxed, |x| Some(op(x)));
-
-    // SAFETY: Because the operation passed to `fetch_update` above always
-    // return `Some`, `old` will always be `Ok`.
-    let old = unsafe { old.unwrap_unchecked() };
-
-    op(old)
+fn atomic_relaxed_op_return(target: &Atomic::<u64>, op: impl Fn(u64)
-> u64) -> u64 {
+    let old = target.load(Relaxed);
+    let new_val = op(old);
+    target.compare_exchange(old, new_val, Relaxed);
+    old
 }

 /// Store the result of `op(target.load)` in `target` if `target.load() !=
 /// pred`, returning [`true`] if the target was updated.
-fn atomic_relaxed_op_unless(target: &AtomicU64, op: impl Fn(u64) ->
u64, pred: u64) -> bool {
-    target
-        .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |x| {
-            if x == pred {
-                None
-            } else {
-                Some(op(x))
-            }
-        })
-        .is_ok()
+fn atomic_relaxed_op_unless(target: &Atomic::<u64>, op: impl Fn(u64)
-> u64, pred: u64) -> bool {
+    let old = target.load(Relaxed);
+    if old == pred {
+        false
+    } else {
+        target.compare_exchange(old, op(old), Relaxed).is_ok()
+    }
 }

 // SAFETY: All instances of `Request<T>` are reference counted. This

On Sat, Nov 02, 2024 at 03:35:36PM +0800, David Gow wrote:
> On Fri, 1 Nov 2024 at 14:04, Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Hi,
> >
> > This is another RFC version of LKMM atomics in Rust, you can find the
> > previous versions:
> >
> > v1: https://lore.kernel.org/rust-for-linux/20240612223025.1158537-1-boqun.feng@gmail.com/
> > wip: https://lore.kernel.org/rust-for-linux/20240322233838.868874-1-boqun.feng@gmail.com/
> >
> > I add a few more people Cced this time, so if you're curious about why
> > we choose to implement LKMM atomics instead of using the Rust ones, you
> > can find some explanation:
> >
> > * In my Kangrejos talk: https://lwn.net/Articles/993785/
> > * In Linus' email: https://lore.kernel.org/rust-for-linux/CAHk-=whkQk=zq5XiMcaU3xj4v69+jyoP-y6Sywhq-TvxSSvfEA@mail.gmail.com/
> >
> > This time, I try implementing a generic atomic type `Atomic<T>`, since
> > Benno and Gary suggested last time, and also Rust standard library is
> > also going to that direction [1].
> >
> > Honestly, a generic atomic type is still not quite necessary for myself,
> > but here are a few reasons that it's useful:
> >
> > *       It's useful for type alias, for example, if you have:
> >
> >         type c_long = isize;
> >
> >         Then `Atomic<c_clong>` and `Atomic<isize>` is the same type,
> >         this may make FFI code (mapping a C type to a Rust type or vice
> >         versa) more readable.
> >
> > *       In kernel, we sometimes switch atomic to percpu for better
> >         scalabity, percpu is naturally a generic type, because it can
> >         have data that is larger than machine word size. Making atomic
> >         generic ease the potential switching/refactoring.
> >
> > *       Generic atomics provide all the functionalities that non-generic
> >         atomics could have.
> >
> > That said, currently "generic" is limited to a few types: the type must
> > be the same size as atomic_t or atomic64_t, other than basic types, only
> > #[repr(<basic types>)] struct can be used in a `Atomic<T>`.
> >
> > Also this is not a full feature set patchset, things like different
> > arithmetic operations and bit operations are still missing, they can be
> > either future work or for future versions.
> >
> > I included an RCU pointer implementation as one example of the usage, so
> > a patch from Danilo is added, but I will drop it once his patch merged.
> >
> > This is based on today's rust-next, and I've run all kunit tests from
> > the doc test on x86, arm64 and riscv.
> >
> > Feedbacks and comments are welcome! Thanks.
> >
> > Regards,
> > Boqun
> >
> > [1]: https://github.com/rust-lang/rust/issues/130539
> >
> 
> Thanks, Boqun.
> 

Hi David,

> I played around a bit with porting the blk-mq atomic code to this. As
> neither an expert in Rust nor an expert in atomics, this is probably
> both non-idiomatic and wrong, but unlike the `core` atomics, it
> provides an Atomic::<u64> on 32-bit systems, which gets UML's 32-bit
> build working again.
> 
> Diff below -- I'm not likely to have much time to work on this again
> soon, so feel free to pick it up/fix it if it suits.
> 

Thanks. These look good to me, however, I think I prefer Gary's patch
for this:

	https://lore.kernel.org/lkml/20250219201602.1898383-4-gary@garyguo.net/

therefore, I won't take this into the next version. But thank you for
taking a look!

Regards,
Boqun

> Thanks,
> -- David
> 
> ---
> diff --git a/rust/kernel/block/mq/operations.rs
> b/rust/kernel/block/mq/operations.rs
> index 9ba7fdfeb4b2..822d64230e11 100644
> --- a/rust/kernel/block/mq/operations.rs
> +++ b/rust/kernel/block/mq/operations.rs
> @@ -11,7 +11,8 @@
>      error::{from_result, Result},
>      types::ARef,
>  };
> -use core::{marker::PhantomData, sync::atomic::AtomicU64,
> sync::atomic::Ordering};
> +use core::marker::PhantomData;
> +use kernel::sync::atomic::{Atomic, Relaxed};
> 
>  /// Implement this trait to interface blk-mq as block devices.
>  ///
> @@ -77,7 +78,7 @@ impl<T: Operations> OperationsVTable<T> {
>          let request = unsafe { &*(*bd).rq.cast::<Request<T>>() };
> 
>          // One refcount for the ARef, one for being in flight
> -        request.wrapper_ref().refcount().store(2, Ordering::Relaxed);
> +        request.wrapper_ref().refcount().store(2, Relaxed);
> 
>          // SAFETY:
>          //  - We own a refcount that we took above. We pass that to `ARef`.
> @@ -186,7 +187,7 @@ impl<T: Operations> OperationsVTable<T> {
> 
>              // SAFETY: The refcount field is allocated but not initialized, so
>              // it is valid for writes.
> -            unsafe {
> RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(AtomicU64::new(0))
> };
> +            unsafe {
> RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(Atomic::<u64>::new(0))
> };
> 
>              Ok(0)
>          })
> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
> index 7943f43b9575..8d4060d65159 100644
> --- a/rust/kernel/block/mq/request.rs
> +++ b/rust/kernel/block/mq/request.rs
> @@ -13,8 +13,8 @@
>  use core::{
>      marker::PhantomData,
>      ptr::{addr_of_mut, NonNull},
> -    sync::atomic::{AtomicU64, Ordering},
>  };
> +use kernel::sync::atomic::{Atomic, Relaxed};
> 
>  /// A wrapper around a blk-mq [`struct request`]. This represents an
> IO request.
>  ///
> @@ -102,8 +102,7 @@ fn try_set_end(this: ARef<Self>) -> Result<*mut
> bindings::request, ARef<Self>> {
>          if let Err(_old) = this.wrapper_ref().refcount().compare_exchange(
>              2,
>              0,
> -            Ordering::Relaxed,
> -            Ordering::Relaxed,
> +            Relaxed
>          ) {
>              return Err(this);
>          }
> @@ -168,13 +167,13 @@ pub(crate) struct RequestDataWrapper {
>      /// - 0: The request is owned by C block layer.
>      /// - 1: The request is owned by Rust abstractions but there are
> no [`ARef`] references to it.
>      /// - 2+: There are [`ARef`] references to the request.
> -    refcount: AtomicU64,
> +    refcount: Atomic::<u64>,
>  }
> 
>  impl RequestDataWrapper {
>      /// Return a reference to the refcount of the request that is embedding
>      /// `self`.
> -    pub(crate) fn refcount(&self) -> &AtomicU64 {
> +    pub(crate) fn refcount(&self) -> &Atomic::<u64> {
>          &self.refcount
>      }
> 
> @@ -184,7 +183,7 @@ pub(crate) fn refcount(&self) -> &AtomicU64 {
>      /// # Safety
>      ///
>      /// - `this` must point to a live allocation of at least the size
> of `Self`.
> -    pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut AtomicU64 {
> +    pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut Atomic::<u64> {
>          // SAFETY: Because of the safety requirements of this function, the
>          // field projection is safe.
>          unsafe { addr_of_mut!((*this).refcount) }
> @@ -202,28 +201,22 @@ unsafe impl<T: Operations> Sync for Request<T> {}
> 
>  /// Store the result of `op(target.load())` in target, returning new value of
>  /// target.
> -fn atomic_relaxed_op_return(target: &AtomicU64, op: impl Fn(u64) ->
> u64) -> u64 {
> -    let old = target.fetch_update(Ordering::Relaxed,
> Ordering::Relaxed, |x| Some(op(x)));
> -
> -    // SAFETY: Because the operation passed to `fetch_update` above always
> -    // return `Some`, `old` will always be `Ok`.
> -    let old = unsafe { old.unwrap_unchecked() };
> -
> -    op(old)
> +fn atomic_relaxed_op_return(target: &Atomic::<u64>, op: impl Fn(u64)
> -> u64) -> u64 {
> +    let old = target.load(Relaxed);
> +    let new_val = op(old);
> +    target.compare_exchange(old, new_val, Relaxed);
> +    old
>  }
> 
>  /// Store the result of `op(target.load)` in `target` if `target.load() !=
>  /// pred`, returning [`true`] if the target was updated.
> -fn atomic_relaxed_op_unless(target: &AtomicU64, op: impl Fn(u64) ->
> u64, pred: u64) -> bool {
> -    target
> -        .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |x| {
> -            if x == pred {
> -                None
> -            } else {
> -                Some(op(x))
> -            }
> -        })
> -        .is_ok()
> +fn atomic_relaxed_op_unless(target: &Atomic::<u64>, op: impl Fn(u64)
> -> u64, pred: u64) -> bool {
> +    let old = target.load(Relaxed);
> +    if old == pred {
> +        false
> +    } else {
> +        target.compare_exchange(old, op(old), Relaxed).is_ok()
> +    }
>  }
> 
>  // SAFETY: All instances of `Request<T>` are reference counted. This