rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2 00/13] LKMM *generic* atomics in Rust
@ 2024-11-01  6:02 Boqun Feng
  2024-11-01  6:02 ` [RFC v2 01/13] rust: Introduce atomic API helpers Boqun Feng
                   ` (14 more replies)
  0 siblings, 15 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RFC v2 01/13] rust: Introduce atomic API helpers
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:02 ` [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework Boqun Feng
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

In order to support LKMM atomics in Rust, add rust_helper_* for atomic
APIs. These helpers ensure the implementation of LKMM atomics in Rust is
the same as in C. This could save the maintenance burden of having two
similar atomic implementations in asm.

Originally-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/helpers/atomic.c                     | 1038 +++++++++++++++++++++
 rust/helpers/helpers.c                    |    1 +
 scripts/atomic/gen-atomics.sh             |    1 +
 scripts/atomic/gen-rust-atomic-helpers.sh |   65 ++
 4 files changed, 1105 insertions(+)
 create mode 100644 rust/helpers/atomic.c
 create mode 100755 scripts/atomic/gen-rust-atomic-helpers.sh

diff --git a/rust/helpers/atomic.c b/rust/helpers/atomic.c
new file mode 100644
index 000000000000..00bf10887928
--- /dev/null
+++ b/rust/helpers/atomic.c
@@ -0,0 +1,1038 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Generated by scripts/atomic/gen-rust-atomic-helpers.sh
+// DO NOT MODIFY THIS FILE DIRECTLY
+
+/*
+ * This file provides helpers for the various atomic functions for Rust.
+ */
+#ifndef _RUST_ATOMIC_API_H
+#define _RUST_ATOMIC_API_H
+
+#include <linux/atomic.h>
+
+// TODO: Remove this after LTO helper support is added.
+#define __rust_helper
+
+__rust_helper int
+rust_helper_atomic_read(const atomic_t *v)
+{
+	return atomic_read(v);
+}
+
+__rust_helper int
+rust_helper_atomic_read_acquire(const atomic_t *v)
+{
+	return atomic_read_acquire(v);
+}
+
+__rust_helper void
+rust_helper_atomic_set(atomic_t *v, int i)
+{
+	atomic_set(v, i);
+}
+
+__rust_helper void
+rust_helper_atomic_set_release(atomic_t *v, int i)
+{
+	atomic_set_release(v, i);
+}
+
+__rust_helper void
+rust_helper_atomic_add(int i, atomic_t *v)
+{
+	atomic_add(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_add_return(int i, atomic_t *v)
+{
+	return atomic_add_return(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_add_return_acquire(int i, atomic_t *v)
+{
+	return atomic_add_return_acquire(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_add_return_release(int i, atomic_t *v)
+{
+	return atomic_add_return_release(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_add_return_relaxed(int i, atomic_t *v)
+{
+	return atomic_add_return_relaxed(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_add(int i, atomic_t *v)
+{
+	return atomic_fetch_add(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_add_acquire(int i, atomic_t *v)
+{
+	return atomic_fetch_add_acquire(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_add_release(int i, atomic_t *v)
+{
+	return atomic_fetch_add_release(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_add_relaxed(int i, atomic_t *v)
+{
+	return atomic_fetch_add_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic_sub(int i, atomic_t *v)
+{
+	atomic_sub(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_sub_return(int i, atomic_t *v)
+{
+	return atomic_sub_return(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_sub_return_acquire(int i, atomic_t *v)
+{
+	return atomic_sub_return_acquire(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_sub_return_release(int i, atomic_t *v)
+{
+	return atomic_sub_return_release(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_sub_return_relaxed(int i, atomic_t *v)
+{
+	return atomic_sub_return_relaxed(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_sub(int i, atomic_t *v)
+{
+	return atomic_fetch_sub(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_sub_acquire(int i, atomic_t *v)
+{
+	return atomic_fetch_sub_acquire(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_sub_release(int i, atomic_t *v)
+{
+	return atomic_fetch_sub_release(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_sub_relaxed(int i, atomic_t *v)
+{
+	return atomic_fetch_sub_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic_inc(atomic_t *v)
+{
+	atomic_inc(v);
+}
+
+__rust_helper int
+rust_helper_atomic_inc_return(atomic_t *v)
+{
+	return atomic_inc_return(v);
+}
+
+__rust_helper int
+rust_helper_atomic_inc_return_acquire(atomic_t *v)
+{
+	return atomic_inc_return_acquire(v);
+}
+
+__rust_helper int
+rust_helper_atomic_inc_return_release(atomic_t *v)
+{
+	return atomic_inc_return_release(v);
+}
+
+__rust_helper int
+rust_helper_atomic_inc_return_relaxed(atomic_t *v)
+{
+	return atomic_inc_return_relaxed(v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_inc(atomic_t *v)
+{
+	return atomic_fetch_inc(v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_inc_acquire(atomic_t *v)
+{
+	return atomic_fetch_inc_acquire(v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_inc_release(atomic_t *v)
+{
+	return atomic_fetch_inc_release(v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_inc_relaxed(atomic_t *v)
+{
+	return atomic_fetch_inc_relaxed(v);
+}
+
+__rust_helper void
+rust_helper_atomic_dec(atomic_t *v)
+{
+	atomic_dec(v);
+}
+
+__rust_helper int
+rust_helper_atomic_dec_return(atomic_t *v)
+{
+	return atomic_dec_return(v);
+}
+
+__rust_helper int
+rust_helper_atomic_dec_return_acquire(atomic_t *v)
+{
+	return atomic_dec_return_acquire(v);
+}
+
+__rust_helper int
+rust_helper_atomic_dec_return_release(atomic_t *v)
+{
+	return atomic_dec_return_release(v);
+}
+
+__rust_helper int
+rust_helper_atomic_dec_return_relaxed(atomic_t *v)
+{
+	return atomic_dec_return_relaxed(v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_dec(atomic_t *v)
+{
+	return atomic_fetch_dec(v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_dec_acquire(atomic_t *v)
+{
+	return atomic_fetch_dec_acquire(v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_dec_release(atomic_t *v)
+{
+	return atomic_fetch_dec_release(v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_dec_relaxed(atomic_t *v)
+{
+	return atomic_fetch_dec_relaxed(v);
+}
+
+__rust_helper void
+rust_helper_atomic_and(int i, atomic_t *v)
+{
+	atomic_and(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_and(int i, atomic_t *v)
+{
+	return atomic_fetch_and(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_and_acquire(int i, atomic_t *v)
+{
+	return atomic_fetch_and_acquire(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_and_release(int i, atomic_t *v)
+{
+	return atomic_fetch_and_release(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_and_relaxed(int i, atomic_t *v)
+{
+	return atomic_fetch_and_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic_andnot(int i, atomic_t *v)
+{
+	atomic_andnot(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_andnot(int i, atomic_t *v)
+{
+	return atomic_fetch_andnot(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_andnot_acquire(int i, atomic_t *v)
+{
+	return atomic_fetch_andnot_acquire(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_andnot_release(int i, atomic_t *v)
+{
+	return atomic_fetch_andnot_release(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_andnot_relaxed(int i, atomic_t *v)
+{
+	return atomic_fetch_andnot_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic_or(int i, atomic_t *v)
+{
+	atomic_or(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_or(int i, atomic_t *v)
+{
+	return atomic_fetch_or(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_or_acquire(int i, atomic_t *v)
+{
+	return atomic_fetch_or_acquire(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_or_release(int i, atomic_t *v)
+{
+	return atomic_fetch_or_release(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_or_relaxed(int i, atomic_t *v)
+{
+	return atomic_fetch_or_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic_xor(int i, atomic_t *v)
+{
+	atomic_xor(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_xor(int i, atomic_t *v)
+{
+	return atomic_fetch_xor(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_xor_acquire(int i, atomic_t *v)
+{
+	return atomic_fetch_xor_acquire(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_xor_release(int i, atomic_t *v)
+{
+	return atomic_fetch_xor_release(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_xor_relaxed(int i, atomic_t *v)
+{
+	return atomic_fetch_xor_relaxed(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_xchg(atomic_t *v, int new)
+{
+	return atomic_xchg(v, new);
+}
+
+__rust_helper int
+rust_helper_atomic_xchg_acquire(atomic_t *v, int new)
+{
+	return atomic_xchg_acquire(v, new);
+}
+
+__rust_helper int
+rust_helper_atomic_xchg_release(atomic_t *v, int new)
+{
+	return atomic_xchg_release(v, new);
+}
+
+__rust_helper int
+rust_helper_atomic_xchg_relaxed(atomic_t *v, int new)
+{
+	return atomic_xchg_relaxed(v, new);
+}
+
+__rust_helper int
+rust_helper_atomic_cmpxchg(atomic_t *v, int old, int new)
+{
+	return atomic_cmpxchg(v, old, new);
+}
+
+__rust_helper int
+rust_helper_atomic_cmpxchg_acquire(atomic_t *v, int old, int new)
+{
+	return atomic_cmpxchg_acquire(v, old, new);
+}
+
+__rust_helper int
+rust_helper_atomic_cmpxchg_release(atomic_t *v, int old, int new)
+{
+	return atomic_cmpxchg_release(v, old, new);
+}
+
+__rust_helper int
+rust_helper_atomic_cmpxchg_relaxed(atomic_t *v, int old, int new)
+{
+	return atomic_cmpxchg_relaxed(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic_try_cmpxchg(atomic_t *v, int *old, int new)
+{
+	return atomic_try_cmpxchg(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic_try_cmpxchg_acquire(atomic_t *v, int *old, int new)
+{
+	return atomic_try_cmpxchg_acquire(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic_try_cmpxchg_release(atomic_t *v, int *old, int new)
+{
+	return atomic_try_cmpxchg_release(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic_try_cmpxchg_relaxed(atomic_t *v, int *old, int new)
+{
+	return atomic_try_cmpxchg_relaxed(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic_sub_and_test(int i, atomic_t *v)
+{
+	return atomic_sub_and_test(i, v);
+}
+
+__rust_helper bool
+rust_helper_atomic_dec_and_test(atomic_t *v)
+{
+	return atomic_dec_and_test(v);
+}
+
+__rust_helper bool
+rust_helper_atomic_inc_and_test(atomic_t *v)
+{
+	return atomic_inc_and_test(v);
+}
+
+__rust_helper bool
+rust_helper_atomic_add_negative(int i, atomic_t *v)
+{
+	return atomic_add_negative(i, v);
+}
+
+__rust_helper bool
+rust_helper_atomic_add_negative_acquire(int i, atomic_t *v)
+{
+	return atomic_add_negative_acquire(i, v);
+}
+
+__rust_helper bool
+rust_helper_atomic_add_negative_release(int i, atomic_t *v)
+{
+	return atomic_add_negative_release(i, v);
+}
+
+__rust_helper bool
+rust_helper_atomic_add_negative_relaxed(int i, atomic_t *v)
+{
+	return atomic_add_negative_relaxed(i, v);
+}
+
+__rust_helper int
+rust_helper_atomic_fetch_add_unless(atomic_t *v, int a, int u)
+{
+	return atomic_fetch_add_unless(v, a, u);
+}
+
+__rust_helper bool
+rust_helper_atomic_add_unless(atomic_t *v, int a, int u)
+{
+	return atomic_add_unless(v, a, u);
+}
+
+__rust_helper bool
+rust_helper_atomic_inc_not_zero(atomic_t *v)
+{
+	return atomic_inc_not_zero(v);
+}
+
+__rust_helper bool
+rust_helper_atomic_inc_unless_negative(atomic_t *v)
+{
+	return atomic_inc_unless_negative(v);
+}
+
+__rust_helper bool
+rust_helper_atomic_dec_unless_positive(atomic_t *v)
+{
+	return atomic_dec_unless_positive(v);
+}
+
+__rust_helper int
+rust_helper_atomic_dec_if_positive(atomic_t *v)
+{
+	return atomic_dec_if_positive(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_read(const atomic64_t *v)
+{
+	return atomic64_read(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_read_acquire(const atomic64_t *v)
+{
+	return atomic64_read_acquire(v);
+}
+
+__rust_helper void
+rust_helper_atomic64_set(atomic64_t *v, s64 i)
+{
+	atomic64_set(v, i);
+}
+
+__rust_helper void
+rust_helper_atomic64_set_release(atomic64_t *v, s64 i)
+{
+	atomic64_set_release(v, i);
+}
+
+__rust_helper void
+rust_helper_atomic64_add(s64 i, atomic64_t *v)
+{
+	atomic64_add(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_add_return(s64 i, atomic64_t *v)
+{
+	return atomic64_add_return(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_add_return_acquire(s64 i, atomic64_t *v)
+{
+	return atomic64_add_return_acquire(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_add_return_release(s64 i, atomic64_t *v)
+{
+	return atomic64_add_return_release(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_add_return_relaxed(s64 i, atomic64_t *v)
+{
+	return atomic64_add_return_relaxed(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_add(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_add(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_add_acquire(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_add_acquire(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_add_release(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_add_release(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_add_relaxed(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_add_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic64_sub(s64 i, atomic64_t *v)
+{
+	atomic64_sub(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_sub_return(s64 i, atomic64_t *v)
+{
+	return atomic64_sub_return(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_sub_return_acquire(s64 i, atomic64_t *v)
+{
+	return atomic64_sub_return_acquire(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_sub_return_release(s64 i, atomic64_t *v)
+{
+	return atomic64_sub_return_release(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_sub_return_relaxed(s64 i, atomic64_t *v)
+{
+	return atomic64_sub_return_relaxed(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_sub(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_sub(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_sub_acquire(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_sub_acquire(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_sub_release(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_sub_release(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_sub_relaxed(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_sub_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic64_inc(atomic64_t *v)
+{
+	atomic64_inc(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_inc_return(atomic64_t *v)
+{
+	return atomic64_inc_return(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_inc_return_acquire(atomic64_t *v)
+{
+	return atomic64_inc_return_acquire(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_inc_return_release(atomic64_t *v)
+{
+	return atomic64_inc_return_release(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_inc_return_relaxed(atomic64_t *v)
+{
+	return atomic64_inc_return_relaxed(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_inc(atomic64_t *v)
+{
+	return atomic64_fetch_inc(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_inc_acquire(atomic64_t *v)
+{
+	return atomic64_fetch_inc_acquire(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_inc_release(atomic64_t *v)
+{
+	return atomic64_fetch_inc_release(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_inc_relaxed(atomic64_t *v)
+{
+	return atomic64_fetch_inc_relaxed(v);
+}
+
+__rust_helper void
+rust_helper_atomic64_dec(atomic64_t *v)
+{
+	atomic64_dec(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_dec_return(atomic64_t *v)
+{
+	return atomic64_dec_return(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_dec_return_acquire(atomic64_t *v)
+{
+	return atomic64_dec_return_acquire(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_dec_return_release(atomic64_t *v)
+{
+	return atomic64_dec_return_release(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_dec_return_relaxed(atomic64_t *v)
+{
+	return atomic64_dec_return_relaxed(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_dec(atomic64_t *v)
+{
+	return atomic64_fetch_dec(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_dec_acquire(atomic64_t *v)
+{
+	return atomic64_fetch_dec_acquire(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_dec_release(atomic64_t *v)
+{
+	return atomic64_fetch_dec_release(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_dec_relaxed(atomic64_t *v)
+{
+	return atomic64_fetch_dec_relaxed(v);
+}
+
+__rust_helper void
+rust_helper_atomic64_and(s64 i, atomic64_t *v)
+{
+	atomic64_and(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_and(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_and(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_and_acquire(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_and_acquire(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_and_release(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_and_release(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_and_relaxed(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_and_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic64_andnot(s64 i, atomic64_t *v)
+{
+	atomic64_andnot(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_andnot(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_andnot(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_andnot_acquire(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_andnot_acquire(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_andnot_release(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_andnot_release(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_andnot_relaxed(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_andnot_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic64_or(s64 i, atomic64_t *v)
+{
+	atomic64_or(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_or(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_or(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_or_acquire(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_or_acquire(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_or_release(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_or_release(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_or_relaxed(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_or_relaxed(i, v);
+}
+
+__rust_helper void
+rust_helper_atomic64_xor(s64 i, atomic64_t *v)
+{
+	atomic64_xor(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_xor(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_xor(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_xor_acquire(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_xor_acquire(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_xor_release(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_xor_release(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_xor_relaxed(s64 i, atomic64_t *v)
+{
+	return atomic64_fetch_xor_relaxed(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_xchg(atomic64_t *v, s64 new)
+{
+	return atomic64_xchg(v, new);
+}
+
+__rust_helper s64
+rust_helper_atomic64_xchg_acquire(atomic64_t *v, s64 new)
+{
+	return atomic64_xchg_acquire(v, new);
+}
+
+__rust_helper s64
+rust_helper_atomic64_xchg_release(atomic64_t *v, s64 new)
+{
+	return atomic64_xchg_release(v, new);
+}
+
+__rust_helper s64
+rust_helper_atomic64_xchg_relaxed(atomic64_t *v, s64 new)
+{
+	return atomic64_xchg_relaxed(v, new);
+}
+
+__rust_helper s64
+rust_helper_atomic64_cmpxchg(atomic64_t *v, s64 old, s64 new)
+{
+	return atomic64_cmpxchg(v, old, new);
+}
+
+__rust_helper s64
+rust_helper_atomic64_cmpxchg_acquire(atomic64_t *v, s64 old, s64 new)
+{
+	return atomic64_cmpxchg_acquire(v, old, new);
+}
+
+__rust_helper s64
+rust_helper_atomic64_cmpxchg_release(atomic64_t *v, s64 old, s64 new)
+{
+	return atomic64_cmpxchg_release(v, old, new);
+}
+
+__rust_helper s64
+rust_helper_atomic64_cmpxchg_relaxed(atomic64_t *v, s64 old, s64 new)
+{
+	return atomic64_cmpxchg_relaxed(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic64_try_cmpxchg(atomic64_t *v, s64 *old, s64 new)
+{
+	return atomic64_try_cmpxchg(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic64_try_cmpxchg_acquire(atomic64_t *v, s64 *old, s64 new)
+{
+	return atomic64_try_cmpxchg_acquire(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic64_try_cmpxchg_release(atomic64_t *v, s64 *old, s64 new)
+{
+	return atomic64_try_cmpxchg_release(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic64_try_cmpxchg_relaxed(atomic64_t *v, s64 *old, s64 new)
+{
+	return atomic64_try_cmpxchg_relaxed(v, old, new);
+}
+
+__rust_helper bool
+rust_helper_atomic64_sub_and_test(s64 i, atomic64_t *v)
+{
+	return atomic64_sub_and_test(i, v);
+}
+
+__rust_helper bool
+rust_helper_atomic64_dec_and_test(atomic64_t *v)
+{
+	return atomic64_dec_and_test(v);
+}
+
+__rust_helper bool
+rust_helper_atomic64_inc_and_test(atomic64_t *v)
+{
+	return atomic64_inc_and_test(v);
+}
+
+__rust_helper bool
+rust_helper_atomic64_add_negative(s64 i, atomic64_t *v)
+{
+	return atomic64_add_negative(i, v);
+}
+
+__rust_helper bool
+rust_helper_atomic64_add_negative_acquire(s64 i, atomic64_t *v)
+{
+	return atomic64_add_negative_acquire(i, v);
+}
+
+__rust_helper bool
+rust_helper_atomic64_add_negative_release(s64 i, atomic64_t *v)
+{
+	return atomic64_add_negative_release(i, v);
+}
+
+__rust_helper bool
+rust_helper_atomic64_add_negative_relaxed(s64 i, atomic64_t *v)
+{
+	return atomic64_add_negative_relaxed(i, v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u)
+{
+	return atomic64_fetch_add_unless(v, a, u);
+}
+
+__rust_helper bool
+rust_helper_atomic64_add_unless(atomic64_t *v, s64 a, s64 u)
+{
+	return atomic64_add_unless(v, a, u);
+}
+
+__rust_helper bool
+rust_helper_atomic64_inc_not_zero(atomic64_t *v)
+{
+	return atomic64_inc_not_zero(v);
+}
+
+__rust_helper bool
+rust_helper_atomic64_inc_unless_negative(atomic64_t *v)
+{
+	return atomic64_inc_unless_negative(v);
+}
+
+__rust_helper bool
+rust_helper_atomic64_dec_unless_positive(atomic64_t *v)
+{
+	return atomic64_dec_unless_positive(v);
+}
+
+__rust_helper s64
+rust_helper_atomic64_dec_if_positive(atomic64_t *v)
+{
+	return atomic64_dec_if_positive(v);
+}
+
+#endif /* _RUST_ATOMIC_API_H */
+// b032d261814b3e119b72dbf7d21447f6731325ee
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 20a0c69d5cc7..ab5a3f1be241 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -7,6 +7,7 @@
  * Sorted alphabetically.
  */
 
+#include "atomic.c"
 #include "blk.c"
 #include "bug.c"
 #include "build_assert.c"
diff --git a/scripts/atomic/gen-atomics.sh b/scripts/atomic/gen-atomics.sh
index 5b98a8307693..02508d0d6fe4 100755
--- a/scripts/atomic/gen-atomics.sh
+++ b/scripts/atomic/gen-atomics.sh
@@ -11,6 +11,7 @@ cat <<EOF |
 gen-atomic-instrumented.sh      linux/atomic/atomic-instrumented.h
 gen-atomic-long.sh              linux/atomic/atomic-long.h
 gen-atomic-fallback.sh          linux/atomic/atomic-arch-fallback.h
+gen-rust-atomic-helpers.sh      ../rust/helpers/atomic.c
 EOF
 while read script header args; do
 	/bin/sh ${ATOMICDIR}/${script} ${ATOMICTBL} ${args} > ${LINUXDIR}/include/${header}
diff --git a/scripts/atomic/gen-rust-atomic-helpers.sh b/scripts/atomic/gen-rust-atomic-helpers.sh
new file mode 100755
index 000000000000..72f2e5bde0c6
--- /dev/null
+++ b/scripts/atomic/gen-rust-atomic-helpers.sh
@@ -0,0 +1,65 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+ATOMICDIR=$(dirname $0)
+
+. ${ATOMICDIR}/atomic-tbl.sh
+
+#gen_proto_order_variant(meta, pfx, name, sfx, order, atomic, int, arg...)
+gen_proto_order_variant()
+{
+	local meta="$1"; shift
+	local pfx="$1"; shift
+	local name="$1"; shift
+	local sfx="$1"; shift
+	local order="$1"; shift
+	local atomic="$1"; shift
+	local int="$1"; shift
+
+	local atomicname="${atomic}_${pfx}${name}${sfx}${order}"
+
+	local ret="$(gen_ret_type "${meta}" "${int}")"
+	local params="$(gen_params "${int}" "${atomic}" "$@")"
+	local args="$(gen_args "$@")"
+	local retstmt="$(gen_ret_stmt "${meta}")"
+
+cat <<EOF
+__rust_helper ${ret}
+rust_helper_${atomicname}(${params})
+{
+	${retstmt}${atomicname}(${args});
+}
+
+EOF
+}
+
+cat << EOF
+// SPDX-License-Identifier: GPL-2.0
+
+// Generated by $0
+// DO NOT MODIFY THIS FILE DIRECTLY
+
+/*
+ * This file provides helpers for the various atomic functions for Rust.
+ */
+#ifndef _RUST_ATOMIC_API_H
+#define _RUST_ATOMIC_API_H
+
+#include <linux/atomic.h>
+
+// TODO: Remove this after LTO helper support is added.
+#define __rust_helper
+
+EOF
+
+grep '^[a-z]' "$1" | while read name meta args; do
+	gen_proto "${meta}" "${name}" "atomic" "int" ${args}
+done
+
+grep '^[a-z]' "$1" | while read name meta args; do
+	gen_proto "${meta}" "${name}" "atomic64" "s64" ${args}
+done
+
+cat <<EOF
+#endif /* _RUST_ATOMIC_API_H */
+EOF
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
  2024-11-01  6:02 ` [RFC v2 01/13] rust: Introduce atomic API helpers Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-12-12 10:51   ` Alice Ryhl
  2024-11-01  6:02 ` [RFC v2 03/13] rust: sync: atomic: Add ordering annotation types Boqun Feng
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

Preparation for generic atomic implementation. To unify the
ipmlementation of a generic method over `i32` and `i64`, the C side
atomic methods need to be grouped so that in a generic method, they can
be referred as <type>::<method>, otherwise their parameters and return
value are different between `i32` and `i64`, which would require using
`transmute()` to unify the type into a `T`.

Introduce `AtomicIpml` to represent a basic type in Rust that has the
direct mapping to an atomic implementation from C. This trait is sealed,
and currently only `i32` and `i64` ipml this.

Further, different methods are put into different `*Ops` trait groups,
and this is for the future when smaller types like `i8`/`i16` are
supported but only with a limited set of API (e.g. only set(), load(),
xchg() and cmpxchg(), no add() or sub() etc).

While the atomic mod is introduced, documentation is also added for
memory models and data races.

Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
my responsiblity on the Rust atomic mod.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 MAINTAINERS                    |   4 +-
 rust/kernel/sync.rs            |   1 +
 rust/kernel/sync/atomic.rs     |  19 ++++
 rust/kernel/sync/atomic/ops.rs | 199 +++++++++++++++++++++++++++++++++
 4 files changed, 222 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/sync/atomic.rs
 create mode 100644 rust/kernel/sync/atomic/ops.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index b77f4495dcf4..e09471027a63 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3635,7 +3635,7 @@ F:	drivers/input/touchscreen/atmel_mxt_ts.c
 ATOMIC INFRASTRUCTURE
 M:	Will Deacon <will@kernel.org>
 M:	Peter Zijlstra <peterz@infradead.org>
-R:	Boqun Feng <boqun.feng@gmail.com>
+M:	Boqun Feng <boqun.feng@gmail.com>
 R:	Mark Rutland <mark.rutland@arm.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
@@ -3644,6 +3644,8 @@ F:	arch/*/include/asm/atomic*.h
 F:	include/*/atomic*.h
 F:	include/linux/refcount.h
 F:	scripts/atomic/
+F:	rust/kernel/sync/atomic.rs
+F:	rust/kernel/sync/atomic/
 
 ATTO EXPRESSSAS SAS/SATA RAID SCSI DRIVER
 M:	Bradley Grove <linuxdrivers@attotech.com>
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 0ab20975a3b5..66ac3752ca71 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -8,6 +8,7 @@
 use crate::types::Opaque;
 
 mod arc;
+pub mod atomic;
 mod condvar;
 pub mod lock;
 mod locked_by;
diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
new file mode 100644
index 000000000000..21b87563667e
--- /dev/null
+++ b/rust/kernel/sync/atomic.rs
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Atomic primitives.
+//!
+//! These primitives have the same semantics as their C counterparts: and the precise definitions of
+//! semantics can be found at [`LKMM`]. Note that Linux Kernel Memory (Consistency) Model is the
+//! only model for Rust code in kernel, and Rust's own atomics should be avoided.
+//!
+//! # Data races
+//!
+//! [`LKMM`] atomics have different rules regarding data races:
+//!
+//! - A normal read doesn't data-race with an atomic read.
+//! - A normal write from C side is treated as an atomic write if
+//!   CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=y.
+//!
+//! [`LKMM`]: srctree/tools/memory-mode/
+
+pub mod ops;
diff --git a/rust/kernel/sync/atomic/ops.rs b/rust/kernel/sync/atomic/ops.rs
new file mode 100644
index 000000000000..59101a0d0273
--- /dev/null
+++ b/rust/kernel/sync/atomic/ops.rs
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Atomic implementations.
+//!
+//! Provides 1:1 mapping of atomic implementations.
+
+use crate::bindings::*;
+use crate::macros::paste;
+
+mod private {
+    /// Sealed trait marker to disable customized impls on atomic implementation traits.
+    pub trait Sealed {}
+}
+
+// `i32` and `i64` are only supported atomic implementations.
+impl private::Sealed for i32 {}
+impl private::Sealed for i64 {}
+
+/// A marker trait for types that ipmlement atomic operations with C side primitives.
+///
+/// This trait is sealed, and only types that have directly mapping to the C side atomics should
+/// impl this:
+///
+/// - `i32` maps to `atomic_t`.
+/// - `i64` maps to `atomic64_t`.
+pub trait AtomicImpl: Sized + Send + Copy + private::Sealed {}
+
+// `atomic_t` impl atomic operations on `i32`.
+impl AtomicImpl for i32 {}
+
+// `atomic64_t` impl atomic operations on `i64`.
+impl AtomicImpl for i64 {}
+
+// This macro generates the function signature with given argument list and return type.
+macro_rules! declare_atomic_method {
+    (
+        $func:ident($($arg:ident : $arg_type:ty),*) $(-> $ret:ty)?
+    ) => {
+        paste!(
+            #[doc = concat!("Atomic ", stringify!($func))]
+            #[doc = "# Safety"]
+            #[doc = "- any pointer passed to the function has to be a valid pointer"]
+            #[doc = "- Accesses must not cause data races per LKMM:"]
+            #[doc = "  - atomic read racing with normal read, normal write or atomic write is not data race."]
+            #[doc = "  - atomic write racing with normal read or normal write is data-race, unless the"]
+            #[doc = "    normal accesses are done at C side and considered as immune to data"]
+            #[doc = "    races, e.g. CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC."]
+            unsafe fn [< atomic_ $func >]($($arg: $arg_type,)*) $(-> $ret)?;
+        );
+    };
+    (
+        $func:ident [$variant:ident $($rest:ident)*]($($arg_sig:tt)*) $(-> $ret:ty)?
+    ) => {
+        paste!(
+            declare_atomic_method!(
+                [< $func _ $variant >]($($arg_sig)*) $(-> $ret)?
+            );
+        );
+
+        declare_atomic_method!(
+            $func [$($rest)*]($($arg_sig)*) $(-> $ret)?
+        );
+    };
+    (
+        $func:ident []($($arg_sig:tt)*) $(-> $ret:ty)?
+    ) => {
+        declare_atomic_method!(
+            $func($($arg_sig)*) $(-> $ret)?
+        );
+    }
+}
+
+// This macro generates the function implementation with given argument list and return type, and it
+// will replace "call(...)" expression with "$ctype _ $func" to call the real C function.
+macro_rules! impl_atomic_method {
+    (
+        ($ctype:ident) $func:ident($($arg:ident: $arg_type:ty),*) $(-> $ret:ty)? {
+            call($($c_arg:expr),*)
+        }
+    ) => {
+        paste!(
+            #[inline(always)]
+            unsafe fn [< atomic_ $func >]($($arg: $arg_type,)*) $(-> $ret)? {
+                // SAFETY: Per function safety requirement, all pointers are valid, and accesses
+                // won't cause data race per LKMM.
+                unsafe { [< $ctype _ $func >]($($c_arg,)*) }
+            }
+        );
+    };
+    (
+        ($ctype:ident) $func:ident[$variant:ident $($rest:ident)*]($($arg_sig:tt)*) $(-> $ret:ty)? {
+            call($($arg:tt)*)
+        }
+    ) => {
+        paste!(
+            impl_atomic_method!(
+                ($ctype) [< $func _ $variant >]($($arg_sig)*) $( -> $ret)? {
+                    call($($arg)*)
+            }
+            );
+        );
+        impl_atomic_method!(
+            ($ctype) $func [$($rest)*]($($arg_sig)*) $( -> $ret)? {
+                call($($arg)*)
+            }
+        );
+    };
+    (
+        ($ctype:ident) $func:ident[]($($arg_sig:tt)*) $( -> $ret:ty)? {
+            call($($arg:tt)*)
+        }
+    ) => {
+        impl_atomic_method!(
+            ($ctype) $func($($arg_sig)*) $(-> $ret)? {
+                call($($arg)*)
+            }
+        );
+    }
+}
+
+// Delcares $ops trait with methods and implements the trait for `i32` and `i64`.
+macro_rules! declare_and_impl_atomic_methods {
+    ($ops:ident ($doc:literal) {
+        $(
+            $func:ident [$($variant:ident),*]($($arg_sig:tt)*) $( -> $ret:ty)? {
+                call($($arg:tt)*)
+            }
+        )*
+    }) => {
+        #[doc = $doc]
+        pub trait $ops: AtomicImpl {
+            $(
+                declare_atomic_method!(
+                    $func[$($variant)*]($($arg_sig)*) $(-> $ret)?
+                );
+            )*
+        }
+
+        impl $ops for i32 {
+            $(
+                impl_atomic_method!(
+                    (atomic) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
+                        call($($arg)*)
+                    }
+                );
+            )*
+        }
+
+        impl $ops for i64 {
+            $(
+                impl_atomic_method!(
+                    (atomic64) $func[$($variant)*]($($arg_sig)*) $(-> $ret)? {
+                        call($($arg)*)
+                    }
+                );
+            )*
+        }
+    }
+}
+
+declare_and_impl_atomic_methods!(
+    AtomicHasBasicOps ("Basic atomic operations") {
+        read[acquire](ptr: *mut Self) -> Self {
+            call(ptr as *mut _)
+        }
+
+        set[release](ptr: *mut Self, v: Self) {
+            call(ptr as *mut _, v)
+        }
+    }
+);
+
+declare_and_impl_atomic_methods!(
+    AtomicHasXchgOps ("Exchange and compare-and-exchange atomic operations") {
+        xchg[acquire, release, relaxed](ptr: *mut Self, v: Self) -> Self {
+            call(ptr as *mut _, v)
+        }
+
+        cmpxchg[acquire, release, relaxed](ptr: *mut Self, old: Self, new: Self) -> Self {
+            call(ptr as *mut _, old, new)
+        }
+
+        try_cmpxchg[acquire, release, relaxed](ptr: *mut Self, old: *mut Self, new: Self) -> bool {
+            call(ptr as *mut _, old, new)
+        }
+    }
+);
+
+declare_and_impl_atomic_methods!(
+    AtomicHasArithmeticOps ("Atomic arithmetic operations") {
+        add[](ptr: *mut Self, v: Self) {
+            call(v, ptr as *mut _)
+        }
+
+        fetch_add[acquire, release, relaxed](ptr: *mut Self, v: Self) -> Self {
+            call(v, ptr as *mut _)
+        }
+    }
+);
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 03/13] rust: sync: atomic: Add ordering annotation types
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
  2024-11-01  6:02 ` [RFC v2 01/13] rust: Introduce atomic API helpers Boqun Feng
  2024-11-01  6:02 ` [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:02 ` [RFC v2 04/13] rust: sync: atomic: Add generic atomics Boqun Feng
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

Preparation for atomic primitives. Instead of a suffix like _acquire, a
method parameter along with the corresponding generic parameter will be
used to specify the ordering of an atomic operations. For example,
atomic load() can be defined as:

	impl<T: ...> Atomic<T> {
	    pub fn load<O: AcquireOrRelaxed>(&self, _o: O) -> T { ... }
	}

and acquire users would do:

	let r = x.load(Acquire);

relaxed users:

	let r = x.load(Relaxed);

doing the following:

	let r = x.load(Release);

will cause a compiler error.

Compared to suffixes, it's easier to tell what ordering variants an
operation has, and it also make it easier to unify the implementation of
all ordering variants in one method via generic. The `IS_RELAXED` and
`ORDER` associate consts are for generic function to pick up the
particular implementation specified by an ordering annotation.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic.rs          |  3 +
 rust/kernel/sync/atomic/ordering.rs | 94 +++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)
 create mode 100644 rust/kernel/sync/atomic/ordering.rs

diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index 21b87563667e..be2e8583595f 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -17,3 +17,6 @@
 //! [`LKMM`]: srctree/tools/memory-mode/
 
 pub mod ops;
+pub mod ordering;
+
+pub use ordering::{Acquire, Full, Relaxed, Release};
diff --git a/rust/kernel/sync/atomic/ordering.rs b/rust/kernel/sync/atomic/ordering.rs
new file mode 100644
index 000000000000..6cf01cd276c6
--- /dev/null
+++ b/rust/kernel/sync/atomic/ordering.rs
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Memory orderings.
+//!
+//! The semantics of these orderings follows the [`LKMM`] definitions and rules.
+//!
+//! - [`Acquire`] and [`Release`] are similar to their counterpart in Rust memory model.
+//! - [`Full`] means "fully-ordered", that is:
+//!   - It provides ordering between all the preceding memory accesses and the annotated operation.
+//!   - It provides ordering between the annotated operation and all the following memory accesses.
+//!   - It provides ordering between all the preceding memory accesses and all the fllowing memory
+//!     accesses.
+//!   - All the orderings are the same strong as a full memory barrier (i.e. `smp_mb()`).
+//! - [`Relaxed`] is similar to the counterpart in Rust memory model, except that dependency
+//!   orderings are also honored in [`LKMM`]. Dependency orderings are described in "DEPENDENCY
+//!   RELATIONS" in [`LKMM`]'s [`explanation`].
+//!
+//! [`LKMM`]: srctree/tools/memory-model/
+//! [`explanation`]: srctree/tools/memory-model/Documentation/explanation.txt
+
+/// The annotation type for relaxed memory ordering.
+pub struct Relaxed;
+
+/// The annotation type for acquire memory ordering.
+pub struct Acquire;
+
+/// The annotation type for release memory ordering.
+pub struct Release;
+
+/// The annotation type for fully-order memory ordering.
+pub struct Full;
+
+/// The trait bound for operations that only support relaxed ordering.
+pub trait RelaxedOnly {}
+
+impl RelaxedOnly for Relaxed {}
+
+/// The trait bound for operations that only support acquire or relaxed ordering.
+pub trait AcquireOrRelaxed {
+    /// Describes whether an ordering is relaxed or not.
+    const IS_RELAXED: bool = false;
+}
+
+impl AcquireOrRelaxed for Acquire {}
+
+impl AcquireOrRelaxed for Relaxed {
+    const IS_RELAXED: bool = true;
+}
+
+/// The trait bound for operations that only support release or relaxed ordering.
+pub trait ReleaseOrRelaxed {
+    /// Describes whether an ordering is relaxed or not.
+    const IS_RELAXED: bool = false;
+}
+
+impl ReleaseOrRelaxed for Release {}
+
+impl ReleaseOrRelaxed for Relaxed {
+    const IS_RELAXED: bool = true;
+}
+
+/// Describes the exact memory ordering of an `impl` [`All`].
+pub enum OrderingDesc {
+    /// Relaxed ordering.
+    Relaxed,
+    /// Acquire ordering.
+    Acquire,
+    /// Release ordering.
+    Release,
+    /// Fully-ordered.
+    Full,
+}
+
+/// The trait bound for annotating operations that should support all orderings.
+pub trait All {
+    /// Describes the exact memory ordering.
+    const ORDER: OrderingDesc;
+}
+
+impl All for Relaxed {
+    const ORDER: OrderingDesc = OrderingDesc::Relaxed;
+}
+
+impl All for Acquire {
+    const ORDER: OrderingDesc = OrderingDesc::Acquire;
+}
+
+impl All for Release {
+    const ORDER: OrderingDesc = OrderingDesc::Release;
+}
+
+impl All for Full {
+    const ORDER: OrderingDesc = OrderingDesc::Full;
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 04/13] rust: sync: atomic: Add generic atomics
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (2 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 03/13] rust: sync: atomic: Add ordering annotation types Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-12-12 10:57   ` Alice Ryhl
  2024-11-01  6:02 ` [RFC v2 05/13] rust: sync: atomic: Add atomic {cmp,}xchg operations Boqun Feng
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

To provide using LKMM atomics for Rust code, a generic `Atomic<T>` is
added, currently `T` needs to be Send + Copy because these are the
straightforward usages and all basic types support this. The trait
`AllowAtomic` should be only ipmlemented inside atomic mod until the
generic atomic framework is mature enough (unless the ipmlementer is a
`#[repr(transparent)]` new type).

`AtomicIpml` types are automatically `AllowAtomic`, and so far only
basic operations load() and store() are introduced.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic.rs         |   2 +
 rust/kernel/sync/atomic/generic.rs | 253 +++++++++++++++++++++++++++++
 2 files changed, 255 insertions(+)
 create mode 100644 rust/kernel/sync/atomic/generic.rs

diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index be2e8583595f..b791abc59b61 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -16,7 +16,9 @@
 //!
 //! [`LKMM`]: srctree/tools/memory-mode/
 
+pub mod generic;
 pub mod ops;
 pub mod ordering;
 
+pub use generic::Atomic;
 pub use ordering::{Acquire, Full, Relaxed, Release};
diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
new file mode 100644
index 000000000000..204da38e2691
--- /dev/null
+++ b/rust/kernel/sync/atomic/generic.rs
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Generic atomic primitives.
+
+use super::ops::*;
+use super::ordering::*;
+use crate::types::Opaque;
+
+/// A generic atomic variable.
+///
+/// `T` must impl [`AllowAtomic`], that is, an [`AtomicImpl`] has to be chosen.
+///
+/// # Invariants
+///
+/// Doing an atomic operation while holding a reference of [`Self`] won't cause a data race, this
+/// is guaranteed by the safety requirement of [`Self::from_ptr`] and the extra safety requirement
+/// of the usage on pointers returned by [`Self::as_ptr`].
+#[repr(transparent)]
+pub struct Atomic<T: AllowAtomic>(Opaque<T>);
+
+// SAFETY: `Atomic<T>` is safe to share among execution contexts because all accesses are atomic.
+unsafe impl<T: AllowAtomic> Sync for Atomic<T> {}
+
+/// Atomics that support basic atomic operations.
+///
+/// TODO: Unless the `impl` is a `#[repr(transparet)]` new type of an existing [`AllowAtomic`], the
+/// impl block should be only done in atomic mod. And currently only basic integer types can
+/// implement this trait in atomic mod.
+///
+/// # Safety
+///
+/// [`Self`] must have the same size and alignment as [`Self::Repr`].
+pub unsafe trait AllowAtomic: Sized + Send + Copy {
+    /// The backing atomic implementation type.
+    type Repr: AtomicImpl;
+
+    /// Converts into a [`Self::Repr`].
+    fn into_repr(self) -> Self::Repr;
+
+    /// Converts from a [`Self::Repr`].
+    fn from_repr(repr: Self::Repr) -> Self;
+}
+
+// SAFETY: `T::Repr` is `Self` (i.e. `T`), so they have the same size and alignment.
+unsafe impl<T: AtomicImpl> AllowAtomic for T {
+    type Repr = Self;
+
+    fn into_repr(self) -> Self::Repr {
+        self
+    }
+
+    fn from_repr(repr: Self::Repr) -> Self {
+        repr
+    }
+}
+
+impl<T: AllowAtomic> Atomic<T> {
+    /// Creates a new atomic.
+    pub const fn new(v: T) -> Self {
+        Self(Opaque::new(v))
+    }
+
+    /// Creates a reference to [`Self`] from a pointer.
+    ///
+    /// # Safety
+    ///
+    /// - `ptr` has to be a valid pointer.
+    /// - `ptr` has to be valid for both reads and writes for the whole lifetime `'a`.
+    /// - For the whole lifetime of '`a`, other accesses to the object cannot cause data races
+    ///   (defined by [`LKMM`]) against atomic operations on the returned reference.
+    ///
+    /// [`LKMM`]: srctree/tools/memory-model
+    ///
+    /// # Examples
+    ///
+    /// Using [`Atomic::from_ptr()`] combined with [`Atomic::load()`] or [`Atomic::store()`] can
+    /// achieve the same functionality as `READ_ONCE()`/`smp_load_acquire()` or
+    /// `WRITE_ONCE()`/`smp_store_release()` in C side:
+    ///
+    /// ```rust
+    /// # use kernel::types::Opaque;
+    /// use kernel::sync::atomic::{Atomic, Relaxed, Release};
+    ///
+    /// // Assume there is a C struct `Foo`.
+    /// mod cbindings {
+    ///     #[repr(C)]
+    ///     pub(crate) struct foo { pub(crate) a: i32, pub(crate) b: i32 }
+    /// }
+    ///
+    /// let tmp = Opaque::new(cbindings::foo { a: 1, b: 2});
+    ///
+    /// // struct foo *foo_ptr = ..;
+    /// let foo_ptr = tmp.get();
+    ///
+    /// // SAFETY: `foo_ptr` is a valid pointer, and `.a` is inbound.
+    /// let foo_a_ptr = unsafe { core::ptr::addr_of_mut!((*foo_ptr).a) };
+    ///
+    /// // a = READ_ONCE(foo_ptr->a);
+    /// //
+    /// // SAFETY: `foo_a_ptr` is a valid pointer for read, and all accesses on it is atomic, so no
+    /// // data race.
+    /// let a = unsafe { Atomic::from_ptr(foo_a_ptr) }.load(Relaxed);
+    /// # assert_eq!(a, 1);
+    ///
+    /// // smp_store_release(&foo_ptr->a, 2);
+    /// //
+    /// // SAFETY: `foo_a_ptr` is a valid pointer for write, and all accesses on it is atomic, so no
+    /// // data race.
+    /// unsafe { Atomic::from_ptr(foo_a_ptr) }.store(2, Release);
+    /// ```
+    ///
+    /// However, this should be only used when communicating with C side or manipulating a C struct.
+    pub unsafe fn from_ptr<'a>(ptr: *mut T) -> &'a Self
+    where
+        T: Sync,
+    {
+        // CAST: `T` is transparent to `Atomic<T>`.
+        // SAFETY: Per function safety requirement, `ptr` is a valid pointer and the object will
+        // live long enough. It's safe to return a `&Atomic<T>` because function safety requirement
+        // guarantees other accesses won't cause data races.
+        unsafe { &*ptr.cast::<Self>() }
+    }
+
+    /// Returns a pointer to the underlying atomic variable.
+    ///
+    /// Extra safety requirement on using the return pointer: the operations done via the pointer
+    /// cannot cause data races defined by [`LKMM`].
+    ///
+    /// [`LKMM`]: srctree/tools/memory-model
+    pub const fn as_ptr(&self) -> *mut T {
+        self.0.get()
+    }
+
+    /// Returns a mutable reference to the underlying atomic variable.
+    ///
+    /// This is safe because the mutable reference of the atomic variable guarantees the exclusive
+    /// access.
+    pub fn get_mut(&mut self) -> &mut T {
+        // SAFETY: `self.as_ptr()` is a valid pointer to `T`, and the object has already been
+        // initialized. `&mut self` guarantees the exclusive access, so it's safe to reborrow
+        // mutably.
+        unsafe { &mut *self.as_ptr() }
+    }
+}
+
+impl<T: AllowAtomic> Atomic<T>
+where
+    T::Repr: AtomicHasBasicOps,
+{
+    /// Loads the value from the atomic variable.
+    ///
+    /// # Examples
+    ///
+    /// Simple usages:
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{Atomic, Relaxed};
+    ///
+    /// let x = Atomic::new(42i32);
+    ///
+    /// assert_eq!(42, x.load(Relaxed));
+    ///
+    /// let x = Atomic::new(42i64);
+    ///
+    /// assert_eq!(42, x.load(Relaxed));
+    /// ```
+    ///
+    /// Customized new types in [`Atomic`]:
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{generic::AllowAtomic, Atomic, Relaxed};
+    ///
+    /// #[derive(Clone, Copy)]
+    /// #[repr(transparent)]
+    /// struct NewType(u32);
+    ///
+    /// // SAFETY: `NewType` is transparent to `u32`, which has the same size and alignment as
+    /// // `i32`.
+    /// unsafe impl AllowAtomic for NewType {
+    ///     type Repr = i32;
+    ///
+    ///     fn into_repr(self) -> Self::Repr {
+    ///         self.0 as i32
+    ///     }
+    ///
+    ///     fn from_repr(repr: Self::Repr) -> Self {
+    ///         NewType(repr as u32)
+    ///     }
+    /// }
+    ///
+    /// let n = Atomic::new(NewType(0));
+    ///
+    /// assert_eq!(0, n.load(Relaxed).0);
+    /// ```
+    #[inline(always)]
+    pub fn load<Ordering: AcquireOrRelaxed>(&self, _: Ordering) -> T {
+        let a = self.as_ptr().cast::<T::Repr>();
+
+        // SAFETY:
+        // - For calling the atomic_read*() function:
+        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
+        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
+        //   - per the type invariants, the following atomic operation won't cause data races.
+        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
+        //   - atomic operations are used here.
+        let v = unsafe {
+            if Ordering::IS_RELAXED {
+                T::Repr::atomic_read(a)
+            } else {
+                T::Repr::atomic_read_acquire(a)
+            }
+        };
+
+        T::from_repr(v)
+    }
+
+    /// Stores a value to the atomic variable.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{Atomic, Relaxed};
+    ///
+    /// let x = Atomic::new(42i32);
+    ///
+    /// assert_eq!(42, x.load(Relaxed));
+    ///
+    /// x.store(43, Relaxed);
+    ///
+    /// assert_eq!(43, x.load(Relaxed));
+    /// ```
+    ///
+    #[inline(always)]
+    pub fn store<Ordering: ReleaseOrRelaxed>(&self, v: T, _: Ordering) {
+        let v = T::into_repr(v);
+        let a = self.as_ptr().cast::<T::Repr>();
+
+        // SAFETY:
+        // - For calling the atomic_set*() function:
+        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
+        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
+        //   - per the type invariants, the following atomic operation won't cause data races.
+        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
+        //   - atomic operations are used here.
+        unsafe {
+            if Ordering::IS_RELAXED {
+                T::Repr::atomic_set(a, v)
+            } else {
+                T::Repr::atomic_set_release(a, v)
+            }
+        };
+    }
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 05/13] rust: sync: atomic: Add atomic {cmp,}xchg operations
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (3 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 04/13] rust: sync: atomic: Add generic atomics Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:02 ` [RFC v2 06/13] rust: sync: atomic: Add the framework of arithmetic operations Boqun Feng
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

xchg() and cmpxchg() are basic operations on atomic. Provide these based
on C APIs.

Note that cmpxchg() use the similar function signature as
compare_exchange() in Rust std: returning a `Result`, `Ok(old)` means
the operation succeeds and `Err(old)` means the operation fails. With
the compiler optimization and inline helpers, it should provides the
same efficient code generation as using atomic_try_cmpxchg() or
atomic_cmpxchg() correctly.

Except it's not! Because of commit 44fe84459faf ("locking/atomic: Fix
atomic_try_cmpxchg() semantics"), the atomic_try_cmpxchg() on x86 has a
branch even if the caller doesn't care about the success of cmpxchg and
only wants to use the old value. For example, for code like:

    // Uses the latest value regardlessly, same as atomic_cmpxchg() in C.
    let latest = x.cmpxchg(42, 64, Full).unwrap_or_else(|old| old);

It will still generate code:

    movl    $0x40, %ecx
    movl    $0x34, %eax
    lock
    cmpxchgl        %ecx, 0x4(%rsp)
    jne     1f
2:
...
1:  movl    %eax, %ecx
    jmp 2b

Attempting to write an x86 try_cmpxchg_exclusive() for Rust use only,
because the Rust function takes a `&mut` for old pointer, which must be
exclusive to the function, therefore it's unsafe to use some shared
pointer. But maybe I'm missing something?

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic/generic.rs | 151 +++++++++++++++++++++++++++++
 1 file changed, 151 insertions(+)

diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
index 204da38e2691..bfccc4336c75 100644
--- a/rust/kernel/sync/atomic/generic.rs
+++ b/rust/kernel/sync/atomic/generic.rs
@@ -251,3 +251,154 @@ pub fn store<Ordering: ReleaseOrRelaxed>(&self, v: T, _: Ordering) {
         };
     }
 }
+
+impl<T: AllowAtomic> Atomic<T>
+where
+    T::Repr: AtomicHasXchgOps,
+{
+    /// Atomic exchange.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{Atomic, Acquire, Relaxed};
+    ///
+    /// let x = Atomic::new(42);
+    ///
+    /// assert_eq!(42, x.xchg(52, Acquire));
+    /// assert_eq!(52, x.load(Relaxed));
+    /// ```
+    #[inline(always)]
+    pub fn xchg<Ordering: All>(&self, v: T, _: Ordering) -> T {
+        let v = T::into_repr(v);
+        let a = self.as_ptr().cast::<T::Repr>();
+
+        // SAFETY:
+        // - For calling the atomic_xchg*() function:
+        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
+        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
+        //   - per the type invariants, the following atomic operation won't cause data races.
+        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
+        //   - atomic operations are used here.
+        let ret = unsafe {
+            match Ordering::ORDER {
+                OrderingDesc::Full => T::Repr::atomic_xchg(a, v),
+                OrderingDesc::Acquire => T::Repr::atomic_xchg_acquire(a, v),
+                OrderingDesc::Release => T::Repr::atomic_xchg_release(a, v),
+                OrderingDesc::Relaxed => T::Repr::atomic_xchg_relaxed(a, v),
+            }
+        };
+
+        T::from_repr(ret)
+    }
+
+    /// Atomic compare and exchange.
+    ///
+    /// Compare: The comparison is done via the byte level comparison between the atomic variables
+    /// with the `old` value.
+    ///
+    /// Ordering: A failed compare and exchange does provide anything, the read part of a failed
+    /// cmpxchg should be treated as a relaxed read.
+    ///
+    /// Returns `Ok(value)` if cmpxchg succeeds, and `value` is guaranteed to be equal to `old`,
+    /// otherwise returns `Err(value)`, and `value` is the value of the atomic variable when cmpxchg
+    /// was happening.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{Atomic, Full, Relaxed};
+    ///
+    /// let x = Atomic::new(42);
+    ///
+    /// // Checks whether cmpxchg succeeded.
+    /// let success = x.cmpxchg(52, 64, Relaxed).is_ok();
+    /// # assert!(!success);
+    ///
+    /// // Checks whether cmpxchg failed.
+    /// let failure = x.cmpxchg(52, 64, Relaxed).is_err();
+    /// # assert!(failure);
+    ///
+    /// // Uses the old value if failed, probably re-try cmpxchg.
+    /// match x.cmpxchg(52, 64, Relaxed) {
+    ///     Ok(_) => { },
+    ///     Err(old) => {
+    ///         // do something with `old`.
+    ///         # assert_eq!(old, 42);
+    ///     }
+    /// }
+    ///
+    /// // Uses the latest value regardlessly, same as atomic_cmpxchg() in C.
+    /// let latest = x.cmpxchg(42, 64, Full).unwrap_or_else(|old| old);
+    /// # assert_eq!(42, latest);
+    /// assert_eq!(64, x.load(Relaxed));
+    /// ```
+    #[inline(always)]
+    pub fn cmpxchg<Ordering: All>(&self, mut old: T, new: T, o: Ordering) -> Result<T, T> {
+        if self.try_cmpxchg(&mut old, new, o) {
+            Ok(old)
+        } else {
+            Err(old)
+        }
+    }
+
+    /// Atomic compare and exchange and returns whether the operation succeeds.
+    ///
+    /// "Compare" and "Ordering" part are the same as [`Atomic::cmpxchg`].
+    ///
+    /// Returns `true` means the cmpxchg succeeds otherwise returns `false` with `old` updated to
+    /// the value of the atomic variable when cmpxchg was happening.
+    #[inline(always)]
+    fn try_cmpxchg<Ordering: All>(&self, old: &mut T, new: T, _: Ordering) -> bool {
+        let old = (old as *mut T).cast::<T::Repr>();
+        let new = T::into_repr(new);
+        let a = self.0.get().cast::<T::Repr>();
+
+        // SAFETY:
+        // - For calling the atomic_try_cmpchg*() function:
+        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
+        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
+        //   - per the type invariants, the following atomic operation won't cause data races.
+        //   - `old` is a valid pointer to write because it comes from a mutable reference.
+        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
+        //   - atomic operations are used here.
+        unsafe {
+            match Ordering::ORDER {
+                OrderingDesc::Full => T::Repr::atomic_try_cmpxchg(a, old, new),
+                OrderingDesc::Acquire => T::Repr::atomic_try_cmpxchg_acquire(a, old, new),
+                OrderingDesc::Release => T::Repr::atomic_try_cmpxchg_release(a, old, new),
+                OrderingDesc::Relaxed => T::Repr::atomic_try_cmpxchg_relaxed(a, old, new),
+            }
+        }
+    }
+
+    /// Atomic compare and exchange and return the [`Result`].
+    ///
+    /// "Compare" and "Ordering" part are the same as [`Atomic::cmpxchg`].
+    ///
+    /// Returns `Ok(value)` if cmpxchg succeeds, and `value` is guaranteed to be equal to `old`,
+    /// otherwise returns `Err(value)`, and `value` is the value of the atomic variable when cmpxchg
+    /// was happening.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{Atomic, Acquire, Relaxed};
+    ///
+    /// let x = Atomic::new(42i32);
+    ///
+    /// assert!(x.compare_exchange(52, 64, Acquire).is_err());
+    /// assert_eq!(42, x.load(Relaxed));
+    ///
+    /// assert!(x.compare_exchange(42, 64, Acquire).is_ok());
+    /// assert_eq!(64, x.load(Relaxed));
+    /// ```
+    #[inline(always)]
+    pub fn compare_exchange<Ordering: All>(&self, mut old: T, new: T, o: Ordering) -> Result<T, T> {
+        if self.try_cmpxchg(&mut old, new, o) {
+            Ok(old)
+        } else {
+            Err(old)
+        }
+    }
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 06/13] rust: sync: atomic: Add the framework of arithmetic operations
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (4 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 05/13] rust: sync: atomic: Add atomic {cmp,}xchg operations Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:02 ` [RFC v2 07/13] rust: sync: atomic: Add Atomic<u{32,64}> Boqun Feng
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

One important set of atomic operations is the arithmetic operations,
i.e. add(), sub(), fetch_add(), add_return(), etc. However it may not
make senses for all the types that `AllowAtomic` to have arithmetic
operations, for example a `Foo(u32)` may not have a reasonable add() or
sub(), plus subword types (`u8` and `u16`) currently don't have
atomic arithmetic operations even on C side and might not have them in
the future in Rust (because they are usually suboptimal on a few
architecures). Therefore add a subtrait of `AllowAtomic` describing
which types have and can do atomic arithemtic operations.

A few things about this `AllowAtomicArithmetic` trait:

* It has an associate type `Delta` instead of using
  `AllowAllowAtomic::Repr` because, a `Bar(u32)` (whose `Repr` is `i32`)
  may not wants an `add(&self, i32)`, but an `add(&self, u32)`.

* `AtomicImpl` types already implement an `AtomicHasArithmeticOps`
  trait, so add blanket implementation for them. In the future, `i8` and
  `i16` may impl `AtomicImpl` but not `AtomicHasArithmeticOps` if
  arithemtic operations are not available.

Only add() and fetch_add() are added. The rest will be added in the
future.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic/generic.rs | 102 +++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
index bfccc4336c75..a75c3e9f4c89 100644
--- a/rust/kernel/sync/atomic/generic.rs
+++ b/rust/kernel/sync/atomic/generic.rs
@@ -3,6 +3,7 @@
 //! Generic atomic primitives.
 
 use super::ops::*;
+use super::ordering;
 use super::ordering::*;
 use crate::types::Opaque;
 
@@ -54,6 +55,23 @@ fn from_repr(repr: Self::Repr) -> Self {
     }
 }
 
+/// Atomics that allows arithmetic operations with an integer type.
+pub trait AllowAtomicArithmetic: AllowAtomic {
+    /// The delta types for arithmetic operations.
+    type Delta;
+
+    /// Converts [`Self::Delta`] into the representation of the atomic type.
+    fn delta_into_repr(d: Self::Delta) -> Self::Repr;
+}
+
+impl<T: AtomicImpl + AtomicHasArithmeticOps> AllowAtomicArithmetic for T {
+    type Delta = Self;
+
+    fn delta_into_repr(d: Self::Delta) -> Self::Repr {
+        d
+    }
+}
+
 impl<T: AllowAtomic> Atomic<T> {
     /// Creates a new atomic.
     pub const fn new(v: T) -> Self {
@@ -402,3 +420,87 @@ pub fn compare_exchange<Ordering: All>(&self, mut old: T, new: T, o: Ordering) -
         }
     }
 }
+
+impl<T: AllowAtomicArithmetic> Atomic<T>
+where
+    T::Repr: AtomicHasArithmeticOps,
+{
+    /// Atomic add.
+    ///
+    /// The addition is a wrapping addition.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{Atomic, Relaxed};
+    ///
+    /// let x = Atomic::new(42);
+    ///
+    /// assert_eq!(42, x.load(Relaxed));
+    ///
+    /// x.add(12, Relaxed);
+    ///
+    /// assert_eq!(54, x.load(Relaxed));
+    /// ```
+    #[inline(always)]
+    pub fn add<Ordering: RelaxedOnly>(&self, v: T::Delta, _: Ordering) {
+        let v = T::delta_into_repr(v);
+        let a = self.as_ptr().cast::<T::Repr>();
+
+        // SAFETY:
+        // - For calling the atomic_add() function:
+        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
+        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
+        //   - per the type invariants, the following atomic operation won't cause data races.
+        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
+        //   - atomic operations are used here.
+        unsafe {
+            T::Repr::atomic_add(a, v);
+        }
+    }
+
+    /// Atomic fetch and add.
+    ///
+    /// The addition is a wrapping addition.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use kernel::sync::atomic::{Atomic, Acquire, Full, Relaxed};
+    ///
+    /// let x = Atomic::new(42);
+    ///
+    /// assert_eq!(42, x.load(Relaxed));
+    ///
+    /// assert_eq!(54, { x.fetch_add(12, Acquire); x.load(Relaxed) });
+    ///
+    /// let x = Atomic::new(42);
+    ///
+    /// assert_eq!(42, x.load(Relaxed));
+    ///
+    /// assert_eq!(54, { x.fetch_add(12, Full); x.load(Relaxed) } );
+    /// ```
+    #[inline(always)]
+    pub fn fetch_add<Ordering: All>(&self, v: T::Delta, _: Ordering) -> T {
+        let v = T::delta_into_repr(v);
+        let a = self.as_ptr().cast::<T::Repr>();
+
+        // SAFETY:
+        // - For calling the atomic_fetch_add*() function:
+        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
+        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
+        //   - per the type invariants, the following atomic operation won't cause data races.
+        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
+        //   - atomic operations are used here.
+        let ret = unsafe {
+            match Ordering::ORDER {
+                ordering::OrderingDesc::Full => T::Repr::atomic_fetch_add(a, v),
+                ordering::OrderingDesc::Acquire => T::Repr::atomic_fetch_add_acquire(a, v),
+                ordering::OrderingDesc::Release => T::Repr::atomic_fetch_add_release(a, v),
+                ordering::OrderingDesc::Relaxed => T::Repr::atomic_fetch_add_relaxed(a, v),
+            }
+        };
+
+        T::from_repr(ret)
+    }
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 07/13] rust: sync: atomic: Add Atomic<u{32,64}>
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (5 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 06/13] rust: sync: atomic: Add the framework of arithmetic operations Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:02 ` [RFC v2 08/13] rust: sync: atomic: Add Atomic<{usize,isize}> Boqun Feng
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

Add generic atomic support for basic unsigned types that have an
`AtomicIpml` with the same size and alignment.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic.rs | 80 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index b791abc59b61..b2e81e22c105 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -22,3 +22,83 @@
 
 pub use generic::Atomic;
 pub use ordering::{Acquire, Full, Relaxed, Release};
+
+/// ```rust
+/// use kernel::sync::atomic::{Atomic, Relaxed};
+///
+/// let x = Atomic::new(42u64);
+///
+/// assert_eq!(42, x.load(Relaxed));
+/// ```
+// SAFETY: `u64` and `i64` has the same size and alignment.
+unsafe impl generic::AllowAtomic for u64 {
+    type Repr = i64;
+
+    fn into_repr(self) -> Self::Repr {
+        self as _
+    }
+
+    fn from_repr(repr: Self::Repr) -> Self {
+        repr as _
+    }
+}
+
+/// ```rust
+/// use kernel::sync::atomic::{Atomic, Full, Relaxed};
+///
+/// let x = Atomic::new(42u64);
+///
+/// assert_eq!(42, x.fetch_add(12, Full));
+/// assert_eq!(54, x.load(Relaxed));
+///
+/// x.add(13, Relaxed);
+///
+/// assert_eq!(67, x.load(Relaxed));
+/// ```
+impl generic::AllowAtomicArithmetic for u64 {
+    type Delta = u64;
+
+    fn delta_into_repr(d: Self::Delta) -> Self::Repr {
+        d as _
+    }
+}
+
+/// ```rust
+/// use kernel::sync::atomic::{Atomic, Relaxed};
+///
+/// let x = Atomic::new(42u32);
+///
+/// assert_eq!(42, x.load(Relaxed));
+/// ```
+// SAFETY: `u32` and `i32` has the same size and alignment.
+unsafe impl generic::AllowAtomic for u32 {
+    type Repr = i32;
+
+    fn into_repr(self) -> Self::Repr {
+        self as _
+    }
+
+    fn from_repr(repr: Self::Repr) -> Self {
+        repr as _
+    }
+}
+
+/// ```rust
+/// use kernel::sync::atomic::{Atomic, Full, Relaxed};
+///
+/// let x = Atomic::new(42u32);
+///
+/// assert_eq!(42, x.fetch_add(12, Full));
+/// assert_eq!(54, x.load(Relaxed));
+///
+/// x.add(13, Relaxed);
+///
+/// assert_eq!(67, x.load(Relaxed));
+/// ```
+impl generic::AllowAtomicArithmetic for u32 {
+    type Delta = u32;
+
+    fn delta_into_repr(d: Self::Delta) -> Self::Repr {
+        d as _
+    }
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 08/13] rust: sync: atomic: Add Atomic<{usize,isize}>
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (6 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 07/13] rust: sync: atomic: Add Atomic<u{32,64}> Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:02 ` [RFC v2 09/13] rust: sync: atomic: Add Atomic<*mut T> Boqun Feng
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

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 necessarity of creating `atomic_long_*` helpers, which could save
the binary size of kernel if inline helpers are not available.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic.rs | 71 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index b2e81e22c105..4166ad48604f 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -102,3 +102,74 @@ fn delta_into_repr(d: Self::Delta) -> Self::Repr {
         d as _
     }
 }
+
+// SAFETY: `usize` has the same size and the alignment as `i64` for 64bit and the same as `i32` for
+// 32bit.
+unsafe impl generic::AllowAtomic for usize {
+    #[cfg(CONFIG_64BIT)]
+    type Repr = i64;
+    #[cfg(not(CONFIG_64BIT))]
+    type Repr = i32;
+
+    fn into_repr(self) -> Self::Repr {
+        self as _
+    }
+
+    fn from_repr(repr: Self::Repr) -> Self {
+        repr as _
+    }
+}
+
+/// ```rust
+/// use kernel::sync::atomic::{Atomic, Full, Relaxed};
+///
+/// let x = Atomic::new(42usize);
+///
+/// assert_eq!(42, x.fetch_add(12, Full));
+/// assert_eq!(54, x.load(Relaxed));
+///
+/// x.add(13, Relaxed);
+///
+/// assert_eq!(67, x.load(Relaxed));
+/// ```
+impl generic::AllowAtomicArithmetic for usize {
+    type Delta = usize;
+
+    fn delta_into_repr(d: Self::Delta) -> Self::Repr {
+        d as _
+    }
+}
+
+// SAFETY: `isize` has the same size and the alignment as `i64` for 64bit and the same as `i32` for
+// 32bit.
+unsafe impl generic::AllowAtomic for isize {
+    type Repr = i64;
+
+    fn into_repr(self) -> Self::Repr {
+        self as _
+    }
+
+    fn from_repr(repr: Self::Repr) -> Self {
+        repr as _
+    }
+}
+
+/// ```rust
+/// use kernel::sync::atomic::{Atomic, Full, Relaxed};
+///
+/// let x = Atomic::new(42isize);
+///
+/// assert_eq!(42, x.fetch_add(12, Full));
+/// assert_eq!(54, x.load(Relaxed));
+///
+/// x.add(13, Relaxed);
+///
+/// assert_eq!(67, x.load(Relaxed));
+/// ```
+impl generic::AllowAtomicArithmetic for isize {
+    type Delta = isize;
+
+    fn delta_into_repr(d: Self::Delta) -> Self::Repr {
+        d as _
+    }
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 09/13] rust: sync: atomic: Add Atomic<*mut T>
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (7 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 08/13] rust: sync: atomic: Add Atomic<{usize,isize}> Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:02 ` [RFC v2 10/13] rust: sync: atomic: Add arithmetic ops for " Boqun Feng
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

Add atomic support for raw pointer values, similar to `isize` and
`usize`, the representation type is selected based on CONFIG_64BIT.

`*mut T` is not `Send`, however `Atomic<*mut T>` definitely needs to be
a `Sync`, and that's the whole point of atomics: being able to have
multiple shared references in different threads so that they can sync
with each other. As a result, a pointer value will be transferred from
one thread to another via `Atomic<*mut T>`:

	<thread 1>		<thread 2>
	x.store(p1, Relaxed);
				let p = x.load(p1, Relaxed);

This means a raw pointer value (`*mut T`) needs to be able to transfer
across thread boundaries, which is essentially `Send`.

To reflect this in the type system, and based on the fact that pointer
values can be transferred safely (only using them to dereference is
unsafe), as suggested by Alice, extend the `AllowAtomic` trait to
include a customized `Send` semantics, that is: `impl AllowAtomic` has
to be safe to be transferred across thread boundaries.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic.rs         | 24 ++++++++++++++++++++++++
 rust/kernel/sync/atomic/generic.rs | 16 +++++++++++++---
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index 4166ad48604f..e62c3cd1d3ca 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -173,3 +173,27 @@ fn delta_into_repr(d: Self::Delta) -> Self::Repr {
         d as _
     }
 }
+
+/// ```rust
+/// use kernel::sync::atomic::{Atomic, Relaxed};
+///
+/// let x = Atomic::new(core::ptr::null_mut::<i32>());
+///
+/// assert!(x.load(Relaxed).is_null());
+/// ```
+// SAFETY: A `*mut T` has the same size and the alignment as `i64` for 64bit and the same as `i32`
+// for 32bit. And it's safe to transfer the ownership of a pointer value to another thread.
+unsafe impl<T> generic::AllowAtomic for *mut T {
+    #[cfg(CONFIG_64BIT)]
+    type Repr = i64;
+    #[cfg(not(CONFIG_64BIT))]
+    type Repr = i32;
+
+    fn into_repr(self) -> Self::Repr {
+        self as _
+    }
+
+    fn from_repr(repr: Self::Repr) -> Self {
+        repr as _
+    }
+}
diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
index a75c3e9f4c89..cff98469ed35 100644
--- a/rust/kernel/sync/atomic/generic.rs
+++ b/rust/kernel/sync/atomic/generic.rs
@@ -19,6 +19,10 @@
 #[repr(transparent)]
 pub struct Atomic<T: AllowAtomic>(Opaque<T>);
 
+// SAFETY: `Atomic<T>` is safe to send between execution contexts, because `T` is `AllowAtomic` and
+// `AllowAtomic`'s safety requirement guarantees that.
+unsafe impl<T: AllowAtomic> Send for Atomic<T> {}
+
 // SAFETY: `Atomic<T>` is safe to share among execution contexts because all accesses are atomic.
 unsafe impl<T: AllowAtomic> Sync for Atomic<T> {}
 
@@ -30,8 +34,13 @@ unsafe impl<T: AllowAtomic> Sync for Atomic<T> {}
 ///
 /// # Safety
 ///
-/// [`Self`] must have the same size and alignment as [`Self::Repr`].
-pub unsafe trait AllowAtomic: Sized + Send + Copy {
+/// - [`Self`] must have the same size and alignment as [`Self::Repr`].
+/// - The implementer must guarantee it's safe to transfer ownership from one execution context to
+///   another, this means it has to be a [`Send`], but because `*mut T` is not [`Send`] and that's
+///   the basic type needs to support atomic operations, so this safety requirement is added to
+///   [`AllowAtomic`] trait. This safety requirement is automatically satisfied if the type is a
+///   [`Send`].
+pub unsafe trait AllowAtomic: Sized + Copy {
     /// The backing atomic implementation type.
     type Repr: AtomicImpl;
 
@@ -42,7 +51,8 @@ pub unsafe trait AllowAtomic: Sized + Send + Copy {
     fn from_repr(repr: Self::Repr) -> Self;
 }
 
-// SAFETY: `T::Repr` is `Self` (i.e. `T`), so they have the same size and alignment.
+// SAFETY: `T::Repr` is `Self` (i.e. `T`), so they have the same size and alignment. And all
+// `AtomicImpl` types are `Send`.
 unsafe impl<T: AtomicImpl> AllowAtomic for T {
     type Repr = Self;
 
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 10/13] rust: sync: atomic: Add arithmetic ops for Atomic<*mut T>
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (8 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 09/13] rust: sync: atomic: Add Atomic<*mut T> Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:02 ` [RFC v2 11/13] rust: sync: Add memory barriers Boqun Feng
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

(This is more an RFC)

Add arithmetic operations support for `Atomic<*mut T>`. Currently the
semantics of arithmetic atomic operation is the same as pointer
arithmetic, that is, e.g. `Atomic<*mut u64>::add(1)` is adding 8
(`size_of::<u64>`) to the pointer value.

In Rust std library, there are two sets of pointer arithmetic for
`AtomicPtr`:

* ptr_add() and ptr_sub(), which is the same as Atomic<*mut T>::add(),
  pointer arithmetic.

* byte_add() and byte_sub(), which use the input as byte offset to
  change the pointer value, e.g. byte_add(1) means adding 1 to the
  pointer value.

We can either take the approach in the current patch and add byte_add()
later on if needed, or start with ptr_add() and byte_add() naming.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic.rs | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index e62c3cd1d3ca..cbe5d40d9e36 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -197,3 +197,32 @@ fn from_repr(repr: Self::Repr) -> Self {
         repr as _
     }
 }
+
+/// ```rust
+/// use kernel::sync::atomic::{Atomic, Relaxed};
+///
+/// let s: &mut [i32] = &mut [1, 3, 2, 4];
+///
+/// let x = Atomic::new(s.as_mut_ptr());
+///
+/// x.add(1, Relaxed);
+///
+/// let ptr = x.fetch_add(1, Relaxed); // points to the 2nd element.
+/// let ptr2 = x.load(Relaxed); // points to the 3rd element.
+///
+/// // SAFETY: `ptr` and `ptr2` are valid pointers to the 2nd and 3rd elements of `s` with writing
+/// // provenance, and no other thread is accessing these elements.
+/// unsafe { core::ptr::swap(ptr, ptr2); }
+///
+/// assert_eq!(s, &mut [1, 2, 3, 4]);
+/// ```
+impl<T> generic::AllowAtomicArithmetic for *mut T {
+    type Delta = isize;
+
+    /// The behavior of arithmetic operations
+    fn delta_into_repr(d: Self::Delta) -> Self::Repr {
+        // Since atomic arithmetic operations are wrapping, so a wrapping_mul() here suffices even
+        // if overflow may happen.
+        d.wrapping_mul(core::mem::size_of::<T>() as _) as _
+    }
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 11/13] rust: sync: Add memory barriers
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (9 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 10/13] rust: sync: atomic: Add arithmetic ops for " Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:55   ` David Gow
  2024-11-01  7:01   ` [RFC v2.1 " Boqun Feng
  2024-11-01  6:02 ` [RFC v2 12/13] rust: add rcu abstraction Boqun Feng
                   ` (3 subsequent siblings)
  14 siblings, 2 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

Memory barriers are building blocks for concurrent code, hence provide
a minimal set of them.

The compiler barrier, barrier(), is implemented in inline asm instead of
using core::sync::atomic::compiler_fence() because memory models are
different: kernel's atomics are implemented in inline asm therefore the
compiler barrier should be implemented in inline asm as well.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/helpers/helpers.c      |  1 +
 rust/kernel/sync.rs         |  1 +
 rust/kernel/sync/barrier.rs | 67 +++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+)
 create mode 100644 rust/kernel/sync/barrier.rs

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index ab5a3f1be241..f4a94833b29d 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -8,6 +8,7 @@
  */
 
 #include "atomic.c"
+#include "barrier.c"
 #include "blk.c"
 #include "bug.c"
 #include "build_assert.c"
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 66ac3752ca71..0d0b19441ae8 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -9,6 +9,7 @@
 
 mod arc;
 pub mod atomic;
+pub mod barrier;
 mod condvar;
 pub mod lock;
 mod locked_by;
diff --git a/rust/kernel/sync/barrier.rs b/rust/kernel/sync/barrier.rs
new file mode 100644
index 000000000000..277aa09747bf
--- /dev/null
+++ b/rust/kernel/sync/barrier.rs
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Memory barriers.
+//!
+//! These primitives have the same semantics as their C counterparts: and the precise definitions of
+//! semantics can be found at [`LKMM`].
+//!
+//! [`LKMM`]: srctree/tools/memory-mode/
+
+/// A compiler barrier.
+///
+/// An explicic compiler barrier function that prevents the compiler from moving the memory
+/// accesses either side of it to the other side.
+pub fn barrier() {
+    // By default, Rust inline asms are treated as being able to access any memory or flags, hence
+    // it suffices as a compiler barrier.
+    //
+    // SAFETY: An empty asm block should be safe.
+    unsafe {
+        core::arch::asm!("");
+    }
+}
+
+/// A full memory barrier.
+///
+/// A barrier function that prevents both the compiler and the CPU from moving the memory accesses
+/// either side of it to the other side.
+pub fn smp_mb() {
+    if cfg!(CONFIG_SMP) {
+        // SAFETY: `smp_mb()` is safe to call.
+        unsafe {
+            bindings::smp_mb();
+        }
+    } else {
+        barrier();
+    }
+}
+
+/// A write-write memory barrier.
+///
+/// A barrier function that prevents both the compiler and the CPU from moving the memory write
+/// accesses either side of it to the other side.
+pub fn smp_wmb() {
+    if cfg!(CONFIG_SMP) {
+        // SAFETY: `smp_wmb()` is safe to call.
+        unsafe {
+            bindings::smp_wmb();
+        }
+    } else {
+        barrier();
+    }
+}
+
+/// A read-read memory barrier.
+///
+/// A barrier function that prevents both the compiler and the CPU from moving the memory read
+/// accesses either side of it to the other side.
+pub fn smp_rmb() {
+    if cfg!(CONFIG_SMP) {
+        // SAFETY: `smp_rmb()` is safe to call.
+        unsafe {
+            bindings::smp_rmb();
+        }
+    } else {
+        barrier();
+    }
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 12/13] rust: add rcu abstraction
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (10 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 11/13] rust: sync: Add memory barriers Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01  6:02 ` [RFC v2 13/13] rust: sync: rcu: Add RCU protected pointer Boqun Feng
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv, Danilo Krummrich

From: Wedson Almeida Filho <wedsonaf@gmail.com>

Add a simple abstraction to guard critical code sections with an rcu
read lock.

Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/helpers/helpers.c  |  1 +
 rust/helpers/rcu.c      | 13 +++++++++++
 rust/kernel/sync.rs     |  1 +
 rust/kernel/sync/rcu.rs | 52 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+)
 create mode 100644 rust/helpers/rcu.c
 create mode 100644 rust/kernel/sync/rcu.rs

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index f4a94833b29d..65951245879f 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -18,6 +18,7 @@
 #include "mutex.c"
 #include "page.c"
 #include "rbtree.c"
+#include "rcu.c"
 #include "refcount.c"
 #include "signal.c"
 #include "slab.c"
diff --git a/rust/helpers/rcu.c b/rust/helpers/rcu.c
new file mode 100644
index 000000000000..f1cec6583513
--- /dev/null
+++ b/rust/helpers/rcu.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/rcupdate.h>
+
+void rust_helper_rcu_read_lock(void)
+{
+	rcu_read_lock();
+}
+
+void rust_helper_rcu_read_unlock(void)
+{
+	rcu_read_unlock();
+}
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 0d0b19441ae8..f5a413e1ce30 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -13,6 +13,7 @@
 mod condvar;
 pub mod lock;
 mod locked_by;
+pub mod rcu;
 
 pub use arc::{Arc, ArcBorrow, UniqueArc};
 pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult};
diff --git a/rust/kernel/sync/rcu.rs b/rust/kernel/sync/rcu.rs
new file mode 100644
index 000000000000..5a35495f69a4
--- /dev/null
+++ b/rust/kernel/sync/rcu.rs
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! RCU support.
+//!
+//! C header: [`include/linux/rcupdate.h`](srctree/include/linux/rcupdate.h)
+
+use crate::bindings;
+use core::marker::PhantomData;
+
+/// Evidence that the RCU read side lock is held on the current thread/CPU.
+///
+/// The type is explicitly not `Send` because this property is per-thread/CPU.
+///
+/// # Invariants
+///
+/// The RCU read side lock is actually held while instances of this guard exist.
+pub struct Guard {
+    _not_send: PhantomData<*mut ()>,
+}
+
+impl Guard {
+    /// Acquires the RCU read side lock and returns a guard.
+    pub fn new() -> Self {
+        // SAFETY: An FFI call with no additional requirements.
+        unsafe { bindings::rcu_read_lock() };
+        // INVARIANT: The RCU read side lock was just acquired above.
+        Self {
+            _not_send: PhantomData,
+        }
+    }
+
+    /// Explicitly releases the RCU read side lock.
+    pub fn unlock(self) {}
+}
+
+impl Default for Guard {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl Drop for Guard {
+    fn drop(&mut self) {
+        // SAFETY: By the type invariants, the rcu read side is locked, so it is ok to unlock it.
+        unsafe { bindings::rcu_read_unlock() };
+    }
+}
+
+/// Acquires the RCU read side lock.
+pub fn read_lock() -> Guard {
+    Guard::new()
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC v2 13/13] rust: sync: rcu: Add RCU protected pointer
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (11 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 12/13] rust: add rcu abstraction Boqun Feng
@ 2024-11-01  6:02 ` Boqun Feng
  2024-11-01 14:30 ` [RFC v2 00/13] LKMM *generic* atomics in Rust Miguel Ojeda
  2024-11-02  7:35 ` David Gow
  14 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  6:02 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

RCU protected pointers are an atomic pointer that can be loaded and
dereferenced by mulitple RCU readers, but only one updater/writer can
change the value (following a read-copy-update pattern usually).

This is useful in the case where data is read-mostly. The rationale of
this patch is to provide a proof of concept on how RCU should be exposed
to the Rust world, and it also serves as an example for atomic usage.

Similar mechanisms like ArcSwap [1] are already widely used.

Provide a `Rcu<P>` type with an atomic pointer implementation. `P` has
to be a `ForeignOwnable`, which means the ownership of a object can be
represented by a pointer-size value.

`Rcu::dereference()` requires a RCU Guard, which means dereferencing is
only valid under RCU read lock protection.

`Rcu::read_copy_update()` is the operation for updaters, it requries a
`Pin<&mut Self>` for exclusive accesses, since RCU updaters are normally
exclusive with each other.

A lot of RCU functionalities including asynchronously free (call_rcu()
and kfree_rcu()) are still missing, and will be the future work.

Also, we still need language changes like field projection [2] to
provide better ergonomic.

Acknowledgment: this work is based on a lot of productive discussions
and hard work from others, these are the ones I can remember (sorry if I
forgot your contribution):

* Wedson started the work on RCU field projection and Benno followed it
  up and had been working on it as a more general language feature.
  Also, Gary's field-projection repo [3] has been used as an example for
  related discussions.

* During Kangrejos 2023 [4], Gary, Benno and Alice provided a lot of
  feedbacks on the talk from Paul and me: "If you want to use RCU in
  Rust for Linux kernel..."

* During a recent discussion among Benno, Paul and me, Benno suggested
  using `Pin<&mut>` to guarantee the exclusive access on updater
  operations.

Link: https://crates.io/crates/arc-swap [1]
Link: https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/Field.20Projections/near/474648059 [2]
Link: https://github.com/nbdd0121/field-projection [3]
Link: https://kangrejos.com/2023 [4]
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/rcu.rs | 269 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 268 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/sync/rcu.rs b/rust/kernel/sync/rcu.rs
index 5a35495f69a4..8326b2e0986a 100644
--- a/rust/kernel/sync/rcu.rs
+++ b/rust/kernel/sync/rcu.rs
@@ -5,7 +5,11 @@
 //! C header: [`include/linux/rcupdate.h`](srctree/include/linux/rcupdate.h)
 
 use crate::bindings;
-use core::marker::PhantomData;
+use crate::{
+    sync::atomic::{Atomic, Relaxed, Release},
+    types::ForeignOwnable,
+};
+use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
 
 /// Evidence that the RCU read side lock is held on the current thread/CPU.
 ///
@@ -50,3 +54,266 @@ fn drop(&mut self) {
 pub fn read_lock() -> Guard {
     Guard::new()
 }
+
+/// An RCU protected pointer, the pointed object is protected by RCU.
+///
+/// # Invariants
+///
+/// Either the pointer is null, or it points to a return value of [`P::into_foreign`] and the atomic
+/// variable exclusively owns the pointer.
+pub struct Rcu<P: ForeignOwnable>(Atomic<*mut core::ffi::c_void>, PhantomData<P>);
+
+/// A pointer that has been unpublished, but hasn't waited for a grace period yet.
+///
+/// The pointed object may still have an existing RCU reader. Therefore a grace period is needed to
+/// free the object.
+///
+/// # Invariants
+///
+/// The pointer has to be a return value of [`P::into_foreign`] and [`Self`] exclusively owns the
+/// pointer.
+pub struct RcuOld<P: ForeignOwnable>(NonNull<core::ffi::c_void>, PhantomData<P>);
+
+impl<P: ForeignOwnable> Drop for RcuOld<P> {
+    fn drop(&mut self) {
+        // SAFETY: As long as called in a sleepable context, which should be checked by klint,
+        // `synchronize_rcu()` is safe to call.
+        unsafe {
+            bindings::synchronize_rcu();
+        }
+
+        // SAFETY: `self.0` is a return value of `P::into_foreign()`, so it's safe to call
+        // `from_foreign()` on it. Plus, the above `synchronize_rcu()` guarantees no existing
+        // `ForeignOwnable::borrow()` anymore.
+        let p: P = unsafe { P::from_foreign(self.0.as_ptr()) };
+        drop(p);
+    }
+}
+
+impl<P: ForeignOwnable> Rcu<P> {
+    /// Creates a new RCU pointer.
+    pub fn new(p: P) -> Self {
+        // INVARIANTS: The return value of `p.into_foreign()` is directly stored in the atomic
+        // variable.
+        Self(Atomic::new(p.into_foreign().cast_mut()), PhantomData)
+    }
+
+    /// Dereferences the protected object.
+    ///
+    /// Returns `Some(b)`, where `b` is a reference-like borrowed type, if the pointer is not null,
+    /// otherwise returns `None`.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # use kernel::alloc::{flags, KBox};
+    /// use kernel::sync::rcu::{self, Rcu};
+    ///
+    /// let x = Rcu::new(KBox::new(100i32, flags::GFP_KERNEL)?);
+    ///
+    /// let g = rcu::read_lock();
+    /// // Read in under RCU read lock protection.
+    /// let v = x.dereference(&g);
+    ///
+    /// assert_eq!(v, Some(&100i32));
+    ///
+    /// # Ok::<(), Error>(())
+    /// ```
+    ///
+    /// Note the borrowed access can outlive the reference of the [`Rcu<P>`], this is because as
+    /// long as the RCU read lock is held, the pointed object should remain valid.
+    ///
+    /// In the following case, the main thread is responsible for the ownership of `shared`, i.e. it
+    /// will drop it eventually, and a work item can temporarily access the `shared` via `cloned`,
+    /// but the use of the dereferenced object doesn't depend on `cloned`'s existence.
+    ///
+    /// ```rust
+    /// # use kernel::alloc::{flags, KBox};
+    /// # use kernel::workqueue::system;
+    /// # use kernel::sync::{Arc, atomic::{Atomic, Acquire, Release}};
+    /// use kernel::sync::rcu::{self, Rcu};
+    ///
+    /// struct Config {
+    ///     a: i32,
+    ///     b: i32,
+    ///     c: i32,
+    /// }
+    ///
+    /// let config = KBox::new(Config { a: 1, b: 2, c: 3 }, flags::GFP_KERNEL)?;
+    ///
+    /// let shared = Arc::new(Rcu::new(config), flags::GFP_KERNEL)?;
+    /// let cloned = shared.clone();
+    ///
+    /// // Use atomic to simulate a special refcounting.
+    /// static FLAG: Atomic<i32> = Atomic::new(0);
+    ///
+    /// system().try_spawn(flags::GFP_KERNEL, move || {
+    ///     let g = rcu::read_lock();
+    ///     let v = cloned.dereference(&g).unwrap();
+    ///     drop(cloned); // release reference to `shared`.
+    ///     FLAG.store(1, Release);
+    ///
+    ///     // but still need to access `v`.
+    ///     assert_eq!(v.a, 1);
+    ///     drop(g);
+    /// });
+    ///
+    /// // Wait until `cloned` dropped.
+    /// while FLAG.load(Acquire) == 0 {
+    ///     // SAFETY: Sleep should be safe.
+    ///     unsafe { kernel::bindings::schedule(); }
+    /// }
+    ///
+    /// drop(shared);
+    ///
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn dereference<'rcu>(&self, _rcu_guard: &'rcu Guard) -> Option<P::Borrowed<'rcu>> {
+        // Ordering: Address dependency pairs with the `store(Release)` in read_copy_update().
+        let ptr = self.0.load(Relaxed);
+
+        if !ptr.is_null() {
+            // SAFETY:
+            // - Since `ptr` is not null, so it has to be a return value of `P::into_foreign()`.
+            // - The returned `Borrowed<'rcu>` cannot outlive the RCU Guar, this guarantees the
+            //   return value will only be used under RCU read lock, and the RCU read lock prevents
+            //   the pass of a grace period that the drop of `RcuOld` or `Rcu` is waiting for,
+            //   therefore no `from_foreign()` will be called for `ptr` as long as `Borrowed` exists.
+            //
+            //      CPU 0                                       CPU 1
+            //      =====                                       =====
+            //      { `x` is a reference to Rcu<Box<i32>> }
+            //      let g = rcu::read_lock();
+            //
+            //      if let Some(b) = x.dereference(&g) {
+            //      // drop(g); cannot be done, since `b` is still alive.
+            //
+            //                                              if let Some(old) = x.replace(...) {
+            //                                                  // `x` is null now.
+            //          println!("{}", b);
+            //      }
+            //                                                  drop(old):
+            //                                                    synchronize_rcu();
+            //      drop(g);
+            //                                                    // a grace period passed.
+            //                                                    // No `Borrowed` exists now.
+            //                                                    from_foreign(...);
+            //                                              }
+            Some(unsafe { P::borrow(ptr) })
+        } else {
+            None
+        }
+    }
+
+    /// Read, copy and update the pointer with new value.
+    ///
+    /// Returns `None` if the pointer's old value is null, otherwise returns `Some(old)`, where old
+    /// is a [`RcuOld`] which can be used to free the old object eventually.
+    ///
+    /// The `Pin<&mut Self>` is needed because this function needs the exclusive access to
+    /// [`Rcu<P>`], otherwise two `read_copy_update()`s may get the same old object and double free.
+    /// Using `Pin<&mut Self>` provides the exclusive access that C side requires with the type
+    /// system checking.
+    ///
+    /// Also this has to be `Pin` because a `&mut Self` may allow users to `swap()` safely, that
+    /// will break the atomicity. A [`Rcu<P>`] should be structurally pinned in the struct that
+    /// contains it.
+    ///
+    /// Note that `Pin<&mut Self>` cannot assume noalias here because [`Atomic<T>`] is a
+    /// [`Opaque<T>`] which has the same effect on aliasing rules as [`UnsafePinned`].
+    ///
+    /// [`UnsafePinned`]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html
+    pub fn read_copy_update<F>(self: Pin<&mut Self>, f: F) -> Option<RcuOld<P>>
+    where
+        F: FnOnce(Option<P::Borrowed<'_>>) -> Option<P>,
+    {
+        // step 1: READ.
+        // Ordering: Address dependency pairs with the `store(Release)` in read_copy_update().
+        let old_ptr = NonNull::new(self.0.load(Relaxed));
+
+        let old = old_ptr.map(|nonnull| {
+            // SAFETY: Per type invariants `old_ptr` has to be a value return by a previous
+            // `into_foreign()`, and the exclusive reference `self` guarantees that `from_foreign()`
+            // has not been called.
+            unsafe { P::borrow(nonnull.as_ptr()) }
+        });
+
+        // step 2: COPY, or more generally, initializing `new` based on `old`.
+        let new = f(old);
+
+        // step 3: UPDATE.
+        if let Some(new) = new {
+            let new_ptr = new.into_foreign().cast_mut();
+            // Ordering: Pairs with the address dependency in `dereference()` and
+            // `read_copy_update()`.
+            // INVARIANTS: `new.into_foreign()` is directly store into the atomic variable.
+            self.0.store(new_ptr, Release);
+        } else {
+            // Ordering: Setting to a null pointer doesn't need to be Release.
+            // INVARIANTS: The atomic variable is set to be null.
+            self.0.store(core::ptr::null_mut(), Relaxed);
+        }
+
+        // INVARIANTS: The exclusive reference guarantess that the ownership of a previous
+        // `into_foreign()` transferred to the `RcuOld`.
+        Some(RcuOld(old_ptr?, PhantomData))
+    }
+
+    /// Replaces the pointer with new value.
+    ///
+    /// Returns `None` if the pointer's old value is null, otherwise returns `Some(old)`, where old
+    /// is a [`RcuOld`] which can be used to free the old object eventually.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use core::pin::pin;
+    /// # use kernel::alloc::{flags, KBox};
+    /// use kernel::sync::rcu::{self, Rcu};
+    ///
+    /// let mut x = pin!(Rcu::new(KBox::new(100i32, flags::GFP_KERNEL)?));
+    /// let q = KBox::new(101i32, flags::GFP_KERNEL)?;
+    ///
+    /// // Read in under RCU read lock protection.
+    /// let g = rcu::read_lock();
+    /// let v = x.dereference(&g);
+    ///
+    /// // Replace with a new object.
+    /// let old = x.as_mut().replace(q);
+    ///
+    /// assert!(old.is_some());
+    ///
+    /// // `v` should still read the old value.
+    /// assert_eq!(v, Some(&100i32));
+    ///
+    /// // New readers should get the new value.
+    /// assert_eq!(x.dereference(&g), Some(&101i32));
+    ///
+    /// drop(g);
+    ///
+    /// // Can free the object outside the read-side critical section.
+    /// drop(old);
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn replace(self: Pin<&mut Self>, new: P) -> Option<RcuOld<P>> {
+        self.read_copy_update(|_| Some(new))
+    }
+}
+
+impl<P: ForeignOwnable> Drop for Rcu<P> {
+    fn drop(&mut self) {
+        let ptr = *self.0.get_mut();
+        if !ptr.is_null() {
+            // SAFETY: As long as called in a sleepable context, which should be checked by klint,
+            // `synchronize_rcu()` is safe to call.
+            unsafe {
+                bindings::synchronize_rcu();
+            }
+
+            // SAFETY: `self.0` is a return value of `P::into_foreign()`, so it's safe to call
+            // `from_foreign()` on it. Plus, the above `synchronize_rcu()` guarantees no existing
+            // `ForeignOwnable::borrow()` anymore.
+            drop(unsafe { P::from_foreign(ptr) });
+        }
+    }
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RFC v2 11/13] rust: sync: Add memory barriers
  2024-11-01  6:02 ` [RFC v2 11/13] rust: sync: Add memory barriers Boqun Feng
@ 2024-11-01  6:55   ` David Gow
  2024-11-01  7:04     ` Boqun Feng
  2024-11-01  7:01   ` [RFC v2.1 " Boqun Feng
  1 sibling, 1 reply; 28+ messages in thread
From: David Gow @ 2024-11-01  6:55 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

[-- Attachment #1: Type: text/plain, Size: 1136 bytes --]

On Fri, 1 Nov 2024 at 14:07, Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Memory barriers are building blocks for concurrent code, hence provide
> a minimal set of them.
>
> The compiler barrier, barrier(), is implemented in inline asm instead of
> using core::sync::atomic::compiler_fence() because memory models are
> different: kernel's atomics are implemented in inline asm therefore the
> compiler barrier should be implemented in inline asm as well.
>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  rust/helpers/helpers.c      |  1 +
>  rust/kernel/sync.rs         |  1 +
>  rust/kernel/sync/barrier.rs | 67 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 69 insertions(+)
>  create mode 100644 rust/kernel/sync/barrier.rs
>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index ab5a3f1be241..f4a94833b29d 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -8,6 +8,7 @@
>   */
>
>  #include "atomic.c"
> +#include "barrier.c"

It looks like "barrier.c" is missing, so this isn't compiling for me.
I assume it was meant to be added in this patch...?

Thanks,
-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RFC v2.1 11/13] rust: sync: Add memory barriers
  2024-11-01  6:02 ` [RFC v2 11/13] rust: sync: Add memory barriers Boqun Feng
  2024-11-01  6:55   ` David Gow
@ 2024-11-01  7:01   ` Boqun Feng
  1 sibling, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  7:01 UTC (permalink / raw)
  To: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-fsdevel, Trevor Gross, dakr,
	Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett,
	Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
	Lai Jiangshan, Zqiang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-riscv

Memory barriers are building blocks for concurrent code, hence provide
a minimal set of them.

The compiler barrier, barrier(), is implemented in inline asm instead of
using core::sync::atomic::compiler_fence() because memory models are
different: kernel's atomics are implemented in inline asm therefore the
compiler barrier should be implemented in inline asm as well.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
v2 -> v2.1: Add the missing barrier.c

 rust/helpers/barrier.c      | 18 ++++++++++
 rust/helpers/helpers.c      |  1 +
 rust/kernel/sync.rs         |  1 +
 rust/kernel/sync/barrier.rs | 67 +++++++++++++++++++++++++++++++++++++
 4 files changed, 87 insertions(+)
 create mode 100644 rust/helpers/barrier.c
 create mode 100644 rust/kernel/sync/barrier.rs

diff --git a/rust/helpers/barrier.c b/rust/helpers/barrier.c
new file mode 100644
index 000000000000..cdf28ce8e511
--- /dev/null
+++ b/rust/helpers/barrier.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <asm/barrier.h>
+
+void rust_helper_smp_mb(void)
+{
+	smp_mb();
+}
+
+void rust_helper_smp_wmb(void)
+{
+	smp_wmb();
+}
+
+void rust_helper_smp_rmb(void)
+{
+	smp_rmb();
+}
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index ab5a3f1be241..f4a94833b29d 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -8,6 +8,7 @@
  */
 
 #include "atomic.c"
+#include "barrier.c"
 #include "blk.c"
 #include "bug.c"
 #include "build_assert.c"
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 66ac3752ca71..0d0b19441ae8 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -9,6 +9,7 @@
 
 mod arc;
 pub mod atomic;
+pub mod barrier;
 mod condvar;
 pub mod lock;
 mod locked_by;
diff --git a/rust/kernel/sync/barrier.rs b/rust/kernel/sync/barrier.rs
new file mode 100644
index 000000000000..277aa09747bf
--- /dev/null
+++ b/rust/kernel/sync/barrier.rs
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Memory barriers.
+//!
+//! These primitives have the same semantics as their C counterparts: and the precise definitions of
+//! semantics can be found at [`LKMM`].
+//!
+//! [`LKMM`]: srctree/tools/memory-mode/
+
+/// A compiler barrier.
+///
+/// An explicic compiler barrier function that prevents the compiler from moving the memory
+/// accesses either side of it to the other side.
+pub fn barrier() {
+    // By default, Rust inline asms are treated as being able to access any memory or flags, hence
+    // it suffices as a compiler barrier.
+    //
+    // SAFETY: An empty asm block should be safe.
+    unsafe {
+        core::arch::asm!("");
+    }
+}
+
+/// A full memory barrier.
+///
+/// A barrier function that prevents both the compiler and the CPU from moving the memory accesses
+/// either side of it to the other side.
+pub fn smp_mb() {
+    if cfg!(CONFIG_SMP) {
+        // SAFETY: `smp_mb()` is safe to call.
+        unsafe {
+            bindings::smp_mb();
+        }
+    } else {
+        barrier();
+    }
+}
+
+/// A write-write memory barrier.
+///
+/// A barrier function that prevents both the compiler and the CPU from moving the memory write
+/// accesses either side of it to the other side.
+pub fn smp_wmb() {
+    if cfg!(CONFIG_SMP) {
+        // SAFETY: `smp_wmb()` is safe to call.
+        unsafe {
+            bindings::smp_wmb();
+        }
+    } else {
+        barrier();
+    }
+}
+
+/// A read-read memory barrier.
+///
+/// A barrier function that prevents both the compiler and the CPU from moving the memory read
+/// accesses either side of it to the other side.
+pub fn smp_rmb() {
+    if cfg!(CONFIG_SMP) {
+        // SAFETY: `smp_rmb()` is safe to call.
+        unsafe {
+            bindings::smp_rmb();
+        }
+    } else {
+        barrier();
+    }
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RFC v2 11/13] rust: sync: Add memory barriers
  2024-11-01  6:55   ` David Gow
@ 2024-11-01  7:04     ` Boqun Feng
  0 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-11-01  7:04 UTC (permalink / raw)
  To: David Gow
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

On Fri, Nov 01, 2024 at 02:55:23PM +0800, David Gow wrote:
> On Fri, 1 Nov 2024 at 14:07, Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Memory barriers are building blocks for concurrent code, hence provide
> > a minimal set of them.
> >
> > The compiler barrier, barrier(), is implemented in inline asm instead of
> > using core::sync::atomic::compiler_fence() because memory models are
> > different: kernel's atomics are implemented in inline asm therefore the
> > compiler barrier should be implemented in inline asm as well.
> >
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  rust/helpers/helpers.c      |  1 +
> >  rust/kernel/sync.rs         |  1 +
> >  rust/kernel/sync/barrier.rs | 67 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 69 insertions(+)
> >  create mode 100644 rust/kernel/sync/barrier.rs
> >
> > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> > index ab5a3f1be241..f4a94833b29d 100644
> > --- a/rust/helpers/helpers.c
> > +++ b/rust/helpers/helpers.c
> > @@ -8,6 +8,7 @@
> >   */
> >
> >  #include "atomic.c"
> > +#include "barrier.c"
> 
> It looks like "barrier.c" is missing, so this isn't compiling for me.
> I assume it was meant to be added in this patch...?
> 

Yes, I just send an updated one.

Glad being "checking missing files" buddies with you ;-) Thanks!

Regards,
Boqun

> Thanks,
> -- David



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 00/13] LKMM *generic* atomics in Rust
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (12 preceding siblings ...)
  2024-11-01  6:02 ` [RFC v2 13/13] rust: sync: rcu: Add RCU protected pointer Boqun Feng
@ 2024-11-01 14:30 ` Miguel Ojeda
  2024-11-02  7:35 ` David Gow
  14 siblings, 0 replies; 28+ messages in thread
From: Miguel Ojeda @ 2024-11-01 14:30 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 00/13] LKMM *generic* atomics in Rust
  2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
                   ` (13 preceding siblings ...)
  2024-11-01 14:30 ` [RFC v2 00/13] LKMM *generic* atomics in Rust Miguel Ojeda
@ 2024-11-02  7:35 ` David Gow
  2025-04-21 16:27   ` Boqun Feng
  14 siblings, 1 reply; 28+ messages in thread
From: David Gow @ 2024-11-02  7:35 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

[-- Attachment #1: Type: text/plain, Size: 7736 bytes --]

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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework
  2024-11-01  6:02 ` [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework Boqun Feng
@ 2024-12-12 10:51   ` Alice Ryhl
  2024-12-12 17:07     ` Boqun Feng
  0 siblings, 1 reply; 28+ messages in thread
From: Alice Ryhl @ 2024-12-12 10:51 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alan Stern,
	Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
	David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
	Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
	Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
	Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Catalin Marinas, torvalds,
	linux-arm-kernel, linux-fsdevel, Trevor Gross, dakr,
	Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett,
	Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
	Lai Jiangshan, Zqiang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-riscv

On Fri, Nov 1, 2024 at 7:03 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Preparation for generic atomic implementation. To unify the
> ipmlementation of a generic method over `i32` and `i64`, the C side
> atomic methods need to be grouped so that in a generic method, they can
> be referred as <type>::<method>, otherwise their parameters and return
> value are different between `i32` and `i64`, which would require using
> `transmute()` to unify the type into a `T`.
>
> Introduce `AtomicIpml` to represent a basic type in Rust that has the
> direct mapping to an atomic implementation from C. This trait is sealed,
> and currently only `i32` and `i64` ipml this.

There seems to be quite a few instances of "impl" spelled as "ipml" here.

> Further, different methods are put into different `*Ops` trait groups,
> and this is for the future when smaller types like `i8`/`i16` are
> supported but only with a limited set of API (e.g. only set(), load(),
> xchg() and cmpxchg(), no add() or sub() etc).
>
> While the atomic mod is introduced, documentation is also added for
> memory models and data races.
>
> Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
> my responsiblity on the Rust atomic mod.
>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  MAINTAINERS                    |   4 +-
>  rust/kernel/sync.rs            |   1 +
>  rust/kernel/sync/atomic.rs     |  19 ++++
>  rust/kernel/sync/atomic/ops.rs | 199 +++++++++++++++++++++++++++++++++
>  4 files changed, 222 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/sync/atomic.rs
>  create mode 100644 rust/kernel/sync/atomic/ops.rs
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b77f4495dcf4..e09471027a63 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3635,7 +3635,7 @@ F:        drivers/input/touchscreen/atmel_mxt_ts.c
>  ATOMIC INFRASTRUCTURE
>  M:     Will Deacon <will@kernel.org>
>  M:     Peter Zijlstra <peterz@infradead.org>
> -R:     Boqun Feng <boqun.feng@gmail.com>
> +M:     Boqun Feng <boqun.feng@gmail.com>
>  R:     Mark Rutland <mark.rutland@arm.com>
>  L:     linux-kernel@vger.kernel.org
>  S:     Maintained
> @@ -3644,6 +3644,8 @@ F:        arch/*/include/asm/atomic*.h
>  F:     include/*/atomic*.h
>  F:     include/linux/refcount.h
>  F:     scripts/atomic/
> +F:     rust/kernel/sync/atomic.rs
> +F:     rust/kernel/sync/atomic/

This is why mod.rs files are superior :)

> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Atomic primitives.
> +//!
> +//! These primitives have the same semantics as their C counterparts: and the precise definitions of
> +//! semantics can be found at [`LKMM`]. Note that Linux Kernel Memory (Consistency) Model is the
> +//! only model for Rust code in kernel, and Rust's own atomics should be avoided.
> +//!
> +//! # Data races
> +//!
> +//! [`LKMM`] atomics have different rules regarding data races:
> +//!
> +//! - A normal read doesn't data-race with an atomic read.

This was fixed:
https://github.com/rust-lang/rust/pull/128778

> +mod private {
> +    /// Sealed trait marker to disable customized impls on atomic implementation traits.
> +    pub trait Sealed {}
> +}

Just make the trait unsafe?

Alice

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 04/13] rust: sync: atomic: Add generic atomics
  2024-11-01  6:02 ` [RFC v2 04/13] rust: sync: atomic: Add generic atomics Boqun Feng
@ 2024-12-12 10:57   ` Alice Ryhl
  2024-12-12 17:34     ` Boqun Feng
  0 siblings, 1 reply; 28+ messages in thread
From: Alice Ryhl @ 2024-12-12 10:57 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alan Stern,
	Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
	David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
	Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
	Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
	Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Catalin Marinas, torvalds,
	linux-arm-kernel, linux-fsdevel, Trevor Gross, dakr,
	Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett,
	Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
	Lai Jiangshan, Zqiang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-riscv

On Fri, Nov 1, 2024 at 7:03 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> To provide using LKMM atomics for Rust code, a generic `Atomic<T>` is
> added, currently `T` needs to be Send + Copy because these are the
> straightforward usages and all basic types support this. The trait
> `AllowAtomic` should be only ipmlemented inside atomic mod until the
> generic atomic framework is mature enough (unless the ipmlementer is a
> `#[repr(transparent)]` new type).
>
> `AtomicIpml` types are automatically `AllowAtomic`, and so far only
> basic operations load() and store() are introduced.

The ipml typo continues in this patch.

> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  rust/kernel/sync/atomic.rs         |   2 +
>  rust/kernel/sync/atomic/generic.rs | 253 +++++++++++++++++++++++++++++
>  2 files changed, 255 insertions(+)
>  create mode 100644 rust/kernel/sync/atomic/generic.rs
>
> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> index be2e8583595f..b791abc59b61 100644
> --- a/rust/kernel/sync/atomic.rs
> +++ b/rust/kernel/sync/atomic.rs
> @@ -16,7 +16,9 @@
>  //!
>  //! [`LKMM`]: srctree/tools/memory-mode/
>
> +pub mod generic;
>  pub mod ops;
>  pub mod ordering;
>
> +pub use generic::Atomic;
>  pub use ordering::{Acquire, Full, Relaxed, Release};
> diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
> new file mode 100644
> index 000000000000..204da38e2691
> --- /dev/null
> +++ b/rust/kernel/sync/atomic/generic.rs
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Generic atomic primitives.
> +
> +use super::ops::*;
> +use super::ordering::*;
> +use crate::types::Opaque;
> +
> +/// A generic atomic variable.
> +///
> +/// `T` must impl [`AllowAtomic`], that is, an [`AtomicImpl`] has to be chosen.
> +///
> +/// # Invariants
> +///
> +/// Doing an atomic operation while holding a reference of [`Self`] won't cause a data race, this
> +/// is guaranteed by the safety requirement of [`Self::from_ptr`] and the extra safety requirement
> +/// of the usage on pointers returned by [`Self::as_ptr`].
> +#[repr(transparent)]
> +pub struct Atomic<T: AllowAtomic>(Opaque<T>);
> +
> +// SAFETY: `Atomic<T>` is safe to share among execution contexts because all accesses are atomic.
> +unsafe impl<T: AllowAtomic> Sync for Atomic<T> {}

Surely it should also be Send?

> +/// Atomics that support basic atomic operations.
> +///
> +/// TODO: Unless the `impl` is a `#[repr(transparet)]` new type of an existing [`AllowAtomic`], the
> +/// impl block should be only done in atomic mod. And currently only basic integer types can
> +/// implement this trait in atomic mod.

What's up with this TODO? Can't you just write an appropriate safety
requirement?

> +/// # Safety
> +///
> +/// [`Self`] must have the same size and alignment as [`Self::Repr`].
> +pub unsafe trait AllowAtomic: Sized + Send + Copy {
> +    /// The backing atomic implementation type.
> +    type Repr: AtomicImpl;
> +
> +    /// Converts into a [`Self::Repr`].
> +    fn into_repr(self) -> Self::Repr;
> +
> +    /// Converts from a [`Self::Repr`].
> +    fn from_repr(repr: Self::Repr) -> Self;

What do you need these methods for?

> +}
> +
> +// SAFETY: `T::Repr` is `Self` (i.e. `T`), so they have the same size and alignment.
> +unsafe impl<T: AtomicImpl> AllowAtomic for T {
> +    type Repr = Self;
> +
> +    fn into_repr(self) -> Self::Repr {
> +        self
> +    }
> +
> +    fn from_repr(repr: Self::Repr) -> Self {
> +        repr
> +    }
> +}
> +
> +impl<T: AllowAtomic> Atomic<T> {
> +    /// Creates a new atomic.
> +    pub const fn new(v: T) -> Self {
> +        Self(Opaque::new(v))
> +    }
> +
> +    /// Creates a reference to [`Self`] from a pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` has to be a valid pointer.
> +    /// - `ptr` has to be valid for both reads and writes for the whole lifetime `'a`.
> +    /// - For the whole lifetime of '`a`, other accesses to the object cannot cause data races
> +    ///   (defined by [`LKMM`]) against atomic operations on the returned reference.
> +    ///
> +    /// [`LKMM`]: srctree/tools/memory-model
> +    ///
> +    /// # Examples
> +    ///
> +    /// Using [`Atomic::from_ptr()`] combined with [`Atomic::load()`] or [`Atomic::store()`] can
> +    /// achieve the same functionality as `READ_ONCE()`/`smp_load_acquire()` or
> +    /// `WRITE_ONCE()`/`smp_store_release()` in C side:
> +    ///
> +    /// ```rust
> +    /// # use kernel::types::Opaque;
> +    /// use kernel::sync::atomic::{Atomic, Relaxed, Release};
> +    ///
> +    /// // Assume there is a C struct `Foo`.
> +    /// mod cbindings {
> +    ///     #[repr(C)]
> +    ///     pub(crate) struct foo { pub(crate) a: i32, pub(crate) b: i32 }
> +    /// }
> +    ///
> +    /// let tmp = Opaque::new(cbindings::foo { a: 1, b: 2});
> +    ///
> +    /// // struct foo *foo_ptr = ..;
> +    /// let foo_ptr = tmp.get();
> +    ///
> +    /// // SAFETY: `foo_ptr` is a valid pointer, and `.a` is inbound.
> +    /// let foo_a_ptr = unsafe { core::ptr::addr_of_mut!((*foo_ptr).a) };
> +    ///
> +    /// // a = READ_ONCE(foo_ptr->a);
> +    /// //
> +    /// // SAFETY: `foo_a_ptr` is a valid pointer for read, and all accesses on it is atomic, so no
> +    /// // data race.
> +    /// let a = unsafe { Atomic::from_ptr(foo_a_ptr) }.load(Relaxed);
> +    /// # assert_eq!(a, 1);
> +    ///
> +    /// // smp_store_release(&foo_ptr->a, 2);
> +    /// //
> +    /// // SAFETY: `foo_a_ptr` is a valid pointer for write, and all accesses on it is atomic, so no
> +    /// // data race.
> +    /// unsafe { Atomic::from_ptr(foo_a_ptr) }.store(2, Release);
> +    /// ```
> +    ///
> +    /// However, this should be only used when communicating with C side or manipulating a C struct.
> +    pub unsafe fn from_ptr<'a>(ptr: *mut T) -> &'a Self
> +    where
> +        T: Sync,
> +    {
> +        // CAST: `T` is transparent to `Atomic<T>`.
> +        // SAFETY: Per function safety requirement, `ptr` is a valid pointer and the object will
> +        // live long enough. It's safe to return a `&Atomic<T>` because function safety requirement
> +        // guarantees other accesses won't cause data races.
> +        unsafe { &*ptr.cast::<Self>() }
> +    }
> +
> +    /// Returns a pointer to the underlying atomic variable.
> +    ///
> +    /// Extra safety requirement on using the return pointer: the operations done via the pointer
> +    /// cannot cause data races defined by [`LKMM`].
> +    ///
> +    /// [`LKMM`]: srctree/tools/memory-model
> +    pub const fn as_ptr(&self) -> *mut T {
> +        self.0.get()
> +    }
> +
> +    /// Returns a mutable reference to the underlying atomic variable.
> +    ///
> +    /// This is safe because the mutable reference of the atomic variable guarantees the exclusive
> +    /// access.
> +    pub fn get_mut(&mut self) -> &mut T {
> +        // SAFETY: `self.as_ptr()` is a valid pointer to `T`, and the object has already been
> +        // initialized. `&mut self` guarantees the exclusive access, so it's safe to reborrow
> +        // mutably.
> +        unsafe { &mut *self.as_ptr() }
> +    }
> +}
> +
> +impl<T: AllowAtomic> Atomic<T>
> +where
> +    T::Repr: AtomicHasBasicOps,
> +{
> +    /// Loads the value from the atomic variable.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Simple usages:
> +    ///
> +    /// ```rust
> +    /// use kernel::sync::atomic::{Atomic, Relaxed};
> +    ///
> +    /// let x = Atomic::new(42i32);
> +    ///
> +    /// assert_eq!(42, x.load(Relaxed));
> +    ///
> +    /// let x = Atomic::new(42i64);
> +    ///
> +    /// assert_eq!(42, x.load(Relaxed));
> +    /// ```
> +    ///
> +    /// Customized new types in [`Atomic`]:
> +    ///
> +    /// ```rust
> +    /// use kernel::sync::atomic::{generic::AllowAtomic, Atomic, Relaxed};
> +    ///
> +    /// #[derive(Clone, Copy)]
> +    /// #[repr(transparent)]
> +    /// struct NewType(u32);
> +    ///
> +    /// // SAFETY: `NewType` is transparent to `u32`, which has the same size and alignment as
> +    /// // `i32`.
> +    /// unsafe impl AllowAtomic for NewType {
> +    ///     type Repr = i32;
> +    ///
> +    ///     fn into_repr(self) -> Self::Repr {
> +    ///         self.0 as i32
> +    ///     }
> +    ///
> +    ///     fn from_repr(repr: Self::Repr) -> Self {
> +    ///         NewType(repr as u32)
> +    ///     }
> +    /// }
> +    ///
> +    /// let n = Atomic::new(NewType(0));
> +    ///
> +    /// assert_eq!(0, n.load(Relaxed).0);
> +    /// ```
> +    #[inline(always)]
> +    pub fn load<Ordering: AcquireOrRelaxed>(&self, _: Ordering) -> T {
> +        let a = self.as_ptr().cast::<T::Repr>();
> +
> +        // SAFETY:
> +        // - For calling the atomic_read*() function:
> +        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
> +        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
> +        //   - per the type invariants, the following atomic operation won't cause data races.
> +        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
> +        //   - atomic operations are used here.
> +        let v = unsafe {
> +            if Ordering::IS_RELAXED {
> +                T::Repr::atomic_read(a)
> +            } else {
> +                T::Repr::atomic_read_acquire(a)
> +            }
> +        };
> +
> +        T::from_repr(v)
> +    }
> +
> +    /// Stores a value to the atomic variable.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```rust
> +    /// use kernel::sync::atomic::{Atomic, Relaxed};
> +    ///
> +    /// let x = Atomic::new(42i32);
> +    ///
> +    /// assert_eq!(42, x.load(Relaxed));
> +    ///
> +    /// x.store(43, Relaxed);
> +    ///
> +    /// assert_eq!(43, x.load(Relaxed));
> +    /// ```
> +    ///
> +    #[inline(always)]
> +    pub fn store<Ordering: ReleaseOrRelaxed>(&self, v: T, _: Ordering) {
> +        let v = T::into_repr(v);
> +        let a = self.as_ptr().cast::<T::Repr>();
> +
> +        // SAFETY:
> +        // - For calling the atomic_set*() function:
> +        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
> +        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
> +        //   - per the type invariants, the following atomic operation won't cause data races.
> +        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
> +        //   - atomic operations are used here.
> +        unsafe {
> +            if Ordering::IS_RELAXED {
> +                T::Repr::atomic_set(a, v)
> +            } else {
> +                T::Repr::atomic_set_release(a, v)
> +            }
> +        };
> +    }
> +}
> --
> 2.45.2
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework
  2024-12-12 10:51   ` Alice Ryhl
@ 2024-12-12 17:07     ` Boqun Feng
  2024-12-13 14:37       ` Alice Ryhl
  0 siblings, 1 reply; 28+ messages in thread
From: Boqun Feng @ 2024-12-12 17:07 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alan Stern,
	Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
	David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
	Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
	Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
	Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Catalin Marinas, torvalds,
	linux-arm-kernel, linux-fsdevel, Trevor Gross, dakr,
	Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett,
	Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
	Lai Jiangshan, Zqiang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-riscv

On Thu, Dec 12, 2024 at 11:51:23AM +0100, Alice Ryhl wrote:
> On Fri, Nov 1, 2024 at 7:03 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Preparation for generic atomic implementation. To unify the
> > ipmlementation of a generic method over `i32` and `i64`, the C side
> > atomic methods need to be grouped so that in a generic method, they can
> > be referred as <type>::<method>, otherwise their parameters and return
> > value are different between `i32` and `i64`, which would require using
> > `transmute()` to unify the type into a `T`.
> >
> > Introduce `AtomicIpml` to represent a basic type in Rust that has the
> > direct mapping to an atomic implementation from C. This trait is sealed,
> > and currently only `i32` and `i64` ipml this.
> 
> There seems to be quite a few instances of "impl" spelled as "ipml" here.
> 

Will fix!

> > Further, different methods are put into different `*Ops` trait groups,
> > and this is for the future when smaller types like `i8`/`i16` are
> > supported but only with a limited set of API (e.g. only set(), load(),
> > xchg() and cmpxchg(), no add() or sub() etc).
> >
> > While the atomic mod is introduced, documentation is also added for
> > memory models and data races.
> >
> > Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
> > my responsiblity on the Rust atomic mod.
> >
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  MAINTAINERS                    |   4 +-
> >  rust/kernel/sync.rs            |   1 +
> >  rust/kernel/sync/atomic.rs     |  19 ++++
> >  rust/kernel/sync/atomic/ops.rs | 199 +++++++++++++++++++++++++++++++++
> >  4 files changed, 222 insertions(+), 1 deletion(-)
> >  create mode 100644 rust/kernel/sync/atomic.rs
> >  create mode 100644 rust/kernel/sync/atomic/ops.rs
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b77f4495dcf4..e09471027a63 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3635,7 +3635,7 @@ F:        drivers/input/touchscreen/atmel_mxt_ts.c
> >  ATOMIC INFRASTRUCTURE
> >  M:     Will Deacon <will@kernel.org>
> >  M:     Peter Zijlstra <peterz@infradead.org>
> > -R:     Boqun Feng <boqun.feng@gmail.com>
> > +M:     Boqun Feng <boqun.feng@gmail.com>
> >  R:     Mark Rutland <mark.rutland@arm.com>
> >  L:     linux-kernel@vger.kernel.org
> >  S:     Maintained
> > @@ -3644,6 +3644,8 @@ F:        arch/*/include/asm/atomic*.h
> >  F:     include/*/atomic*.h
> >  F:     include/linux/refcount.h
> >  F:     scripts/atomic/
> > +F:     rust/kernel/sync/atomic.rs
> > +F:     rust/kernel/sync/atomic/
> 
> This is why mod.rs files are superior :)
> 

;-) Not going to do anything right now, but let me think about this.

> > @@ -0,0 +1,19 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Atomic primitives.
> > +//!
> > +//! These primitives have the same semantics as their C counterparts: and the precise definitions of
> > +//! semantics can be found at [`LKMM`]. Note that Linux Kernel Memory (Consistency) Model is the
> > +//! only model for Rust code in kernel, and Rust's own atomics should be avoided.
> > +//!
> > +//! # Data races
> > +//!
> > +//! [`LKMM`] atomics have different rules regarding data races:
> > +//!
> > +//! - A normal read doesn't data-race with an atomic read.
> 
> This was fixed:
> https://github.com/rust-lang/rust/pull/128778
> 

Yeah, I was aware of that effort, and good to know it's finally merged.
Thanks!

This will be in 1.83, right? If so, we will still need the above until
we bump up the minimal rustc version to 1.83 or beyond. I will handle
this properly with the minimal rustc 1.83 (i.e. if this goes in first,
will send a follow up patch). I will also mention in the above that this
has been changed in 1.83.

This also reminds that I should add that LKMM allows mixed-size atomic
accesses (as non data race), I will add that in the version.

> > +mod private {
> > +    /// Sealed trait marker to disable customized impls on atomic implementation traits.
> > +    pub trait Sealed {}
> > +}
> 
> Just make the trait unsafe?
> 

And make the safety requirement of `AtomicImpl` something like:

    The type must have the implementation for atomic operations.

? Hmm.. I don't think that's a good safety requirement TBH. Actually the
reason that we need to restrict `AtomicImpl` types is more of an
iplementation issue (the implementation need to be done if we want to
support i8 or i16) rather than safety issue. So a sealed trait is proper
here. Does this make sense? Or am I missing something?

Regards,
Boqun

> Alice

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 04/13] rust: sync: atomic: Add generic atomics
  2024-12-12 10:57   ` Alice Ryhl
@ 2024-12-12 17:34     ` Boqun Feng
  2024-12-13 14:32       ` Alice Ryhl
  0 siblings, 1 reply; 28+ messages in thread
From: Boqun Feng @ 2024-12-12 17:34 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alan Stern,
	Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
	David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
	Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
	Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
	Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Catalin Marinas, torvalds,
	linux-arm-kernel, linux-fsdevel, Trevor Gross, dakr,
	Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett,
	Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
	Lai Jiangshan, Zqiang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-riscv

On Thu, Dec 12, 2024 at 11:57:07AM +0100, Alice Ryhl wrote:
[...]
> > diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
> > new file mode 100644
> > index 000000000000..204da38e2691
> > --- /dev/null
> > +++ b/rust/kernel/sync/atomic/generic.rs
> > @@ -0,0 +1,253 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Generic atomic primitives.
> > +
> > +use super::ops::*;
> > +use super::ordering::*;
> > +use crate::types::Opaque;
> > +
> > +/// A generic atomic variable.
> > +///
> > +/// `T` must impl [`AllowAtomic`], that is, an [`AtomicImpl`] has to be chosen.
> > +///
> > +/// # Invariants
> > +///
> > +/// Doing an atomic operation while holding a reference of [`Self`] won't cause a data race, this
> > +/// is guaranteed by the safety requirement of [`Self::from_ptr`] and the extra safety requirement
> > +/// of the usage on pointers returned by [`Self::as_ptr`].
> > +#[repr(transparent)]
> > +pub struct Atomic<T: AllowAtomic>(Opaque<T>);
> > +
> > +// SAFETY: `Atomic<T>` is safe to share among execution contexts because all accesses are atomic.
> > +unsafe impl<T: AllowAtomic> Sync for Atomic<T> {}
> 
> Surely it should also be Send?
> 

It's `Send` here because `Opaque<T>` is `Send` when `T` is `Send`. And
in patch #9, I changed the definition of `AllowAtomic`, which is not a
subtrait of `Send` anymore, and an `impl Send` block was added there.

> > +/// Atomics that support basic atomic operations.
> > +///
> > +/// TODO: Unless the `impl` is a `#[repr(transparet)]` new type of an existing [`AllowAtomic`], the
> > +/// impl block should be only done in atomic mod. And currently only basic integer types can
> > +/// implement this trait in atomic mod.
> 
> What's up with this TODO? Can't you just write an appropriate safety
> requirement?
> 

Because the limited scope of types that allows atomic is an artificial
choice, i.e. we want to start with a limited number of types and make
forward progress, and the types that we don't want to support atomics
for now are not because of safety reasons, but more of a lack of
users/motivations. So I don't think this is something we should use
safety requirement to describe.

> > +/// # Safety
> > +///
> > +/// [`Self`] must have the same size and alignment as [`Self::Repr`].
> > +pub unsafe trait AllowAtomic: Sized + Send + Copy {
> > +    /// The backing atomic implementation type.
> > +    type Repr: AtomicImpl;
> > +
> > +    /// Converts into a [`Self::Repr`].
> > +    fn into_repr(self) -> Self::Repr;
> > +
> > +    /// Converts from a [`Self::Repr`].
> > +    fn from_repr(repr: Self::Repr) -> Self;
> 
> What do you need these methods for?
> 

Converting a `AtomicImpl` value (currently only `i32` and `i64`) to a
`AllowAtomic` value without using transmute in `impl` block of
`Atomic<T>`. Any better idea?

Regards,
Boqun

> > +}
> > +
> > +// SAFETY: `T::Repr` is `Self` (i.e. `T`), so they have the same size and alignment.
> > +unsafe impl<T: AtomicImpl> AllowAtomic for T {
> > +    type Repr = Self;
> > +
> > +    fn into_repr(self) -> Self::Repr {
> > +        self
> > +    }
> > +
> > +    fn from_repr(repr: Self::Repr) -> Self {
> > +        repr
> > +    }
> > +}
> > +
> > +impl<T: AllowAtomic> Atomic<T> {
> > +    /// Creates a new atomic.
> > +    pub const fn new(v: T) -> Self {
> > +        Self(Opaque::new(v))
> > +    }
> > +
> > +    /// Creates a reference to [`Self`] from a pointer.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// - `ptr` has to be a valid pointer.
> > +    /// - `ptr` has to be valid for both reads and writes for the whole lifetime `'a`.
> > +    /// - For the whole lifetime of '`a`, other accesses to the object cannot cause data races
> > +    ///   (defined by [`LKMM`]) against atomic operations on the returned reference.
> > +    ///
> > +    /// [`LKMM`]: srctree/tools/memory-model
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// Using [`Atomic::from_ptr()`] combined with [`Atomic::load()`] or [`Atomic::store()`] can
> > +    /// achieve the same functionality as `READ_ONCE()`/`smp_load_acquire()` or
> > +    /// `WRITE_ONCE()`/`smp_store_release()` in C side:
> > +    ///
> > +    /// ```rust
> > +    /// # use kernel::types::Opaque;
> > +    /// use kernel::sync::atomic::{Atomic, Relaxed, Release};
> > +    ///
> > +    /// // Assume there is a C struct `Foo`.
> > +    /// mod cbindings {
> > +    ///     #[repr(C)]
> > +    ///     pub(crate) struct foo { pub(crate) a: i32, pub(crate) b: i32 }
> > +    /// }
> > +    ///
> > +    /// let tmp = Opaque::new(cbindings::foo { a: 1, b: 2});
> > +    ///
> > +    /// // struct foo *foo_ptr = ..;
> > +    /// let foo_ptr = tmp.get();
> > +    ///
> > +    /// // SAFETY: `foo_ptr` is a valid pointer, and `.a` is inbound.
> > +    /// let foo_a_ptr = unsafe { core::ptr::addr_of_mut!((*foo_ptr).a) };
> > +    ///
> > +    /// // a = READ_ONCE(foo_ptr->a);
> > +    /// //
> > +    /// // SAFETY: `foo_a_ptr` is a valid pointer for read, and all accesses on it is atomic, so no
> > +    /// // data race.
> > +    /// let a = unsafe { Atomic::from_ptr(foo_a_ptr) }.load(Relaxed);
> > +    /// # assert_eq!(a, 1);
> > +    ///
> > +    /// // smp_store_release(&foo_ptr->a, 2);
> > +    /// //
> > +    /// // SAFETY: `foo_a_ptr` is a valid pointer for write, and all accesses on it is atomic, so no
> > +    /// // data race.
> > +    /// unsafe { Atomic::from_ptr(foo_a_ptr) }.store(2, Release);
> > +    /// ```
> > +    ///
> > +    /// However, this should be only used when communicating with C side or manipulating a C struct.
> > +    pub unsafe fn from_ptr<'a>(ptr: *mut T) -> &'a Self
> > +    where
> > +        T: Sync,
> > +    {
> > +        // CAST: `T` is transparent to `Atomic<T>`.
> > +        // SAFETY: Per function safety requirement, `ptr` is a valid pointer and the object will
> > +        // live long enough. It's safe to return a `&Atomic<T>` because function safety requirement
> > +        // guarantees other accesses won't cause data races.
> > +        unsafe { &*ptr.cast::<Self>() }
> > +    }
> > +
> > +    /// Returns a pointer to the underlying atomic variable.
> > +    ///
> > +    /// Extra safety requirement on using the return pointer: the operations done via the pointer
> > +    /// cannot cause data races defined by [`LKMM`].
> > +    ///
> > +    /// [`LKMM`]: srctree/tools/memory-model
> > +    pub const fn as_ptr(&self) -> *mut T {
> > +        self.0.get()
> > +    }
> > +
> > +    /// Returns a mutable reference to the underlying atomic variable.
> > +    ///
> > +    /// This is safe because the mutable reference of the atomic variable guarantees the exclusive
> > +    /// access.
> > +    pub fn get_mut(&mut self) -> &mut T {
> > +        // SAFETY: `self.as_ptr()` is a valid pointer to `T`, and the object has already been
> > +        // initialized. `&mut self` guarantees the exclusive access, so it's safe to reborrow
> > +        // mutably.
> > +        unsafe { &mut *self.as_ptr() }
> > +    }
> > +}
> > +
> > +impl<T: AllowAtomic> Atomic<T>
> > +where
> > +    T::Repr: AtomicHasBasicOps,
> > +{
> > +    /// Loads the value from the atomic variable.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// Simple usages:
> > +    ///
> > +    /// ```rust
> > +    /// use kernel::sync::atomic::{Atomic, Relaxed};
> > +    ///
> > +    /// let x = Atomic::new(42i32);
> > +    ///
> > +    /// assert_eq!(42, x.load(Relaxed));
> > +    ///
> > +    /// let x = Atomic::new(42i64);
> > +    ///
> > +    /// assert_eq!(42, x.load(Relaxed));
> > +    /// ```
> > +    ///
> > +    /// Customized new types in [`Atomic`]:
> > +    ///
> > +    /// ```rust
> > +    /// use kernel::sync::atomic::{generic::AllowAtomic, Atomic, Relaxed};
> > +    ///
> > +    /// #[derive(Clone, Copy)]
> > +    /// #[repr(transparent)]
> > +    /// struct NewType(u32);
> > +    ///
> > +    /// // SAFETY: `NewType` is transparent to `u32`, which has the same size and alignment as
> > +    /// // `i32`.
> > +    /// unsafe impl AllowAtomic for NewType {
> > +    ///     type Repr = i32;
> > +    ///
> > +    ///     fn into_repr(self) -> Self::Repr {
> > +    ///         self.0 as i32
> > +    ///     }
> > +    ///
> > +    ///     fn from_repr(repr: Self::Repr) -> Self {
> > +    ///         NewType(repr as u32)
> > +    ///     }
> > +    /// }
> > +    ///
> > +    /// let n = Atomic::new(NewType(0));
> > +    ///
> > +    /// assert_eq!(0, n.load(Relaxed).0);
> > +    /// ```
> > +    #[inline(always)]
> > +    pub fn load<Ordering: AcquireOrRelaxed>(&self, _: Ordering) -> T {
> > +        let a = self.as_ptr().cast::<T::Repr>();
> > +
> > +        // SAFETY:
> > +        // - For calling the atomic_read*() function:
> > +        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
> > +        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
> > +        //   - per the type invariants, the following atomic operation won't cause data races.
> > +        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
> > +        //   - atomic operations are used here.
> > +        let v = unsafe {
> > +            if Ordering::IS_RELAXED {
> > +                T::Repr::atomic_read(a)
> > +            } else {
> > +                T::Repr::atomic_read_acquire(a)
> > +            }
> > +        };
> > +
> > +        T::from_repr(v)
> > +    }
> > +
> > +    /// Stores a value to the atomic variable.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```rust
> > +    /// use kernel::sync::atomic::{Atomic, Relaxed};
> > +    ///
> > +    /// let x = Atomic::new(42i32);
> > +    ///
> > +    /// assert_eq!(42, x.load(Relaxed));
> > +    ///
> > +    /// x.store(43, Relaxed);
> > +    ///
> > +    /// assert_eq!(43, x.load(Relaxed));
> > +    /// ```
> > +    ///
> > +    #[inline(always)]
> > +    pub fn store<Ordering: ReleaseOrRelaxed>(&self, v: T, _: Ordering) {
> > +        let v = T::into_repr(v);
> > +        let a = self.as_ptr().cast::<T::Repr>();
> > +
> > +        // SAFETY:
> > +        // - For calling the atomic_set*() function:
> > +        //   - `self.as_ptr()` is a valid pointer, and per the safety requirement of `AllocAtomic`,
> > +        //      a `*mut T` is a valid `*mut T::Repr`. Therefore `a` is a valid pointer,
> > +        //   - per the type invariants, the following atomic operation won't cause data races.
> > +        // - For extra safety requirement of usage on pointers returned by `self.as_ptr():
> > +        //   - atomic operations are used here.
> > +        unsafe {
> > +            if Ordering::IS_RELAXED {
> > +                T::Repr::atomic_set(a, v)
> > +            } else {
> > +                T::Repr::atomic_set_release(a, v)
> > +            }
> > +        };
> > +    }
> > +}
> > --
> > 2.45.2
> >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 04/13] rust: sync: atomic: Add generic atomics
  2024-12-12 17:34     ` Boqun Feng
@ 2024-12-13 14:32       ` Alice Ryhl
  2024-12-13 20:13         ` Boqun Feng
  0 siblings, 1 reply; 28+ messages in thread
From: Alice Ryhl @ 2024-12-13 14:32 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alan Stern,
	Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
	David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
	Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
	Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
	Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Catalin Marinas, torvalds,
	linux-arm-kernel, linux-fsdevel, Trevor Gross, dakr,
	Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett,
	Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
	Lai Jiangshan, Zqiang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-riscv

On Thu, Dec 12, 2024 at 6:34 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Thu, Dec 12, 2024 at 11:57:07AM +0100, Alice Ryhl wrote:
> [...]
> > > diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
> > > new file mode 100644
> > > index 000000000000..204da38e2691
> > > --- /dev/null
> > > +++ b/rust/kernel/sync/atomic/generic.rs
> > > @@ -0,0 +1,253 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +//! Generic atomic primitives.
> > > +
> > > +use super::ops::*;
> > > +use super::ordering::*;
> > > +use crate::types::Opaque;
> > > +
> > > +/// A generic atomic variable.
> > > +///
> > > +/// `T` must impl [`AllowAtomic`], that is, an [`AtomicImpl`] has to be chosen.
> > > +///
> > > +/// # Invariants
> > > +///
> > > +/// Doing an atomic operation while holding a reference of [`Self`] won't cause a data race, this
> > > +/// is guaranteed by the safety requirement of [`Self::from_ptr`] and the extra safety requirement
> > > +/// of the usage on pointers returned by [`Self::as_ptr`].
> > > +#[repr(transparent)]
> > > +pub struct Atomic<T: AllowAtomic>(Opaque<T>);
> > > +
> > > +// SAFETY: `Atomic<T>` is safe to share among execution contexts because all accesses are atomic.
> > > +unsafe impl<T: AllowAtomic> Sync for Atomic<T> {}
> >
> > Surely it should also be Send?
> >
>
> It's `Send` here because `Opaque<T>` is `Send` when `T` is `Send`. And
> in patch #9, I changed the definition of `AllowAtomic`, which is not a
> subtrait of `Send` anymore, and an `impl Send` block was added there.
>
> > > +/// Atomics that support basic atomic operations.
> > > +///
> > > +/// TODO: Unless the `impl` is a `#[repr(transparet)]` new type of an existing [`AllowAtomic`], the
> > > +/// impl block should be only done in atomic mod. And currently only basic integer types can
> > > +/// implement this trait in atomic mod.
> >
> > What's up with this TODO? Can't you just write an appropriate safety
> > requirement?
> >
>
> Because the limited scope of types that allows atomic is an artificial
> choice, i.e. we want to start with a limited number of types and make
> forward progress, and the types that we don't want to support atomics
> for now are not because of safety reasons, but more of a lack of
> users/motivations. So I don't think this is something we should use
> safety requirement to describe.

I found the wording very confusing. Could you reword it to say
something about future possibilities?

> > > +/// # Safety
> > > +///
> > > +/// [`Self`] must have the same size and alignment as [`Self::Repr`].
> > > +pub unsafe trait AllowAtomic: Sized + Send + Copy {
> > > +    /// The backing atomic implementation type.
> > > +    type Repr: AtomicImpl;
> > > +
> > > +    /// Converts into a [`Self::Repr`].
> > > +    fn into_repr(self) -> Self::Repr;
> > > +
> > > +    /// Converts from a [`Self::Repr`].
> > > +    fn from_repr(repr: Self::Repr) -> Self;
> >
> > What do you need these methods for?
> >
>
> Converting a `AtomicImpl` value (currently only `i32` and `i64`) to a
> `AllowAtomic` value without using transmute in `impl` block of
> `Atomic<T>`. Any better idea?

You could use transmute?

Alice

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework
  2024-12-12 17:07     ` Boqun Feng
@ 2024-12-13 14:37       ` Alice Ryhl
  2024-12-13 20:28         ` Boqun Feng
  0 siblings, 1 reply; 28+ messages in thread
From: Alice Ryhl @ 2024-12-13 14:37 UTC (permalink / raw)
  To: Boqun Feng
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alan Stern,
	Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
	David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
	Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
	Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
	Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Catalin Marinas, torvalds,
	linux-arm-kernel, linux-fsdevel, Trevor Gross, dakr,
	Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett,
	Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
	Lai Jiangshan, Zqiang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-riscv

On Thu, Dec 12, 2024 at 6:07 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Thu, Dec 12, 2024 at 11:51:23AM +0100, Alice Ryhl wrote:
> > On Fri, Nov 1, 2024 at 7:03 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > Preparation for generic atomic implementation. To unify the
> > > ipmlementation of a generic method over `i32` and `i64`, the C side
> > > atomic methods need to be grouped so that in a generic method, they can
> > > be referred as <type>::<method>, otherwise their parameters and return
> > > value are different between `i32` and `i64`, which would require using
> > > `transmute()` to unify the type into a `T`.
> > >
> > > Introduce `AtomicIpml` to represent a basic type in Rust that has the
> > > direct mapping to an atomic implementation from C. This trait is sealed,
> > > and currently only `i32` and `i64` ipml this.
> >
> > There seems to be quite a few instances of "impl" spelled as "ipml" here.
> >
>
> Will fix!
>
> > > Further, different methods are put into different `*Ops` trait groups,
> > > and this is for the future when smaller types like `i8`/`i16` are
> > > supported but only with a limited set of API (e.g. only set(), load(),
> > > xchg() and cmpxchg(), no add() or sub() etc).
> > >
> > > While the atomic mod is introduced, documentation is also added for
> > > memory models and data races.
> > >
> > > Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
> > > my responsiblity on the Rust atomic mod.
> > >
> > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > > ---
> > >  MAINTAINERS                    |   4 +-
> > >  rust/kernel/sync.rs            |   1 +
> > >  rust/kernel/sync/atomic.rs     |  19 ++++
> > >  rust/kernel/sync/atomic/ops.rs | 199 +++++++++++++++++++++++++++++++++
> > >  4 files changed, 222 insertions(+), 1 deletion(-)
> > >  create mode 100644 rust/kernel/sync/atomic.rs
> > >  create mode 100644 rust/kernel/sync/atomic/ops.rs
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index b77f4495dcf4..e09471027a63 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -3635,7 +3635,7 @@ F:        drivers/input/touchscreen/atmel_mxt_ts.c
> > >  ATOMIC INFRASTRUCTURE
> > >  M:     Will Deacon <will@kernel.org>
> > >  M:     Peter Zijlstra <peterz@infradead.org>
> > > -R:     Boqun Feng <boqun.feng@gmail.com>
> > > +M:     Boqun Feng <boqun.feng@gmail.com>
> > >  R:     Mark Rutland <mark.rutland@arm.com>
> > >  L:     linux-kernel@vger.kernel.org
> > >  S:     Maintained
> > > @@ -3644,6 +3644,8 @@ F:        arch/*/include/asm/atomic*.h
> > >  F:     include/*/atomic*.h
> > >  F:     include/linux/refcount.h
> > >  F:     scripts/atomic/
> > > +F:     rust/kernel/sync/atomic.rs
> > > +F:     rust/kernel/sync/atomic/
> >
> > This is why mod.rs files are superior :)
> >
>
> ;-) Not going to do anything right now, but let me think about this.
>
> > > @@ -0,0 +1,19 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +//! Atomic primitives.
> > > +//!
> > > +//! These primitives have the same semantics as their C counterparts: and the precise definitions of
> > > +//! semantics can be found at [`LKMM`]. Note that Linux Kernel Memory (Consistency) Model is the
> > > +//! only model for Rust code in kernel, and Rust's own atomics should be avoided.
> > > +//!
> > > +//! # Data races
> > > +//!
> > > +//! [`LKMM`] atomics have different rules regarding data races:
> > > +//!
> > > +//! - A normal read doesn't data-race with an atomic read.
> >
> > This was fixed:
> > https://github.com/rust-lang/rust/pull/128778
> >
>
> Yeah, I was aware of that effort, and good to know it's finally merged.
> Thanks!
>
> This will be in 1.83, right? If so, we will still need the above until
> we bump up the minimal rustc version to 1.83 or beyond. I will handle
> this properly with the minimal rustc 1.83 (i.e. if this goes in first,
> will send a follow up patch). I will also mention in the above that this
> has been changed in 1.83.
>
> This also reminds that I should add that LKMM allows mixed-size atomic
> accesses (as non data race), I will add that in the version.

This is just documentation. I don't think you need to do any special
MSRV handling.

> > > +mod private {
> > > +    /// Sealed trait marker to disable customized impls on atomic implementation traits.
> > > +    pub trait Sealed {}
> > > +}
> >
> > Just make the trait unsafe?
> >
>
> And make the safety requirement of `AtomicImpl` something like:
>
>     The type must have the implementation for atomic operations.
>
> ? Hmm.. I don't think that's a good safety requirement TBH. Actually the
> reason that we need to restrict `AtomicImpl` types is more of an
> iplementation issue (the implementation need to be done if we want to
> support i8 or i16) rather than safety issue. So a sealed trait is proper
> here. Does this make sense? Or am I missing something?

Where is the AtomicImpl trait used?

Alice

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 04/13] rust: sync: atomic: Add generic atomics
  2024-12-13 14:32       ` Alice Ryhl
@ 2024-12-13 20:13         ` Boqun Feng
  0 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-12-13 20:13 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alan Stern,
	Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
	David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
	Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
	Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
	Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Catalin Marinas, torvalds,
	linux-arm-kernel, linux-fsdevel, Trevor Gross, dakr,
	Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett,
	Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
	Lai Jiangshan, Zqiang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-riscv

On Fri, Dec 13, 2024 at 03:32:47PM +0100, Alice Ryhl wrote:
> On Thu, Dec 12, 2024 at 6:34 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Thu, Dec 12, 2024 at 11:57:07AM +0100, Alice Ryhl wrote:
> > [...]
> > > > diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs
> > > > new file mode 100644
> > > > index 000000000000..204da38e2691
> > > > --- /dev/null
> > > > +++ b/rust/kernel/sync/atomic/generic.rs
> > > > @@ -0,0 +1,253 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +//! Generic atomic primitives.
> > > > +
> > > > +use super::ops::*;
> > > > +use super::ordering::*;
> > > > +use crate::types::Opaque;
> > > > +
> > > > +/// A generic atomic variable.
> > > > +///
> > > > +/// `T` must impl [`AllowAtomic`], that is, an [`AtomicImpl`] has to be chosen.
> > > > +///
> > > > +/// # Invariants
> > > > +///
> > > > +/// Doing an atomic operation while holding a reference of [`Self`] won't cause a data race, this
> > > > +/// is guaranteed by the safety requirement of [`Self::from_ptr`] and the extra safety requirement
> > > > +/// of the usage on pointers returned by [`Self::as_ptr`].
> > > > +#[repr(transparent)]
> > > > +pub struct Atomic<T: AllowAtomic>(Opaque<T>);
> > > > +
> > > > +// SAFETY: `Atomic<T>` is safe to share among execution contexts because all accesses are atomic.
> > > > +unsafe impl<T: AllowAtomic> Sync for Atomic<T> {}
> > >
> > > Surely it should also be Send?
> > >
> >
> > It's `Send` here because `Opaque<T>` is `Send` when `T` is `Send`. And
> > in patch #9, I changed the definition of `AllowAtomic`, which is not a
> > subtrait of `Send` anymore, and an `impl Send` block was added there.
> >
> > > > +/// Atomics that support basic atomic operations.
> > > > +///
> > > > +/// TODO: Unless the `impl` is a `#[repr(transparet)]` new type of an existing [`AllowAtomic`], the
> > > > +/// impl block should be only done in atomic mod. And currently only basic integer types can
> > > > +/// implement this trait in atomic mod.
> > >
> > > What's up with this TODO? Can't you just write an appropriate safety
> > > requirement?
> > >
> >
> > Because the limited scope of types that allows atomic is an artificial
> > choice, i.e. we want to start with a limited number of types and make
> > forward progress, and the types that we don't want to support atomics
> > for now are not because of safety reasons, but more of a lack of
> > users/motivations. So I don't think this is something we should use
> > safety requirement to describe.
> 
> I found the wording very confusing. Could you reword it to say
> something about future possibilities?
> 

Sure, how about:

/// TODO: Currently the [`AllowAtomic`] types are restricted within
/// basic integer types (and their transparent new types). In the
/// future, we could extend the scope to more data types when there is a
/// clear and meaningful usage, but for now, [`AllowAtomic`] should only
/// be implemented inside atomic mod for the restricted types mentioned
/// above.

?

> > > > +/// # Safety
> > > > +///
> > > > +/// [`Self`] must have the same size and alignment as [`Self::Repr`].
> > > > +pub unsafe trait AllowAtomic: Sized + Send + Copy {
> > > > +    /// The backing atomic implementation type.
> > > > +    type Repr: AtomicImpl;
> > > > +
> > > > +    /// Converts into a [`Self::Repr`].
> > > > +    fn into_repr(self) -> Self::Repr;
> > > > +
> > > > +    /// Converts from a [`Self::Repr`].
> > > > +    fn from_repr(repr: Self::Repr) -> Self;
> > >
> > > What do you need these methods for?
> > >
> >
> > Converting a `AtomicImpl` value (currently only `i32` and `i64`) to a
> > `AllowAtomic` value without using transmute in `impl` block of
> > `Atomic<T>`. Any better idea?
> 
> You could use transmute?
> 

In a draft version, I did use transmute, but Benno commented that he
wanted to avoid arbitrary transmute as hard as possible (if I didn't
misunderstand him). Hence these two functions are provided. Now think
about it, I don't think doing either way (transmute or *_repr()
function) would affect most of users, since most of users won't need to 
impl `AllowAtomic` themselves, therefore I think keeping it as it is is
fine. Do you have any user observable concern of defining these
functions?

Regards,
Boqun

> Alice

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework
  2024-12-13 14:37       ` Alice Ryhl
@ 2024-12-13 20:28         ` Boqun Feng
  0 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2024-12-13 20:28 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alan Stern,
	Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
	David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
	Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
	Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
	Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Catalin Marinas, torvalds,
	linux-arm-kernel, linux-fsdevel, Trevor Gross, dakr,
	Frederic Weisbecker, Neeraj Upadhyay, Josh Triplett,
	Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
	Lai Jiangshan, Zqiang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	linux-riscv

On Fri, Dec 13, 2024 at 03:37:13PM +0100, Alice Ryhl wrote:
[...]
> > > > @@ -0,0 +1,19 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +//! Atomic primitives.
> > > > +//!
> > > > +//! These primitives have the same semantics as their C counterparts: and the precise definitions of
> > > > +//! semantics can be found at [`LKMM`]. Note that Linux Kernel Memory (Consistency) Model is the
> > > > +//! only model for Rust code in kernel, and Rust's own atomics should be avoided.
> > > > +//!
> > > > +//! # Data races
> > > > +//!
> > > > +//! [`LKMM`] atomics have different rules regarding data races:
> > > > +//!
> > > > +//! - A normal read doesn't data-race with an atomic read.
> > >
> > > This was fixed:
> > > https://github.com/rust-lang/rust/pull/128778
> > >
> >
> > Yeah, I was aware of that effort, and good to know it's finally merged.
> > Thanks!
> >
> > This will be in 1.83, right? If so, we will still need the above until
> > we bump up the minimal rustc version to 1.83 or beyond. I will handle
> > this properly with the minimal rustc 1.83 (i.e. if this goes in first,
> > will send a follow up patch). I will also mention in the above that this
> > has been changed in 1.83.
> >
> > This also reminds that I should add that LKMM allows mixed-size atomic
> > accesses (as non data race), I will add that in the version.
> 
> This is just documentation. I don't think you need to do any special

The PR also contained miri changes, so the same code will be reported
differently by miri. That was what I was thinking of. However, now think
about it, we are not going to use Rust atomics, so this difference
shouldn't affect us. Therefore I agree, I will drop this.

> MSRV handling.
> 
> > > > +mod private {
> > > > +    /// Sealed trait marker to disable customized impls on atomic implementation traits.
> > > > +    pub trait Sealed {}
> > > > +}
> > >
> > > Just make the trait unsafe?
> > >
> >
> > And make the safety requirement of `AtomicImpl` something like:
> >
> >     The type must have the implementation for atomic operations.
> >
> > ? Hmm.. I don't think that's a good safety requirement TBH. Actually the
> > reason that we need to restrict `AtomicImpl` types is more of an
> > iplementation issue (the implementation need to be done if we want to
> > support i8 or i16) rather than safety issue. So a sealed trait is proper
> > here. Does this make sense? Or am I missing something?
> 
> Where is the AtomicImpl trait used?
> 

It's used when `impl`ing an `AllowAtomic` type, `AllowAtomic` has an
associate type named `Repr` which must be an `AtomicImpl`, i.e. each
type that has atomic operation support must select the underlying
implementation types (currently we only have i32 and i64 from C side
APIs). Using a sealed trait is appropriate in this case, because unless
you are adding atomic support for different sizes of data, you shouldn't
impl `AtomicImpl`.

Regards,
Boqun

> Alice

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC v2 00/13] LKMM *generic* atomics in Rust
  2024-11-02  7:35 ` David Gow
@ 2025-04-21 16:27   ` Boqun Feng
  0 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2025-04-21 16:27 UTC (permalink / raw)
  To: David Gow
  Cc: rust-for-linux, rcu, linux-kernel, linux-arch, llvm, lkmm,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
	Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
	Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
	Nathan Chancellor, Nick Desaulniers, kent.overstreet,
	Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
	Catalin Marinas, torvalds, linux-arm-kernel, linux-fsdevel,
	Trevor Gross, dakr, Frederic Weisbecker, Neeraj Upadhyay,
	Josh Triplett, Uladzislau Rezki, Steven Rostedt,
	Mathieu Desnoyers, Lai Jiangshan, Zqiang, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, linux-riscv

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



^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2025-04-21 16:27 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-01  6:02 [RFC v2 00/13] LKMM *generic* atomics in Rust Boqun Feng
2024-11-01  6:02 ` [RFC v2 01/13] rust: Introduce atomic API helpers Boqun Feng
2024-11-01  6:02 ` [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework Boqun Feng
2024-12-12 10:51   ` Alice Ryhl
2024-12-12 17:07     ` Boqun Feng
2024-12-13 14:37       ` Alice Ryhl
2024-12-13 20:28         ` Boqun Feng
2024-11-01  6:02 ` [RFC v2 03/13] rust: sync: atomic: Add ordering annotation types Boqun Feng
2024-11-01  6:02 ` [RFC v2 04/13] rust: sync: atomic: Add generic atomics Boqun Feng
2024-12-12 10:57   ` Alice Ryhl
2024-12-12 17:34     ` Boqun Feng
2024-12-13 14:32       ` Alice Ryhl
2024-12-13 20:13         ` Boqun Feng
2024-11-01  6:02 ` [RFC v2 05/13] rust: sync: atomic: Add atomic {cmp,}xchg operations Boqun Feng
2024-11-01  6:02 ` [RFC v2 06/13] rust: sync: atomic: Add the framework of arithmetic operations Boqun Feng
2024-11-01  6:02 ` [RFC v2 07/13] rust: sync: atomic: Add Atomic<u{32,64}> Boqun Feng
2024-11-01  6:02 ` [RFC v2 08/13] rust: sync: atomic: Add Atomic<{usize,isize}> Boqun Feng
2024-11-01  6:02 ` [RFC v2 09/13] rust: sync: atomic: Add Atomic<*mut T> Boqun Feng
2024-11-01  6:02 ` [RFC v2 10/13] rust: sync: atomic: Add arithmetic ops for " Boqun Feng
2024-11-01  6:02 ` [RFC v2 11/13] rust: sync: Add memory barriers Boqun Feng
2024-11-01  6:55   ` David Gow
2024-11-01  7:04     ` Boqun Feng
2024-11-01  7:01   ` [RFC v2.1 " Boqun Feng
2024-11-01  6:02 ` [RFC v2 12/13] rust: add rcu abstraction Boqun Feng
2024-11-01  6:02 ` [RFC v2 13/13] rust: sync: rcu: Add RCU protected pointer Boqun Feng
2024-11-01 14:30 ` [RFC v2 00/13] LKMM *generic* atomics in Rust Miguel Ojeda
2024-11-02  7:35 ` David Gow
2025-04-21 16:27   ` Boqun Feng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).