[RFC 0/2] Initial LKMM atomics support in Rust

[RFC 0/2] Initial LKMM atomics support in Rust

[Boqun Feng]

Hi,

This is a follow-up of [1]. Thanks for all the inputs from that thread.
I use Mark's outline atomic scripts, but make them specific for atomics
in Rust. The reason is that I want to use Gary's work [2], and inline
atomics if possible in Rust code. My local test can confirm it works:

With RUST_LTO_HELPERS=n

  224edc:       52800180        mov     w0, #0xc                        // #12
  224ee0:       94000000        bl      219640 <rust_helper_atomic_fetch_add_relaxed>

With RUST_LTO_HELPERS=y

  222fd4:       52800189        mov     w9, #0xc                        // #12
  222fd8:       b8290108        ldadd   w9, w8, [x8]


Only AtomicI32 (atomic_t) and AtomicI64 (atomic64_t) are added, and for
AtomicPtr (atomic pointers) I plan to implement with compile-time
selection on either of these two.

You can find a branch contains this series and Gray's patchset at:

	https://github.com/fbq/linux.git dev/rust/atomic-rfc


For testing, I randomly picked up some function and inspected the
generated code, plus Rust code can use the function document as tests,
so I added two tests there. Ideally we should have something similar to
lib/atomic64_test.c, but I want to get some feedback on the
implementation part first, plus it's just using C functions, so as long
as C code passes the tests, it should be fine (famous last words).

ARM64 maintainers, I use the following to simulate cases where LSE is
configured but not available from hardware:

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 48e7029f1054..99e6e2b2867f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1601,6 +1601,8 @@ static bool
 has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
 {
        u64 val = read_scoped_sysreg(entry, scope);
+       if (entry->capability == ARM64_HAS_LSE_ATOMICS)
+               return false;
        return feature_matches(val, entry);
 }

and my tests in a qemu emulated VM passed for both RUST_LTO_HELPERS=n
and =y cases. Let me know what I can also do to test this.


Notes for people who are working on Rust code and need atomics, my
target of this set of APIs is 6.11 or 6.12 (will work hard on it, but no
guarantee ;-)). If you are currently using Rust own atomics, it's OK, we
can always clean up quickly after this merged.

Regards,
Boqun

[1]: https://lore.kernel.org/rust-for-linux/20240322233838.868874-1-boqun.feng@gmail.com/
[2]: https://lore.kernel.org/rust-for-linux/20240529202817.3641974-1-gary@garyguo.net/

Boqun Feng (2):
  rust: Introduce atomic API helpers
  rust: sync: Add atomic support

 MAINTAINERS                               |    4 +-
 arch/arm64/kernel/cpufeature.c            |    2 +
 rust/atomic_helpers.h                     | 1035 ++++++++++++++++
 rust/helpers.c                            |    2 +
 rust/kernel/sync.rs                       |    1 +
 rust/kernel/sync/atomic.rs                |   63 +
 rust/kernel/sync/atomic/impl.rs           | 1375 +++++++++++++++++++++
 scripts/atomic/gen-atomics.sh             |    2 +
 scripts/atomic/gen-rust-atomic-helpers.sh |   64 +
 scripts/atomic/gen-rust-atomic.sh         |  136 ++
 10 files changed, 2683 insertions(+), 1 deletion(-)
 create mode 100644 rust/atomic_helpers.h
 create mode 100644 rust/kernel/sync/atomic.rs
 create mode 100644 rust/kernel/sync/atomic/impl.rs
 create mode 100755 scripts/atomic/gen-rust-atomic-helpers.sh
 create mode 100755 scripts/atomic/gen-rust-atomic.sh

-- 
2.45.2

[RFC 1/2] rust: Introduce atomic API helpers

[Boqun Feng]

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/atomic_helpers.h                     | 1035 +++++++++++++++++++++
 rust/helpers.c                            |    2 +
 scripts/atomic/gen-atomics.sh             |    1 +
 scripts/atomic/gen-rust-atomic-helpers.sh |   64 ++
 4 files changed, 1102 insertions(+)
 create mode 100644 rust/atomic_helpers.h
 create mode 100755 scripts/atomic/gen-rust-atomic-helpers.sh

diff --git a/rust/atomic_helpers.h b/rust/atomic_helpers.h
new file mode 100644
index 000000000000..4b24eceef5fc
--- /dev/null
+++ b/rust/atomic_helpers.h
@@ -0,0 +1,1035 @@
+// 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>
+
+__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 */
+// e4edb6174dd42a265284958f00a7cea7ddb464b1
diff --git a/rust/helpers.c b/rust/helpers.c
index 3abf96f14148..2da644877a29 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -34,6 +34,8 @@
 #include <linux/workqueue.h>
 #include "helpers.h"
 
+#include "atomic_helpers.h"
+
 __rust_helper __noreturn void rust_helper_BUG(void)
 {
 	BUG();
diff --git a/scripts/atomic/gen-atomics.sh b/scripts/atomic/gen-atomics.sh
index 5b98a8307693..3927aee034c8 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/atomic_helpers.h
 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..5ef68017dd89
--- /dev/null
+++ b/scripts/atomic/gen-rust-atomic-helpers.sh
@@ -0,0 +1,64 @@
+#!/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, raw, 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 raw="$1"; shift
+	local attrs="${raw:+noinstr }"
+
+	local atomicname="${raw}${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 ${attrs}${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>
+
+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

Re: [RFC 1/2] rust: Introduce atomic API helpers

[Greg Kroah-Hartman]

On Wed, Jun 12, 2024 at 03:30:24PM -0700, Boqun Feng wrote:
> +// Generated by scripts/atomic/gen-rust-atomic-helpers.sh
> +// DO NOT MODIFY THIS FILE DIRECTLY

Why not just build this at build time and not check the file into the
tree if it is always automatically generated?  That way it never gets
out of sync.  We do this for other types of auto-generated files in the
kernel today already.

thanks,

greg k-h

Re: [RFC 1/2] rust: Introduce atomic API helpers

[Peter Zijlstra]

On Thu, Jun 13, 2024 at 07:38:51AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jun 12, 2024 at 03:30:24PM -0700, Boqun Feng wrote:
> > +// Generated by scripts/atomic/gen-rust-atomic-helpers.sh
> > +// DO NOT MODIFY THIS FILE DIRECTLY
> 
> Why not just build this at build time and not check the file into the
> tree if it is always automatically generated?  That way it never gets
> out of sync.  We do this for other types of auto-generated files in the
> kernel today already.

Part of the problem is, is that a *TON* of files depend on the atomic.h
headers. If we'd generate it on every build, you'd basically get to
rebuild the whole kernel every single time.

Also, these files don't change too often. And if you look, there's a
hash in those files which is used to check if things somehow got stale.

Re: [RFC 1/2] rust: Introduce atomic API helpers

[Greg Kroah-Hartman]

On Thu, Jun 13, 2024 at 11:17:47AM +0200, Peter Zijlstra wrote:
> On Thu, Jun 13, 2024 at 07:38:51AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jun 12, 2024 at 03:30:24PM -0700, Boqun Feng wrote:
> > > +// Generated by scripts/atomic/gen-rust-atomic-helpers.sh
> > > +// DO NOT MODIFY THIS FILE DIRECTLY
> > 
> > Why not just build this at build time and not check the file into the
> > tree if it is always automatically generated?  That way it never gets
> > out of sync.  We do this for other types of auto-generated files in the
> > kernel today already.
> 
> Part of the problem is, is that a *TON* of files depend on the atomic.h
> headers. If we'd generate it on every build, you'd basically get to
> rebuild the whole kernel every single time.
> 
> Also, these files don't change too often. And if you look, there's a
> hash in those files which is used to check if things somehow got stale.

Ok, fair enough, if you all don't think this will change often...

Re: [RFC 1/2] rust: Introduce atomic API helpers

[Mark Rutland]

On Thu, Jun 13, 2024 at 11:17:47AM +0200, Peter Zijlstra wrote:
> On Thu, Jun 13, 2024 at 07:38:51AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jun 12, 2024 at 03:30:24PM -0700, Boqun Feng wrote:
> > > +// Generated by scripts/atomic/gen-rust-atomic-helpers.sh
> > > +// DO NOT MODIFY THIS FILE DIRECTLY
> > 
> > Why not just build this at build time and not check the file into the
> > tree if it is always automatically generated?  That way it never gets
> > out of sync.  We do this for other types of auto-generated files in the
> > kernel today already.
> 
> Part of the problem is, is that a *TON* of files depend on the atomic.h
> headers. If we'd generate it on every build, you'd basically get to
> rebuild the whole kernel every single time.
> 
> Also, these files don't change too often. And if you look, there's a
> hash in those files which is used to check if things somehow got stale.

That and:

1) The generation is currently slow (multiple seconds), which gets
   people angry if they have to wait for it for a clean build. Improving
   that would require changing the way we generate the headers (e.g.
   rewrite that in something else to avoid the subshell fork problem).

   Last I looked there wasn't an obvious better way to do this, because
   nice options ended up pulling in more build-time dependencies.

2) Linus wanted git grep to work, which meant checking in the generated
   files:

   https://lore.kernel.org/all/CA+55aFxjU0op8QLLu0n-RjHBs7gQsLvD8jcyedgw6jeZFN7B+Q@mail.gmail.com/

Mark.

Re: [RFC 1/2] rust: Introduce atomic API helpers

[Mark Rutland]

On Wed, Jun 12, 2024 at 03:30:24PM -0700, Boqun Feng wrote:
> 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>

FWIW, I'm happy with the concept; I have a couple of minor comments
below.

> ---
>  rust/atomic_helpers.h                     | 1035 +++++++++++++++++++++
>  rust/helpers.c                            |    2 +
>  scripts/atomic/gen-atomics.sh             |    1 +
>  scripts/atomic/gen-rust-atomic-helpers.sh |   64 ++
>  4 files changed, 1102 insertions(+)
>  create mode 100644 rust/atomic_helpers.h
>  create mode 100755 scripts/atomic/gen-rust-atomic-helpers.sh

[...]

> +#gen_proto_order_variant(meta, pfx, name, sfx, order, atomic, int, raw, 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 raw="$1"; shift
> +	local attrs="${raw:+noinstr }"

You removed the 'raw_' atomic generation below, so you can drop the
'raw' parameter and the 'attrs' variable (both here and in the
template)...

> +	local atomicname="${raw}${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 ${attrs}${ret}

... e.g. you can remove '${attrs}' here.

[...]

> +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

With the 'raw' parameter removed above, the '""' argument can be
dropped.

Any reason to not have the atomic_long_*() API? It seems like an odd
ommision.

Mark.

Re: [RFC 1/2] rust: Introduce atomic API helpers

[Boqun Feng]

On Fri, Jun 14, 2024 at 11:31:33AM +0100, Mark Rutland wrote:
> On Wed, Jun 12, 2024 at 03:30:24PM -0700, Boqun Feng wrote:
> > 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>
> 
> FWIW, I'm happy with the concept; I have a couple of minor comments

;-)

> below.
> 
> > ---
> >  rust/atomic_helpers.h                     | 1035 +++++++++++++++++++++
> >  rust/helpers.c                            |    2 +
> >  scripts/atomic/gen-atomics.sh             |    1 +
> >  scripts/atomic/gen-rust-atomic-helpers.sh |   64 ++
> >  4 files changed, 1102 insertions(+)
> >  create mode 100644 rust/atomic_helpers.h
> >  create mode 100755 scripts/atomic/gen-rust-atomic-helpers.sh
> 
> [...]
> 
> > +#gen_proto_order_variant(meta, pfx, name, sfx, order, atomic, int, raw, 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 raw="$1"; shift
> > +	local attrs="${raw:+noinstr }"
> 
> You removed the 'raw_' atomic generation below, so you can drop the
> 'raw' parameter and the 'attrs' variable (both here and in the
> template)...
> 
> > +	local atomicname="${raw}${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 ${attrs}${ret}
> 
> ... e.g. you can remove '${attrs}' here.
> 
> [...]
> 
> > +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
> 
> With the 'raw' parameter removed above, the '""' argument can be
> dropped.
> 

Fix all above locally.

> Any reason to not have the atomic_long_*() API? It seems like an odd
> ommision.
> 

See my reply to Peter, but there's also a more technical reason: right
now, we use core::ffi::c_long for bindgen to translate C's long. But
instead of `isize` (Rust's version of pointer-sized integer)
core::ffi::c_long is `i64` on 64bit and `i32` on 32bit. So right now,
atomic_long_add_return()'s helper signature would be (on 64bit):

	extern "C" {
	    #[link_name="rust_helper_atomic_long_add_return"]
	    pub fn atomic_long_add_return(
		i: i64,
		v: *mut atomic_long_t,
	    ) -> i64;
	}

and I would need to cast the types in order to put it in an
`AtomicIsize` method:

	impl AtomicIsize {
	    pub fn add_return(&self, i: isize) -> isize {
	        unsafe {
		    return atomic_long_add_return(i as i64, self.0.get()) as isize
		}
	    }
	}

see these two `as`s. I want to avoid handling them in a bash script ;-)

A better solution would be what Gary has:

	https://github.com/nbdd0121/linux/commit/b604a43db56f149a90084fa8aed7988a8066894b	

, which defines kernel's own ffi types and teach bindgen to pick the
right type for c_long. If we prefer script code generating to Rust macro
code generating, I will work with Gary on getting that done first and
then add atomic_long_t support unless we feel atomic_long_t support is
urgent.

Regards,
Boqun

> Mark.

[RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

Provide two atomic types: AtomicI32 and AtomicI64 with the existing
implemenation of C atomics. These atomics have the same semantics of the
corresponding LKMM C atomics, and using one memory (ordering) model
certainly reduces the reasoning difficulty and potential bugs from the
interaction of two different memory models.

Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
my responsiblity on these Rust APIs.

Note that `Atomic*::new()`s are implemented vi open coding on struct
atomic*_t. This allows `new()` being a `const` function, so that it can
be used in constant contexts.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 MAINTAINERS                       |    4 +-
 arch/arm64/kernel/cpufeature.c    |    2 +
 rust/kernel/sync.rs               |    1 +
 rust/kernel/sync/atomic.rs        |   63 ++
 rust/kernel/sync/atomic/impl.rs   | 1375 +++++++++++++++++++++++++++++
 scripts/atomic/gen-atomics.sh     |    1 +
 scripts/atomic/gen-rust-atomic.sh |  136 +++
 7 files changed, 1581 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/sync/atomic.rs
 create mode 100644 rust/kernel/sync/atomic/impl.rs
 create mode 100755 scripts/atomic/gen-rust-atomic.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index d6c90161c7bf..a8528d27b260 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3458,7 +3458,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
@@ -3467,6 +3467,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/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 48e7029f1054..99e6e2b2867f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1601,6 +1601,8 @@ static bool
 has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
 {
 	u64 val = read_scoped_sysreg(entry, scope);
+	if (entry->capability == ARM64_HAS_LSE_ATOMICS)
+		return false;
 	return feature_matches(val, entry);
 }
 
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..b0f852cf1741
--- /dev/null
+++ b/rust/kernel/sync/atomic.rs
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Atomic primitives.
+//!
+//! These primitives have the same semantics as their C counterparts, for precise definitions of
+//! the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory (Consistency)
+//! Model is the only model for Rust development in kernel right now, please avoid to use Rust's
+//! own atomics.
+
+use crate::bindings::{atomic64_t, atomic_t};
+use crate::types::Opaque;
+
+mod r#impl;
+
+/// Atomic 32bit signed integers.
+pub struct AtomicI32(Opaque<atomic_t>);
+
+/// Atomic 64bit signed integers.
+pub struct AtomicI64(Opaque<atomic64_t>);
+
+impl AtomicI32 {
+    /// Creates an atomic variable with initial value `v`.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # use kernel::sync::atomic::*;
+    ///
+    /// let x = AtomicI32::new(0);
+    ///
+    /// assert_eq!(x.read(), 0);
+    /// assert_eq!(x.fetch_add_relaxed(12), 0);
+    /// assert_eq!(x.read(), 12);
+    /// ```
+    pub const fn new(v: i32) -> Self {
+        AtomicI32(Opaque::new(atomic_t { counter: v }))
+    }
+}
+
+// SAFETY: `AtomicI32` is safe to share among execution contexts because all accesses are atomic.
+unsafe impl Sync for AtomicI32 {}
+
+impl AtomicI64 {
+    /// Creates an atomic variable with initial value `v`.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # use kernel::sync::atomic::*;
+    ///
+    /// let x = AtomicI64::new(0);
+    ///
+    /// assert_eq!(x.read(), 0);
+    /// assert_eq!(x.fetch_add_relaxed(12), 0);
+    /// assert_eq!(x.read(), 12);
+    /// ```
+    pub const fn new(v: i64) -> Self {
+        AtomicI64(Opaque::new(atomic64_t { counter: v }))
+    }
+}
+
+// SAFETY: `AtomicI64` is safe to share among execution contexts because all accesses are atomic.
+unsafe impl Sync for AtomicI64 {}
diff --git a/rust/kernel/sync/atomic/impl.rs b/rust/kernel/sync/atomic/impl.rs
new file mode 100644
index 000000000000..1bbb0a714834
--- /dev/null
+++ b/rust/kernel/sync/atomic/impl.rs
@@ -0,0 +1,1375 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Generated by scripts/atomic/gen-rust-atomic.sh
+//! DO NOT MODIFY THIS FILE DIRECTLY
+
+use super::*;
+use crate::bindings::*;
+
+impl AtomicI32 {
+    /// See `atomic_read`.
+    #[inline(always)]
+    pub fn read(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_read(self.0.get());
+        }
+    }
+    /// See `atomic_read_acquire`.
+    #[inline(always)]
+    pub fn read_acquire(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_read_acquire(self.0.get());
+        }
+    }
+    /// See `atomic_set`.
+    #[inline(always)]
+    pub fn set(&self, i: i32) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_set(self.0.get(), i);
+        }
+    }
+    /// See `atomic_set_release`.
+    #[inline(always)]
+    pub fn set_release(&self, i: i32) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_set_release(self.0.get(), i);
+        }
+    }
+    /// See `atomic_add`.
+    #[inline(always)]
+    pub fn add(&self, i: i32) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_add(i, self.0.get());
+        }
+    }
+    /// See `atomic_add_return`.
+    #[inline(always)]
+    pub fn add_return(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_add_return(i, self.0.get());
+        }
+    }
+    /// See `atomic_add_return_acquire`.
+    #[inline(always)]
+    pub fn add_return_acquire(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_add_return_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic_add_return_release`.
+    #[inline(always)]
+    pub fn add_return_release(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_add_return_release(i, self.0.get());
+        }
+    }
+    /// See `atomic_add_return_relaxed`.
+    #[inline(always)]
+    pub fn add_return_relaxed(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_add_return_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_add`.
+    #[inline(always)]
+    pub fn fetch_add(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_add(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_add_acquire`.
+    #[inline(always)]
+    pub fn fetch_add_acquire(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_add_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_add_release`.
+    #[inline(always)]
+    pub fn fetch_add_release(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_add_release(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_add_relaxed`.
+    #[inline(always)]
+    pub fn fetch_add_relaxed(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_add_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic_sub`.
+    #[inline(always)]
+    pub fn sub(&self, i: i32) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_sub(i, self.0.get());
+        }
+    }
+    /// See `atomic_sub_return`.
+    #[inline(always)]
+    pub fn sub_return(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_sub_return(i, self.0.get());
+        }
+    }
+    /// See `atomic_sub_return_acquire`.
+    #[inline(always)]
+    pub fn sub_return_acquire(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_sub_return_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic_sub_return_release`.
+    #[inline(always)]
+    pub fn sub_return_release(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_sub_return_release(i, self.0.get());
+        }
+    }
+    /// See `atomic_sub_return_relaxed`.
+    #[inline(always)]
+    pub fn sub_return_relaxed(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_sub_return_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_sub`.
+    #[inline(always)]
+    pub fn fetch_sub(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_sub(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_sub_acquire`.
+    #[inline(always)]
+    pub fn fetch_sub_acquire(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_sub_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_sub_release`.
+    #[inline(always)]
+    pub fn fetch_sub_release(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_sub_release(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_sub_relaxed`.
+    #[inline(always)]
+    pub fn fetch_sub_relaxed(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_sub_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic_inc`.
+    #[inline(always)]
+    pub fn inc(&self) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_inc(self.0.get());
+        }
+    }
+    /// See `atomic_inc_return`.
+    #[inline(always)]
+    pub fn inc_return(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_inc_return(self.0.get());
+        }
+    }
+    /// See `atomic_inc_return_acquire`.
+    #[inline(always)]
+    pub fn inc_return_acquire(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_inc_return_acquire(self.0.get());
+        }
+    }
+    /// See `atomic_inc_return_release`.
+    #[inline(always)]
+    pub fn inc_return_release(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_inc_return_release(self.0.get());
+        }
+    }
+    /// See `atomic_inc_return_relaxed`.
+    #[inline(always)]
+    pub fn inc_return_relaxed(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_inc_return_relaxed(self.0.get());
+        }
+    }
+    /// See `atomic_fetch_inc`.
+    #[inline(always)]
+    pub fn fetch_inc(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_inc(self.0.get());
+        }
+    }
+    /// See `atomic_fetch_inc_acquire`.
+    #[inline(always)]
+    pub fn fetch_inc_acquire(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_inc_acquire(self.0.get());
+        }
+    }
+    /// See `atomic_fetch_inc_release`.
+    #[inline(always)]
+    pub fn fetch_inc_release(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_inc_release(self.0.get());
+        }
+    }
+    /// See `atomic_fetch_inc_relaxed`.
+    #[inline(always)]
+    pub fn fetch_inc_relaxed(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_inc_relaxed(self.0.get());
+        }
+    }
+    /// See `atomic_dec`.
+    #[inline(always)]
+    pub fn dec(&self) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_dec(self.0.get());
+        }
+    }
+    /// See `atomic_dec_return`.
+    #[inline(always)]
+    pub fn dec_return(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_dec_return(self.0.get());
+        }
+    }
+    /// See `atomic_dec_return_acquire`.
+    #[inline(always)]
+    pub fn dec_return_acquire(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_dec_return_acquire(self.0.get());
+        }
+    }
+    /// See `atomic_dec_return_release`.
+    #[inline(always)]
+    pub fn dec_return_release(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_dec_return_release(self.0.get());
+        }
+    }
+    /// See `atomic_dec_return_relaxed`.
+    #[inline(always)]
+    pub fn dec_return_relaxed(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_dec_return_relaxed(self.0.get());
+        }
+    }
+    /// See `atomic_fetch_dec`.
+    #[inline(always)]
+    pub fn fetch_dec(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_dec(self.0.get());
+        }
+    }
+    /// See `atomic_fetch_dec_acquire`.
+    #[inline(always)]
+    pub fn fetch_dec_acquire(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_dec_acquire(self.0.get());
+        }
+    }
+    /// See `atomic_fetch_dec_release`.
+    #[inline(always)]
+    pub fn fetch_dec_release(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_dec_release(self.0.get());
+        }
+    }
+    /// See `atomic_fetch_dec_relaxed`.
+    #[inline(always)]
+    pub fn fetch_dec_relaxed(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_dec_relaxed(self.0.get());
+        }
+    }
+    /// See `atomic_and`.
+    #[inline(always)]
+    pub fn and(&self, i: i32) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_and(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_and`.
+    #[inline(always)]
+    pub fn fetch_and(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_and(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_and_acquire`.
+    #[inline(always)]
+    pub fn fetch_and_acquire(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_and_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_and_release`.
+    #[inline(always)]
+    pub fn fetch_and_release(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_and_release(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_and_relaxed`.
+    #[inline(always)]
+    pub fn fetch_and_relaxed(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_and_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic_andnot`.
+    #[inline(always)]
+    pub fn andnot(&self, i: i32) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_andnot(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_andnot`.
+    #[inline(always)]
+    pub fn fetch_andnot(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_andnot(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_andnot_acquire`.
+    #[inline(always)]
+    pub fn fetch_andnot_acquire(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_andnot_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_andnot_release`.
+    #[inline(always)]
+    pub fn fetch_andnot_release(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_andnot_release(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_andnot_relaxed`.
+    #[inline(always)]
+    pub fn fetch_andnot_relaxed(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_andnot_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic_or`.
+    #[inline(always)]
+    pub fn or(&self, i: i32) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_or(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_or`.
+    #[inline(always)]
+    pub fn fetch_or(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_or(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_or_acquire`.
+    #[inline(always)]
+    pub fn fetch_or_acquire(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_or_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_or_release`.
+    #[inline(always)]
+    pub fn fetch_or_release(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_or_release(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_or_relaxed`.
+    #[inline(always)]
+    pub fn fetch_or_relaxed(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_or_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic_xor`.
+    #[inline(always)]
+    pub fn xor(&self, i: i32) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic_xor(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_xor`.
+    #[inline(always)]
+    pub fn fetch_xor(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_xor(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_xor_acquire`.
+    #[inline(always)]
+    pub fn fetch_xor_acquire(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_xor_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_xor_release`.
+    #[inline(always)]
+    pub fn fetch_xor_release(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_xor_release(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_xor_relaxed`.
+    #[inline(always)]
+    pub fn fetch_xor_relaxed(&self, i: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_xor_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic_xchg`.
+    #[inline(always)]
+    pub fn xchg(&self, new: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_xchg(self.0.get(), new);
+        }
+    }
+    /// See `atomic_xchg_acquire`.
+    #[inline(always)]
+    pub fn xchg_acquire(&self, new: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_xchg_acquire(self.0.get(), new);
+        }
+    }
+    /// See `atomic_xchg_release`.
+    #[inline(always)]
+    pub fn xchg_release(&self, new: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_xchg_release(self.0.get(), new);
+        }
+    }
+    /// See `atomic_xchg_relaxed`.
+    #[inline(always)]
+    pub fn xchg_relaxed(&self, new: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_xchg_relaxed(self.0.get(), new);
+        }
+    }
+    /// See `atomic_cmpxchg`.
+    #[inline(always)]
+    pub fn cmpxchg(&self, old: i32, new: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_cmpxchg(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic_cmpxchg_acquire`.
+    #[inline(always)]
+    pub fn cmpxchg_acquire(&self, old: i32, new: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_cmpxchg_acquire(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic_cmpxchg_release`.
+    #[inline(always)]
+    pub fn cmpxchg_release(&self, old: i32, new: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_cmpxchg_release(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic_cmpxchg_relaxed`.
+    #[inline(always)]
+    pub fn cmpxchg_relaxed(&self, old: i32, new: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_cmpxchg_relaxed(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic_try_cmpxchg`.
+    #[inline(always)]
+    pub fn try_cmpxchg(&self, old: &mut i32, new: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_try_cmpxchg(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic_try_cmpxchg_acquire`.
+    #[inline(always)]
+    pub fn try_cmpxchg_acquire(&self, old: &mut i32, new: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_try_cmpxchg_acquire(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic_try_cmpxchg_release`.
+    #[inline(always)]
+    pub fn try_cmpxchg_release(&self, old: &mut i32, new: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_try_cmpxchg_release(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic_try_cmpxchg_relaxed`.
+    #[inline(always)]
+    pub fn try_cmpxchg_relaxed(&self, old: &mut i32, new: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_try_cmpxchg_relaxed(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic_sub_and_test`.
+    #[inline(always)]
+    pub fn sub_and_test(&self, i: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_sub_and_test(i, self.0.get());
+        }
+    }
+    /// See `atomic_dec_and_test`.
+    #[inline(always)]
+    pub fn dec_and_test(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_dec_and_test(self.0.get());
+        }
+    }
+    /// See `atomic_inc_and_test`.
+    #[inline(always)]
+    pub fn inc_and_test(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_inc_and_test(self.0.get());
+        }
+    }
+    /// See `atomic_add_negative`.
+    #[inline(always)]
+    pub fn add_negative(&self, i: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_add_negative(i, self.0.get());
+        }
+    }
+    /// See `atomic_add_negative_acquire`.
+    #[inline(always)]
+    pub fn add_negative_acquire(&self, i: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_add_negative_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic_add_negative_release`.
+    #[inline(always)]
+    pub fn add_negative_release(&self, i: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_add_negative_release(i, self.0.get());
+        }
+    }
+    /// See `atomic_add_negative_relaxed`.
+    #[inline(always)]
+    pub fn add_negative_relaxed(&self, i: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_add_negative_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic_fetch_add_unless`.
+    #[inline(always)]
+    pub fn fetch_add_unless(&self, a: i32, u: i32) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_fetch_add_unless(self.0.get(), a, u);
+        }
+    }
+    /// See `atomic_add_unless`.
+    #[inline(always)]
+    pub fn add_unless(&self, a: i32, u: i32) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_add_unless(self.0.get(), a, u);
+        }
+    }
+    /// See `atomic_inc_not_zero`.
+    #[inline(always)]
+    pub fn inc_not_zero(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_inc_not_zero(self.0.get());
+        }
+    }
+    /// See `atomic_inc_unless_negative`.
+    #[inline(always)]
+    pub fn inc_unless_negative(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_inc_unless_negative(self.0.get());
+        }
+    }
+    /// See `atomic_dec_unless_positive`.
+    #[inline(always)]
+    pub fn dec_unless_positive(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_dec_unless_positive(self.0.get());
+        }
+    }
+    /// See `atomic_dec_if_positive`.
+    #[inline(always)]
+    pub fn dec_if_positive(&self) -> i32 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic_dec_if_positive(self.0.get());
+        }
+    }
+}
+
+impl AtomicI64 {
+    /// See `atomic64_read`.
+    #[inline(always)]
+    pub fn read(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_read(self.0.get());
+        }
+    }
+    /// See `atomic64_read_acquire`.
+    #[inline(always)]
+    pub fn read_acquire(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_read_acquire(self.0.get());
+        }
+    }
+    /// See `atomic64_set`.
+    #[inline(always)]
+    pub fn set(&self, i: i64) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_set(self.0.get(), i);
+        }
+    }
+    /// See `atomic64_set_release`.
+    #[inline(always)]
+    pub fn set_release(&self, i: i64) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_set_release(self.0.get(), i);
+        }
+    }
+    /// See `atomic64_add`.
+    #[inline(always)]
+    pub fn add(&self, i: i64) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_add(i, self.0.get());
+        }
+    }
+    /// See `atomic64_add_return`.
+    #[inline(always)]
+    pub fn add_return(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_add_return(i, self.0.get());
+        }
+    }
+    /// See `atomic64_add_return_acquire`.
+    #[inline(always)]
+    pub fn add_return_acquire(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_add_return_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic64_add_return_release`.
+    #[inline(always)]
+    pub fn add_return_release(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_add_return_release(i, self.0.get());
+        }
+    }
+    /// See `atomic64_add_return_relaxed`.
+    #[inline(always)]
+    pub fn add_return_relaxed(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_add_return_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_add`.
+    #[inline(always)]
+    pub fn fetch_add(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_add(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_add_acquire`.
+    #[inline(always)]
+    pub fn fetch_add_acquire(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_add_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_add_release`.
+    #[inline(always)]
+    pub fn fetch_add_release(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_add_release(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_add_relaxed`.
+    #[inline(always)]
+    pub fn fetch_add_relaxed(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_add_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic64_sub`.
+    #[inline(always)]
+    pub fn sub(&self, i: i64) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_sub(i, self.0.get());
+        }
+    }
+    /// See `atomic64_sub_return`.
+    #[inline(always)]
+    pub fn sub_return(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_sub_return(i, self.0.get());
+        }
+    }
+    /// See `atomic64_sub_return_acquire`.
+    #[inline(always)]
+    pub fn sub_return_acquire(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_sub_return_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic64_sub_return_release`.
+    #[inline(always)]
+    pub fn sub_return_release(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_sub_return_release(i, self.0.get());
+        }
+    }
+    /// See `atomic64_sub_return_relaxed`.
+    #[inline(always)]
+    pub fn sub_return_relaxed(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_sub_return_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_sub`.
+    #[inline(always)]
+    pub fn fetch_sub(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_sub(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_sub_acquire`.
+    #[inline(always)]
+    pub fn fetch_sub_acquire(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_sub_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_sub_release`.
+    #[inline(always)]
+    pub fn fetch_sub_release(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_sub_release(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_sub_relaxed`.
+    #[inline(always)]
+    pub fn fetch_sub_relaxed(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_sub_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic64_inc`.
+    #[inline(always)]
+    pub fn inc(&self) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_inc(self.0.get());
+        }
+    }
+    /// See `atomic64_inc_return`.
+    #[inline(always)]
+    pub fn inc_return(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_inc_return(self.0.get());
+        }
+    }
+    /// See `atomic64_inc_return_acquire`.
+    #[inline(always)]
+    pub fn inc_return_acquire(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_inc_return_acquire(self.0.get());
+        }
+    }
+    /// See `atomic64_inc_return_release`.
+    #[inline(always)]
+    pub fn inc_return_release(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_inc_return_release(self.0.get());
+        }
+    }
+    /// See `atomic64_inc_return_relaxed`.
+    #[inline(always)]
+    pub fn inc_return_relaxed(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_inc_return_relaxed(self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_inc`.
+    #[inline(always)]
+    pub fn fetch_inc(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_inc(self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_inc_acquire`.
+    #[inline(always)]
+    pub fn fetch_inc_acquire(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_inc_acquire(self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_inc_release`.
+    #[inline(always)]
+    pub fn fetch_inc_release(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_inc_release(self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_inc_relaxed`.
+    #[inline(always)]
+    pub fn fetch_inc_relaxed(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_inc_relaxed(self.0.get());
+        }
+    }
+    /// See `atomic64_dec`.
+    #[inline(always)]
+    pub fn dec(&self) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_dec(self.0.get());
+        }
+    }
+    /// See `atomic64_dec_return`.
+    #[inline(always)]
+    pub fn dec_return(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_dec_return(self.0.get());
+        }
+    }
+    /// See `atomic64_dec_return_acquire`.
+    #[inline(always)]
+    pub fn dec_return_acquire(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_dec_return_acquire(self.0.get());
+        }
+    }
+    /// See `atomic64_dec_return_release`.
+    #[inline(always)]
+    pub fn dec_return_release(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_dec_return_release(self.0.get());
+        }
+    }
+    /// See `atomic64_dec_return_relaxed`.
+    #[inline(always)]
+    pub fn dec_return_relaxed(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_dec_return_relaxed(self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_dec`.
+    #[inline(always)]
+    pub fn fetch_dec(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_dec(self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_dec_acquire`.
+    #[inline(always)]
+    pub fn fetch_dec_acquire(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_dec_acquire(self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_dec_release`.
+    #[inline(always)]
+    pub fn fetch_dec_release(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_dec_release(self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_dec_relaxed`.
+    #[inline(always)]
+    pub fn fetch_dec_relaxed(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_dec_relaxed(self.0.get());
+        }
+    }
+    /// See `atomic64_and`.
+    #[inline(always)]
+    pub fn and(&self, i: i64) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_and(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_and`.
+    #[inline(always)]
+    pub fn fetch_and(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_and(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_and_acquire`.
+    #[inline(always)]
+    pub fn fetch_and_acquire(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_and_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_and_release`.
+    #[inline(always)]
+    pub fn fetch_and_release(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_and_release(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_and_relaxed`.
+    #[inline(always)]
+    pub fn fetch_and_relaxed(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_and_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic64_andnot`.
+    #[inline(always)]
+    pub fn andnot(&self, i: i64) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_andnot(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_andnot`.
+    #[inline(always)]
+    pub fn fetch_andnot(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_andnot(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_andnot_acquire`.
+    #[inline(always)]
+    pub fn fetch_andnot_acquire(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_andnot_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_andnot_release`.
+    #[inline(always)]
+    pub fn fetch_andnot_release(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_andnot_release(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_andnot_relaxed`.
+    #[inline(always)]
+    pub fn fetch_andnot_relaxed(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_andnot_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic64_or`.
+    #[inline(always)]
+    pub fn or(&self, i: i64) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_or(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_or`.
+    #[inline(always)]
+    pub fn fetch_or(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_or(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_or_acquire`.
+    #[inline(always)]
+    pub fn fetch_or_acquire(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_or_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_or_release`.
+    #[inline(always)]
+    pub fn fetch_or_release(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_or_release(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_or_relaxed`.
+    #[inline(always)]
+    pub fn fetch_or_relaxed(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_or_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic64_xor`.
+    #[inline(always)]
+    pub fn xor(&self, i: i64) {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            atomic64_xor(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_xor`.
+    #[inline(always)]
+    pub fn fetch_xor(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_xor(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_xor_acquire`.
+    #[inline(always)]
+    pub fn fetch_xor_acquire(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_xor_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_xor_release`.
+    #[inline(always)]
+    pub fn fetch_xor_release(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_xor_release(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_xor_relaxed`.
+    #[inline(always)]
+    pub fn fetch_xor_relaxed(&self, i: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_xor_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic64_xchg`.
+    #[inline(always)]
+    pub fn xchg(&self, new: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_xchg(self.0.get(), new);
+        }
+    }
+    /// See `atomic64_xchg_acquire`.
+    #[inline(always)]
+    pub fn xchg_acquire(&self, new: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_xchg_acquire(self.0.get(), new);
+        }
+    }
+    /// See `atomic64_xchg_release`.
+    #[inline(always)]
+    pub fn xchg_release(&self, new: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_xchg_release(self.0.get(), new);
+        }
+    }
+    /// See `atomic64_xchg_relaxed`.
+    #[inline(always)]
+    pub fn xchg_relaxed(&self, new: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_xchg_relaxed(self.0.get(), new);
+        }
+    }
+    /// See `atomic64_cmpxchg`.
+    #[inline(always)]
+    pub fn cmpxchg(&self, old: i64, new: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_cmpxchg(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic64_cmpxchg_acquire`.
+    #[inline(always)]
+    pub fn cmpxchg_acquire(&self, old: i64, new: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_cmpxchg_acquire(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic64_cmpxchg_release`.
+    #[inline(always)]
+    pub fn cmpxchg_release(&self, old: i64, new: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_cmpxchg_release(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic64_cmpxchg_relaxed`.
+    #[inline(always)]
+    pub fn cmpxchg_relaxed(&self, old: i64, new: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_cmpxchg_relaxed(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic64_try_cmpxchg`.
+    #[inline(always)]
+    pub fn try_cmpxchg(&self, old: &mut i64, new: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_try_cmpxchg(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic64_try_cmpxchg_acquire`.
+    #[inline(always)]
+    pub fn try_cmpxchg_acquire(&self, old: &mut i64, new: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_try_cmpxchg_acquire(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic64_try_cmpxchg_release`.
+    #[inline(always)]
+    pub fn try_cmpxchg_release(&self, old: &mut i64, new: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_try_cmpxchg_release(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic64_try_cmpxchg_relaxed`.
+    #[inline(always)]
+    pub fn try_cmpxchg_relaxed(&self, old: &mut i64, new: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_try_cmpxchg_relaxed(self.0.get(), old, new);
+        }
+    }
+    /// See `atomic64_sub_and_test`.
+    #[inline(always)]
+    pub fn sub_and_test(&self, i: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_sub_and_test(i, self.0.get());
+        }
+    }
+    /// See `atomic64_dec_and_test`.
+    #[inline(always)]
+    pub fn dec_and_test(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_dec_and_test(self.0.get());
+        }
+    }
+    /// See `atomic64_inc_and_test`.
+    #[inline(always)]
+    pub fn inc_and_test(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_inc_and_test(self.0.get());
+        }
+    }
+    /// See `atomic64_add_negative`.
+    #[inline(always)]
+    pub fn add_negative(&self, i: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_add_negative(i, self.0.get());
+        }
+    }
+    /// See `atomic64_add_negative_acquire`.
+    #[inline(always)]
+    pub fn add_negative_acquire(&self, i: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_add_negative_acquire(i, self.0.get());
+        }
+    }
+    /// See `atomic64_add_negative_release`.
+    #[inline(always)]
+    pub fn add_negative_release(&self, i: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_add_negative_release(i, self.0.get());
+        }
+    }
+    /// See `atomic64_add_negative_relaxed`.
+    #[inline(always)]
+    pub fn add_negative_relaxed(&self, i: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_add_negative_relaxed(i, self.0.get());
+        }
+    }
+    /// See `atomic64_fetch_add_unless`.
+    #[inline(always)]
+    pub fn fetch_add_unless(&self, a: i64, u: i64) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_fetch_add_unless(self.0.get(), a, u);
+        }
+    }
+    /// See `atomic64_add_unless`.
+    #[inline(always)]
+    pub fn add_unless(&self, a: i64, u: i64) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_add_unless(self.0.get(), a, u);
+        }
+    }
+    /// See `atomic64_inc_not_zero`.
+    #[inline(always)]
+    pub fn inc_not_zero(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_inc_not_zero(self.0.get());
+        }
+    }
+    /// See `atomic64_inc_unless_negative`.
+    #[inline(always)]
+    pub fn inc_unless_negative(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_inc_unless_negative(self.0.get());
+        }
+    }
+    /// See `atomic64_dec_unless_positive`.
+    #[inline(always)]
+    pub fn dec_unless_positive(&self) -> bool {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_dec_unless_positive(self.0.get());
+        }
+    }
+    /// See `atomic64_dec_if_positive`.
+    #[inline(always)]
+    pub fn dec_if_positive(&self) -> i64 {
+        // SAFETY:`self.0.get()` is a valid pointer.
+        unsafe {
+            return atomic64_dec_if_positive(self.0.get());
+        }
+    }
+}
+
+// 258c6b7d580a83146e21973b1068cc92d9e65b87
diff --git a/scripts/atomic/gen-atomics.sh b/scripts/atomic/gen-atomics.sh
index 3927aee034c8..8d250c885c24 100755
--- a/scripts/atomic/gen-atomics.sh
+++ b/scripts/atomic/gen-atomics.sh
@@ -12,6 +12,7 @@ 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/atomic_helpers.h
+gen-rust-atomic.sh              ../rust/kernel/sync/atomic/impl.rs
 EOF
 while read script header args; do
 	/bin/sh ${ATOMICDIR}/${script} ${ATOMICTBL} ${args} > ${LINUXDIR}/include/${header}
diff --git a/scripts/atomic/gen-rust-atomic.sh b/scripts/atomic/gen-rust-atomic.sh
new file mode 100755
index 000000000000..491c643301a9
--- /dev/null
+++ b/scripts/atomic/gen-rust-atomic.sh
@@ -0,0 +1,136 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+ATOMICDIR=$(dirname $0)
+
+. ${ATOMICDIR}/atomic-tbl.sh
+
+#gen_ret_type(meta, int)
+gen_ret_type() {
+	local meta="$1"; shift
+	local int="$1"; shift
+
+	case "${meta}" in
+	[sv]) printf "";;
+	[bB]) printf -- "-> bool ";;
+	[aiIfFlR]) printf -- "-> ${int} ";;
+	esac
+}
+
+# gen_param_type(arg, int)
+gen_param_type()
+{
+	local type="${1%%:*}"; shift
+	local int="$1"; shift
+
+	case "${type}" in
+	i) type="${int}";;
+	p) type="&mut ${int}";;
+	esac
+
+	printf "${type}"
+}
+
+#gen_param(arg, int)
+gen_param()
+{
+	local arg="$1"; shift
+	local int="$1"; shift
+	local name="$(gen_param_name "${arg}")"
+	local type="$(gen_param_type "${arg}" "${int}")"
+
+	printf "${name}: ${type}"
+}
+
+#gen_params(int, arg...)
+gen_params()
+{
+	local int="$1"; shift
+
+	while [ "$#" -gt 0 ]; do
+		if [[ "$1" != "v" && "$1" != "cv" ]]; then
+			printf ", "
+			gen_param "$1" "${int}"
+		fi
+		shift;
+	done
+}
+
+#gen_args(arg...)
+gen_args()
+{
+	while [ "$#" -gt 0 ]; do
+		if [[ "$1" == "v" || "$1" == "cv" ]]; then
+			printf "self.0.get()"
+			[ "$#" -gt 1 ] && printf ", "
+		else
+			printf "$(gen_param_name "$1")"
+			[ "$#" -gt 1 ] && printf ", "
+		fi
+		shift;
+	done
+}
+
+#gen_proto_order_variant(meta, pfx, name, sfx, order, atomic, ty, int, raw, 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 ty="$1"; shift
+	local int="$1"; shift
+	local raw="$1"; shift
+
+	local fn_name="${raw}${pfx}${name}${sfx}${order}"
+	local atomicname="${raw}${atomic}_${pfx}${name}${sfx}${order}"
+
+	local ret="$(gen_ret_type "${meta}" "${int}")"
+	local params="$(gen_params "${int}" $@)"
+	local args="$(gen_args "$@")"
+	local retstmt="$(gen_ret_stmt "${meta}")"
+
+cat <<EOF
+    /// See \`${atomicname}\`.
+    #[inline(always)]
+    pub fn ${fn_name}(&self${params}) ${ret}{
+        // SAFETY:\`self.0.get()\` is a valid pointer.
+        unsafe {
+            ${retstmt}${atomicname}(${args});
+        }
+    }
+EOF
+}
+
+cat << EOF
+// SPDX-License-Identifier: GPL-2.0
+
+//! Generated by $0
+//! DO NOT MODIFY THIS FILE DIRECTLY
+
+use super::*;
+use crate::bindings::*;
+
+impl AtomicI32 {
+EOF
+
+grep '^[a-z]' "$1" | while read name meta args; do
+	gen_proto "${meta}" "${name}" "atomic" "AtomicI32" "i32" "" ${args}
+done
+
+cat << EOF
+}
+
+impl AtomicI64 {
+EOF
+
+grep '^[a-z]' "$1" | while read name meta args; do
+	gen_proto "${meta}" "${name}" "atomic64" "AtomicI64" "i64" "" ${args}
+done
+
+cat << EOF
+}
+
+EOF
-- 
2.45.2

Re: [RFC 2/2] rust: sync: Add atomic support

[Greg Kroah-Hartman]

On Wed, Jun 12, 2024 at 03:30:25PM -0700, Boqun Feng wrote:
> --- /dev/null
> +++ b/rust/kernel/sync/atomic/impl.rs
> @@ -0,0 +1,1375 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Generated by scripts/atomic/gen-rust-atomic.sh
> +//! DO NOT MODIFY THIS FILE DIRECTLY

Again, why not generate at build time?

thanks,

greg k-h

Re: [RFC 2/2] rust: sync: Add atomic support

[Gary Guo]

On Wed, 12 Jun 2024 15:30:25 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> Provide two atomic types: AtomicI32 and AtomicI64 with the existing
> implemenation of C atomics. These atomics have the same semantics of the
> corresponding LKMM C atomics, and using one memory (ordering) model
> certainly reduces the reasoning difficulty and potential bugs from the
> interaction of two different memory models.
> 
> Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
> my responsiblity on these Rust APIs.
> 
> Note that `Atomic*::new()`s are implemented vi open coding on struct
> atomic*_t. This allows `new()` being a `const` function, so that it can
> be used in constant contexts.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  MAINTAINERS                       |    4 +-
>  arch/arm64/kernel/cpufeature.c    |    2 +
>  rust/kernel/sync.rs               |    1 +
>  rust/kernel/sync/atomic.rs        |   63 ++
>  rust/kernel/sync/atomic/impl.rs   | 1375 +++++++++++++++++++++++++++++
>  scripts/atomic/gen-atomics.sh     |    1 +
>  scripts/atomic/gen-rust-atomic.sh |  136 +++
>  7 files changed, 1581 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/sync/atomic.rs
>  create mode 100644 rust/kernel/sync/atomic/impl.rs
>  create mode 100755 scripts/atomic/gen-rust-atomic.sh
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d6c90161c7bf..a8528d27b260 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3458,7 +3458,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
> @@ -3467,6 +3467,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/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 48e7029f1054..99e6e2b2867f 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1601,6 +1601,8 @@ static bool
>  has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
>  {
>  	u64 val = read_scoped_sysreg(entry, scope);
> +	if (entry->capability == ARM64_HAS_LSE_ATOMICS)
> +		return false;
>  	return feature_matches(val, entry);
>  }
>  
> 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..b0f852cf1741
> --- /dev/null
> +++ b/rust/kernel/sync/atomic.rs
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Atomic primitives.
> +//!
> +//! These primitives have the same semantics as their C counterparts, for precise definitions of
> +//! the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory (Consistency)
> +//! Model is the only model for Rust development in kernel right now, please avoid to use Rust's
> +//! own atomics.
> +
> +use crate::bindings::{atomic64_t, atomic_t};
> +use crate::types::Opaque;
> +
> +mod r#impl;
> +
> +/// Atomic 32bit signed integers.
> +pub struct AtomicI32(Opaque<atomic_t>);
> +
> +/// Atomic 64bit signed integers.
> +pub struct AtomicI64(Opaque<atomic64_t>);


Can we avoid two types and use a generic `Atomic<T>` and then implement
on `Atomic<i32>` and `Atomic<i64>` instead? Like the recent
generic NonZero in Rust standard library or the atomic crate
(https://docs.rs/atomic/).

I think this is better for ergonomics. The impl do need some extra
casting though.

Best,
Gary

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Thu, Jun 13, 2024 at 02:44:32PM +0100, Gary Guo wrote:
> On Wed, 12 Jun 2024 15:30:25 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> > Provide two atomic types: AtomicI32 and AtomicI64 with the existing
> > implemenation of C atomics. These atomics have the same semantics of the
> > corresponding LKMM C atomics, and using one memory (ordering) model
> > certainly reduces the reasoning difficulty and potential bugs from the
> > interaction of two different memory models.
> > 
> > Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
> > my responsiblity on these Rust APIs.
> > 
> > Note that `Atomic*::new()`s are implemented vi open coding on struct
> > atomic*_t. This allows `new()` being a `const` function, so that it can
> > be used in constant contexts.
> > 
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  MAINTAINERS                       |    4 +-
> >  arch/arm64/kernel/cpufeature.c    |    2 +
> >  rust/kernel/sync.rs               |    1 +
> >  rust/kernel/sync/atomic.rs        |   63 ++
> >  rust/kernel/sync/atomic/impl.rs   | 1375 +++++++++++++++++++++++++++++
> >  scripts/atomic/gen-atomics.sh     |    1 +
> >  scripts/atomic/gen-rust-atomic.sh |  136 +++
> >  7 files changed, 1581 insertions(+), 1 deletion(-)
> >  create mode 100644 rust/kernel/sync/atomic.rs
> >  create mode 100644 rust/kernel/sync/atomic/impl.rs
> >  create mode 100755 scripts/atomic/gen-rust-atomic.sh
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d6c90161c7bf..a8528d27b260 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3458,7 +3458,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
> > @@ -3467,6 +3467,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/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index 48e7029f1054..99e6e2b2867f 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -1601,6 +1601,8 @@ static bool
> >  has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
> >  {
> >  	u64 val = read_scoped_sysreg(entry, scope);
> > +	if (entry->capability == ARM64_HAS_LSE_ATOMICS)
> > +		return false;
> >  	return feature_matches(val, entry);
> >  }
> >  
> > 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..b0f852cf1741
> > --- /dev/null
> > +++ b/rust/kernel/sync/atomic.rs
> > @@ -0,0 +1,63 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Atomic primitives.
> > +//!
> > +//! These primitives have the same semantics as their C counterparts, for precise definitions of
> > +//! the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory (Consistency)
> > +//! Model is the only model for Rust development in kernel right now, please avoid to use Rust's
> > +//! own atomics.
> > +
> > +use crate::bindings::{atomic64_t, atomic_t};
> > +use crate::types::Opaque;
> > +
> > +mod r#impl;
> > +
> > +/// Atomic 32bit signed integers.
> > +pub struct AtomicI32(Opaque<atomic_t>);
> > +
> > +/// Atomic 64bit signed integers.
> > +pub struct AtomicI64(Opaque<atomic64_t>);
> 
> 
> Can we avoid two types and use a generic `Atomic<T>` and then implement
> on `Atomic<i32>` and `Atomic<i64>` instead? Like the recent
> generic NonZero in Rust standard library or the atomic crate
> (https://docs.rs/atomic/).
> 

We can always add a layer on top of what we have here to provide the
generic `Atomic<T>`. However, I personally don't think generic
`Atomic<T>` is a good idea, for a few reasons:

*	I'm not sure it will bring benefits to users, the current atomic
	users in kernel are pretty specific on the size of atomic they
	use, so they want to directly use AtomicI32 or AtomicI64 in
	their type definitions rather than use a `Atomic<T>` where their
	users can provide type later.

*	I can also see the future where we have different APIs on
	different types of atomics, for example, we could have a:

		impl AtomicI64 {
		    pub fn split(&self) -> (&AtomicI32, &AtomicI32)
		}

	which doesn't exist for AtomicI32. Note this is not a UB because
	we write our atomic implementation in asm, so it's perfectly
	fine for mix-sized atomics.

So let's start with some basic and simple until we really have a need
for generic `Atomic<T>`. Thoughts?

Regards,
Boqun

> I think this is better for ergonomics. The impl do need some extra
> casting though.
> 
> Best,
> Gary

Re: [RFC 2/2] rust: sync: Add atomic support

[Gary Guo]

On Thu, 13 Jun 2024 09:30:26 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> > > diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> > > new file mode 100644
> > > index 000000000000..b0f852cf1741
> > > --- /dev/null
> > > +++ b/rust/kernel/sync/atomic.rs
> > > @@ -0,0 +1,63 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +//! Atomic primitives.
> > > +//!
> > > +//! These primitives have the same semantics as their C counterparts, for precise definitions of
> > > +//! the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory (Consistency)
> > > +//! Model is the only model for Rust development in kernel right now, please avoid to use Rust's
> > > +//! own atomics.
> > > +
> > > +use crate::bindings::{atomic64_t, atomic_t};
> > > +use crate::types::Opaque;
> > > +
> > > +mod r#impl;
> > > +
> > > +/// Atomic 32bit signed integers.
> > > +pub struct AtomicI32(Opaque<atomic_t>);
> > > +
> > > +/// Atomic 64bit signed integers.
> > > +pub struct AtomicI64(Opaque<atomic64_t>);
> > 
> > 
> > Can we avoid two types and use a generic `Atomic<T>` and then implement
> > on `Atomic<i32>` and `Atomic<i64>` instead? Like the recent
> > generic NonZero in Rust standard library or the atomic crate
> > (https://docs.rs/atomic/).
> > 
> 
> We can always add a layer on top of what we have here to provide the
> generic `Atomic<T>`. However, I personally don't think generic
> `Atomic<T>` is a good idea, for a few reasons:
> 
> *	I'm not sure it will bring benefits to users, the current atomic
> 	users in kernel are pretty specific on the size of atomic they
> 	use, so they want to directly use AtomicI32 or AtomicI64 in
> 	their type definitions rather than use a `Atomic<T>` where their
> 	users can provide type later.

You can write `Atomic<i32>` and `Atomic<i64>`.

> 
> *	I can also see the future where we have different APIs on
> 	different types of atomics, for example, we could have a:
> 
> 		impl AtomicI64 {
> 		    pub fn split(&self) -> (&AtomicI32, &AtomicI32)
> 		}
> 
> 	which doesn't exist for AtomicI32. Note this is not a UB because
> 	we write our atomic implementation in asm, so it's perfectly
> 	fine for mix-sized atomics.

You can still have

	impl Atomic<i64> {
	    pub fn split(&self) -> (&Atomic<i32>, &Atomic<i32>)
	}

I see `Atomic<i32>/Atomic<i64>` being generally more flexible than
`AtomicI32/AtomicI64`, without very little downsides. We may not have
generic users but I think it doesn't do any harm to have it in a form
that makes future generics easy.

Best,
Gary

Re: [RFC 2/2] rust: sync: Add atomic support

[Miguel Ojeda]

On Thu, Jun 13, 2024 at 6:31 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> So let's start with some basic and simple until we really have a need
> for generic `Atomic<T>`. Thoughts?

I don't want to delay this, but using generics would be more flexible,
right? e.g. it could allow us to have atomics of, say, newtypes, if
that were to be useful.

Is there a particular disadvantage of using the generics? The two
cases you mentioned would just be written explicitly, right?

One disadvantage would be that they are different from the Rust
standard library ones, e.g. in case we wanted third-party code to use
them, but could be provided if needed later on.

Cheers,
Miguel

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Thu, Jun 13, 2024 at 07:22:54PM +0200, Miguel Ojeda wrote:
> On Thu, Jun 13, 2024 at 6:31 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > So let's start with some basic and simple until we really have a need
> > for generic `Atomic<T>`. Thoughts?
> 
> I don't want to delay this, but using generics would be more flexible,
> right? e.g. it could allow us to have atomics of, say, newtypes, if
> that were to be useful.
> 
> Is there a particular disadvantage of using the generics? The two
> cases you mentioned would just be written explicitly, right?
> 
> One disadvantage would be that they are different from the Rust
> standard library ones, e.g. in case we wanted third-party code to use
> them, but could be provided if needed later on.
> 

Well, the other thing is AtomicI32 -> atomic_t and AtomicI64 ->
atomic64_t are perfect mappings, and we can treat AtomicI32 and
AtomicI64 as a separate layer that wires C atomics into Rust. As I said,
we can build `Atomic<T>` on top of this layer, like:

    Atomic<T>
      |
      V
    AtomicI{32,64}
      |
      V
    atomic{,64}_t

and if we drop this layer, the dependencies become:

    Atomic<i32,i64> <- Atomic<u32,u64>
      |
      V
    atomic{,64}_t

i.e. in the same layer of Atomic<T>, some of them directly depend on
some other Atomic<T> types, which doesn't look very clean to me. And it
might be difficult for architecture maintainers to track the exact
dependency for Rust code.

This is also the reason why I didn't use Rust macros to generate
AtomicI32 and AtomicI64 implementation: I use a script to generate .rs
file. This ensures AtomicI32 and AtomicI64 stay with the exact same set
of APIs as atomic{,64}_t (described by scripts/atomic/atomics.tbl. Put
it in another way, I guess you can think AtomicI32 and AtomicI64 as some
sort of intrinsic layer provided by C. And should we need it, we can
build an Atomic<T> layer on top of it.

Does this make sense?

Regards,
Boqun

> Cheers,
> Miguel

Re: [RFC 2/2] rust: sync: Add atomic support

[Miguel Ojeda]

On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Does this make sense?

Implementation-wise, if you think it is simpler or more clear/elegant
to have the extra lower level layer, then that sounds fine.

However, I was mainly talking about what we would eventually expose to
users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
then we could make the lower layer private already.

We can defer that extra layer/work if needed even if we go for
`Atomic<T>`, but it would be nice to understand if we have consensus
for an eventual user-facing API, or if someone has any other opinion
or concerns on one vs. the other.

Cheers,
Miguel

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Does this make sense?
> 
> Implementation-wise, if you think it is simpler or more clear/elegant
> to have the extra lower level layer, then that sounds fine.
> 
> However, I was mainly talking about what we would eventually expose to
> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,

The truth is I don't know ;-) I don't have much data on which one is
better. Personally, I think AtomicI32 and AtomicI64 make the users have
to think about size, alignment, etc, and I think that's important for
atomic users and people who review their code, because before one uses
atomics, one should ask themselves: why don't I use a lock? Atomics
provide the ablities to do low level stuffs and when doing low level
stuffs, you want to be more explicit than ergonomic.

That said, I keep an open mind on `Atomic<T>`, maybe it will show its
value at last. But right now, I'm not convinced personally.

> then we could make the lower layer private already.
> 
> We can defer that extra layer/work if needed even if we go for
> `Atomic<T>`, but it would be nice to understand if we have consensus
> for an eventual user-facing API, or if someone has any other opinion
> or concerns on one vs. the other.
> 

Yes, that'll be great. I'd love to see others' inputs!

Regards,
Boqun

> Cheers,
> Miguel

Re: [RFC 2/2] rust: sync: Add atomic support

[Benno Lossin]

On 14.06.24 16:33, Boqun Feng wrote:
> On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>>>
>>> Does this make sense?
>>
>> Implementation-wise, if you think it is simpler or more clear/elegant
>> to have the extra lower level layer, then that sounds fine.
>>
>> However, I was mainly talking about what we would eventually expose to
>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> 
> The truth is I don't know ;-) I don't have much data on which one is
> better. Personally, I think AtomicI32 and AtomicI64 make the users have
> to think about size, alignment, etc, and I think that's important for
> atomic users and people who review their code, because before one uses
> atomics, one should ask themselves: why don't I use a lock? Atomics
> provide the ablities to do low level stuffs and when doing low level
> stuffs, you want to be more explicit than ergonomic.

How would this be different with `Atomic<i32>` and `Atomic<i64>`? Just
because the underlying `Atomic<I>` type is generic shouldn't change
this, right?

---
Cheers,
Benno

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Fri, Jun 14, 2024 at 09:22:24PM +0000, Benno Lossin wrote:
> On 14.06.24 16:33, Boqun Feng wrote:
> > On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
> >> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >>>
> >>> Does this make sense?
> >>
> >> Implementation-wise, if you think it is simpler or more clear/elegant
> >> to have the extra lower level layer, then that sounds fine.
> >>
> >> However, I was mainly talking about what we would eventually expose to
> >> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> > 
> > The truth is I don't know ;-) I don't have much data on which one is
> > better. Personally, I think AtomicI32 and AtomicI64 make the users have
> > to think about size, alignment, etc, and I think that's important for
> > atomic users and people who review their code, because before one uses
> > atomics, one should ask themselves: why don't I use a lock? Atomics
> > provide the ablities to do low level stuffs and when doing low level
> > stuffs, you want to be more explicit than ergonomic.
> 
> How would this be different with `Atomic<i32>` and `Atomic<i64>`? Just

The difference is that with Atomic{I32,I64} APIs, one has to choose (and
think about) the size when using atomics, and cannot leave that option
open. It's somewhere unconvenient, but as I said, atomics variables are
different. For example, if someone is going to implement a reference
counter struct, they can define as follow:

	struct Refcount<T> {
	    refcount: AtomicI32,
	    data: UnsafeCell<T>
	}

but with atomic generic, people can leave that option open and do:

	struct Refcount<R, T> {
	    refcount: Atomic<R>,
	    data: UnsafeCell<T>
	}

while it provides configurable options for experienced users, but it
also provides opportunities for sub-optimal types, e.g. Refcount<u8, T>:
on ll/sc architectures, because `data` and `refcount` can be in the same
machine-word, the accesses of `refcount` are affected by the accesses of
`data`.

The point I'm trying to make here is: when you are using atomics, you
care about performance a lot (otherwise, why don't you use a lock?), and
because of that, you should care about the size of the atomics, because
it may affect the performance significantly.

Regards,
Boqun

> because the underlying `Atomic<I>` type is generic shouldn't change
> this, right?
> 
> ---
> Cheers,
> Benno
> 

Re: [RFC 2/2] rust: sync: Add atomic support

[Benno Lossin]

On 15.06.24 03:33, Boqun Feng wrote:
> On Fri, Jun 14, 2024 at 09:22:24PM +0000, Benno Lossin wrote:
>> On 14.06.24 16:33, Boqun Feng wrote:
>>> On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
>>>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>>>>>
>>>>> Does this make sense?
>>>>
>>>> Implementation-wise, if you think it is simpler or more clear/elegant
>>>> to have the extra lower level layer, then that sounds fine.
>>>>
>>>> However, I was mainly talking about what we would eventually expose to
>>>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
>>>
>>> The truth is I don't know ;-) I don't have much data on which one is
>>> better. Personally, I think AtomicI32 and AtomicI64 make the users have
>>> to think about size, alignment, etc, and I think that's important for
>>> atomic users and people who review their code, because before one uses
>>> atomics, one should ask themselves: why don't I use a lock? Atomics
>>> provide the ablities to do low level stuffs and when doing low level
>>> stuffs, you want to be more explicit than ergonomic.
>>
>> How would this be different with `Atomic<i32>` and `Atomic<i64>`? Just
> 
> The difference is that with Atomic{I32,I64} APIs, one has to choose (and
> think about) the size when using atomics, and cannot leave that option
> open. It's somewhere unconvenient, but as I said, atomics variables are
> different. For example, if someone is going to implement a reference
> counter struct, they can define as follow:
> 
> 	struct Refcount<T> {
> 	    refcount: AtomicI32,
> 	    data: UnsafeCell<T>
> 	}
> 
> but with atomic generic, people can leave that option open and do:
> 
> 	struct Refcount<R, T> {
> 	    refcount: Atomic<R>,
> 	    data: UnsafeCell<T>
> 	}
> 
> while it provides configurable options for experienced users, but it
> also provides opportunities for sub-optimal types, e.g. Refcount<u8, T>:
> on ll/sc architectures, because `data` and `refcount` can be in the same
> machine-word, the accesses of `refcount` are affected by the accesses of
> `data`.

I think this is a non-issue. We have two options of counteracting this:
1. We can just point this out in reviews and force people to use
   `Atomic<T>` with a concrete type. In cases where there really is the
   need to be generic, we can have it.
2. We can add a private trait in the bounds for the generic, nobody
   outside of the module can access it and thus they need to use a
   concrete type:

        // needs a better name
        trait Integer {}
        impl Integer for i32 {}
        impl Integer for i64 {}

        pub struct Atomic<T: Integer> {
            /* ... */
        }

And then in the other module, you can't do this (with compiler error):

        pub struct Refcount<R: Integer, T> {
                            // ^^^^^^^ not found in this scope
                            // note: trait `crate::atomic::Integer` exists but is inaccessible
            refcount: Atomic<R>,
            data: UnsafeCell<T>,
        }

I think that we can start with approach 2 and if we find a use-case
where generics are really unavoidable, we can either put it in the same
module as `Atomic<T>`, or change the access of `Integer`.

---
Cheers,
Benno

> The point I'm trying to make here is: when you are using atomics, you
> care about performance a lot (otherwise, why don't you use a lock?), and
> because of that, you should care about the size of the atomics, because
> it may affect the performance significantly.

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sat, Jun 15, 2024 at 07:09:30AM +0000, Benno Lossin wrote:
> On 15.06.24 03:33, Boqun Feng wrote:
> > On Fri, Jun 14, 2024 at 09:22:24PM +0000, Benno Lossin wrote:
> >> On 14.06.24 16:33, Boqun Feng wrote:
> >>> On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
> >>>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >>>>>
> >>>>> Does this make sense?
> >>>>
> >>>> Implementation-wise, if you think it is simpler or more clear/elegant
> >>>> to have the extra lower level layer, then that sounds fine.
> >>>>
> >>>> However, I was mainly talking about what we would eventually expose to
> >>>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> >>>
> >>> The truth is I don't know ;-) I don't have much data on which one is
> >>> better. Personally, I think AtomicI32 and AtomicI64 make the users have
> >>> to think about size, alignment, etc, and I think that's important for
> >>> atomic users and people who review their code, because before one uses
> >>> atomics, one should ask themselves: why don't I use a lock? Atomics
> >>> provide the ablities to do low level stuffs and when doing low level
> >>> stuffs, you want to be more explicit than ergonomic.
> >>
> >> How would this be different with `Atomic<i32>` and `Atomic<i64>`? Just
> > 
> > The difference is that with Atomic{I32,I64} APIs, one has to choose (and
> > think about) the size when using atomics, and cannot leave that option
> > open. It's somewhere unconvenient, but as I said, atomics variables are
> > different. For example, if someone is going to implement a reference
> > counter struct, they can define as follow:
> > 
> > 	struct Refcount<T> {
> > 	    refcount: AtomicI32,
> > 	    data: UnsafeCell<T>
> > 	}
> > 
> > but with atomic generic, people can leave that option open and do:
> > 
> > 	struct Refcount<R, T> {
> > 	    refcount: Atomic<R>,
> > 	    data: UnsafeCell<T>
> > 	}
> > 
> > while it provides configurable options for experienced users, but it
> > also provides opportunities for sub-optimal types, e.g. Refcount<u8, T>:
> > on ll/sc architectures, because `data` and `refcount` can be in the same
> > machine-word, the accesses of `refcount` are affected by the accesses of
> > `data`.
> 
> I think this is a non-issue. We have two options of counteracting this:
> 1. We can just point this out in reviews and force people to use
>    `Atomic<T>` with a concrete type. In cases where there really is the
>    need to be generic, we can have it.
> 2. We can add a private trait in the bounds for the generic, nobody
>    outside of the module can access it and thus they need to use a
>    concrete type:
> 
>         // needs a better name
>         trait Integer {}
>         impl Integer for i32 {}
>         impl Integer for i64 {}
> 
>         pub struct Atomic<T: Integer> {
>             /* ... */
>         }
> 
> And then in the other module, you can't do this (with compiler error):
> 
>         pub struct Refcount<R: Integer, T> {
>                             // ^^^^^^^ not found in this scope
>                             // note: trait `crate::atomic::Integer` exists but is inaccessible
>             refcount: Atomic<R>,
>             data: UnsafeCell<T>,
>         }
> 
> I think that we can start with approach 2 and if we find a use-case
> where generics are really unavoidable, we can either put it in the same
> module as `Atomic<T>`, or change the access of `Integer`.
> 

What's the issue of having AtomicI32 and AtomicI64 first then? We don't
need to do 1 or 2 until the real users show up.

And I'd like also to point out that there are a few more trait bound
designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
have different sets of API (no inc_unless_negative() for u32).

Don't make me wrong, I have no doubt we can handle this in the type
system, but given the design work need, won't it make sense that we take
baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
already have real users, and then if there are some values of generic
atomics, we introduce them and have proper discussion on design.

To me, it's perfectly fine that Atomic{I32,I64} co-exist with Atomic<T>.
What's the downside? A bit specific example would help me understand
the real concern here.


Regards,
Boqun

> ---
> Cheers,
> Benno
> 
> > The point I'm trying to make here is: when you are using atomics, you
> > care about performance a lot (otherwise, why don't you use a lock?), and
> > because of that, you should care about the size of the atomics, because
> > it may affect the performance significantly.
> 

Re: [RFC 2/2] rust: sync: Add atomic support

[Benno Lossin]

On 16.06.24 00:12, Boqun Feng wrote:
> On Sat, Jun 15, 2024 at 07:09:30AM +0000, Benno Lossin wrote:
>> On 15.06.24 03:33, Boqun Feng wrote:
>>> On Fri, Jun 14, 2024 at 09:22:24PM +0000, Benno Lossin wrote:
>>>> On 14.06.24 16:33, Boqun Feng wrote:
>>>>> On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
>>>>>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>>>>>>>
>>>>>>> Does this make sense?
>>>>>>
>>>>>> Implementation-wise, if you think it is simpler or more clear/elegant
>>>>>> to have the extra lower level layer, then that sounds fine.
>>>>>>
>>>>>> However, I was mainly talking about what we would eventually expose to
>>>>>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
>>>>>
>>>>> The truth is I don't know ;-) I don't have much data on which one is
>>>>> better. Personally, I think AtomicI32 and AtomicI64 make the users have
>>>>> to think about size, alignment, etc, and I think that's important for
>>>>> atomic users and people who review their code, because before one uses
>>>>> atomics, one should ask themselves: why don't I use a lock? Atomics
>>>>> provide the ablities to do low level stuffs and when doing low level
>>>>> stuffs, you want to be more explicit than ergonomic.
>>>>
>>>> How would this be different with `Atomic<i32>` and `Atomic<i64>`? Just
>>>
>>> The difference is that with Atomic{I32,I64} APIs, one has to choose (and
>>> think about) the size when using atomics, and cannot leave that option
>>> open. It's somewhere unconvenient, but as I said, atomics variables are
>>> different. For example, if someone is going to implement a reference
>>> counter struct, they can define as follow:
>>>
>>> 	struct Refcount<T> {
>>> 	    refcount: AtomicI32,
>>> 	    data: UnsafeCell<T>
>>> 	}
>>>
>>> but with atomic generic, people can leave that option open and do:
>>>
>>> 	struct Refcount<R, T> {
>>> 	    refcount: Atomic<R>,
>>> 	    data: UnsafeCell<T>
>>> 	}
>>>
>>> while it provides configurable options for experienced users, but it
>>> also provides opportunities for sub-optimal types, e.g. Refcount<u8, T>:
>>> on ll/sc architectures, because `data` and `refcount` can be in the same
>>> machine-word, the accesses of `refcount` are affected by the accesses of
>>> `data`.
>>
>> I think this is a non-issue. We have two options of counteracting this:
>> 1. We can just point this out in reviews and force people to use
>>    `Atomic<T>` with a concrete type. In cases where there really is the
>>    need to be generic, we can have it.
>> 2. We can add a private trait in the bounds for the generic, nobody
>>    outside of the module can access it and thus they need to use a
>>    concrete type:
>>
>>         // needs a better name
>>         trait Integer {}
>>         impl Integer for i32 {}
>>         impl Integer for i64 {}
>>
>>         pub struct Atomic<T: Integer> {
>>             /* ... */
>>         }
>>
>> And then in the other module, you can't do this (with compiler error):
>>
>>         pub struct Refcount<R: Integer, T> {
>>                             // ^^^^^^^ not found in this scope
>>                             // note: trait `crate::atomic::Integer` exists but is inaccessible
>>             refcount: Atomic<R>,
>>             data: UnsafeCell<T>,
>>         }
>>
>> I think that we can start with approach 2 and if we find a use-case
>> where generics are really unavoidable, we can either put it in the same
>> module as `Atomic<T>`, or change the access of `Integer`.
>>
> 
> What's the issue of having AtomicI32 and AtomicI64 first then? We don't
> need to do 1 or 2 until the real users show up.

Generics allow you to avoid code duplication (I don't think that you
want to create the `Atomic{I32,I64}` types via macros...). We would have
to do a lot of refactoring, when we want to introduce it. I don't see
the harm of introducing generics from the get-go.

> And I'd like also to point out that there are a few more trait bound
> designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
> have different sets of API (no inc_unless_negative() for u32).

Sure, just like Gary said, you can just do:

    impl Atomic<i32> {
        pub fn inc_unless_negative(&self, ordering: Ordering) -> bool;
    }

Or add a `HasNegative` trait.

> Don't make me wrong, I have no doubt we can handle this in the type
> system, but given the design work need, won't it make sense that we take
> baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
> already have real users, and then if there are some values of generic
> atomics, we introduce them and have proper discussion on design.

I don't understand this point, why can't we put in the effort for a good
design? AFAIK we normally spend considerable time to get the API right
and I think in this case it would include making it generic.

> To me, it's perfectly fine that Atomic{I32,I64} co-exist with Atomic<T>.
> What's the downside? A bit specific example would help me understand
> the real concern here.

I don't like that, why have two ways of doing the same thing? People
will be confused whether they should use `AtomicI32` vs `Atomic<i32>`...

---
Cheers,
Benno

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 09:46:45AM +0000, Benno Lossin wrote:
> On 16.06.24 00:12, Boqun Feng wrote:
> > On Sat, Jun 15, 2024 at 07:09:30AM +0000, Benno Lossin wrote:
> >> On 15.06.24 03:33, Boqun Feng wrote:
> >>> On Fri, Jun 14, 2024 at 09:22:24PM +0000, Benno Lossin wrote:
> >>>> On 14.06.24 16:33, Boqun Feng wrote:
> >>>>> On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
> >>>>>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >>>>>>>
> >>>>>>> Does this make sense?
> >>>>>>
> >>>>>> Implementation-wise, if you think it is simpler or more clear/elegant
> >>>>>> to have the extra lower level layer, then that sounds fine.
> >>>>>>
> >>>>>> However, I was mainly talking about what we would eventually expose to
> >>>>>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> >>>>>
> >>>>> The truth is I don't know ;-) I don't have much data on which one is
> >>>>> better. Personally, I think AtomicI32 and AtomicI64 make the users have
> >>>>> to think about size, alignment, etc, and I think that's important for
> >>>>> atomic users and people who review their code, because before one uses
> >>>>> atomics, one should ask themselves: why don't I use a lock? Atomics
> >>>>> provide the ablities to do low level stuffs and when doing low level
> >>>>> stuffs, you want to be more explicit than ergonomic.
> >>>>
> >>>> How would this be different with `Atomic<i32>` and `Atomic<i64>`? Just
> >>>
> >>> The difference is that with Atomic{I32,I64} APIs, one has to choose (and
> >>> think about) the size when using atomics, and cannot leave that option
> >>> open. It's somewhere unconvenient, but as I said, atomics variables are
> >>> different. For example, if someone is going to implement a reference
> >>> counter struct, they can define as follow:
> >>>
> >>> 	struct Refcount<T> {
> >>> 	    refcount: AtomicI32,
> >>> 	    data: UnsafeCell<T>
> >>> 	}
> >>>
> >>> but with atomic generic, people can leave that option open and do:
> >>>
> >>> 	struct Refcount<R, T> {
> >>> 	    refcount: Atomic<R>,
> >>> 	    data: UnsafeCell<T>
> >>> 	}
> >>>
> >>> while it provides configurable options for experienced users, but it
> >>> also provides opportunities for sub-optimal types, e.g. Refcount<u8, T>:
> >>> on ll/sc architectures, because `data` and `refcount` can be in the same
> >>> machine-word, the accesses of `refcount` are affected by the accesses of
> >>> `data`.
> >>
> >> I think this is a non-issue. We have two options of counteracting this:
> >> 1. We can just point this out in reviews and force people to use
> >>    `Atomic<T>` with a concrete type. In cases where there really is the
> >>    need to be generic, we can have it.
> >> 2. We can add a private trait in the bounds for the generic, nobody
> >>    outside of the module can access it and thus they need to use a
> >>    concrete type:
> >>
> >>         // needs a better name
> >>         trait Integer {}
> >>         impl Integer for i32 {}
> >>         impl Integer for i64 {}
> >>
> >>         pub struct Atomic<T: Integer> {
> >>             /* ... */
> >>         }
> >>
> >> And then in the other module, you can't do this (with compiler error):
> >>
> >>         pub struct Refcount<R: Integer, T> {
> >>                             // ^^^^^^^ not found in this scope
> >>                             // note: trait `crate::atomic::Integer` exists but is inaccessible
> >>             refcount: Atomic<R>,
> >>             data: UnsafeCell<T>,
> >>         }
> >>
> >> I think that we can start with approach 2 and if we find a use-case
> >> where generics are really unavoidable, we can either put it in the same
> >> module as `Atomic<T>`, or change the access of `Integer`.
> >>
> > 
> > What's the issue of having AtomicI32 and AtomicI64 first then? We don't
> > need to do 1 or 2 until the real users show up.
> 
> Generics allow you to avoid code duplication (I don't think that you
> want to create the `Atomic{I32,I64}` types via macros...). We would have
> to do a lot of refactoring, when we want to introduce it. I don't see

You can simply do

	type AtomicI32=Atomic<i32>;

Plus, we always do refactoring in kernel, because it's impossible to get
everything right at the first time. TBH, it's too confident to think one
can.

> the harm of introducing generics from the get-go.
> 
> > And I'd like also to point out that there are a few more trait bound
> > designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
> > have different sets of API (no inc_unless_negative() for u32).
> 
> Sure, just like Gary said, you can just do:
> 
>     impl Atomic<i32> {
>         pub fn inc_unless_negative(&self, ordering: Ordering) -> bool;
>     }
> 
> Or add a `HasNegative` trait.
> 
> > Don't make me wrong, I have no doubt we can handle this in the type
> > system, but given the design work need, won't it make sense that we take
> > baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
> > already have real users, and then if there are some values of generic
> > atomics, we introduce them and have proper discussion on design.
> 
> I don't understand this point, why can't we put in the effort for a good
> design? AFAIK we normally spend considerable time to get the API right
> and I think in this case it would include making it generic.
> 

What's the design you propose here? Well, the conversation between us is
only the design bit I saw, elsewhere it's all handwaving that "generics
are overall really good". I'm happy to get the API right, and it's easy
and simple to do on concrete types. But IIUC, Gary's suggestion is to
only have Atomic<i32> and Atomic<i64> first, and do the design later,
which I really don't like. It may not be a complete design, but I need
to see the design now to understand whether we need to go to that
direction. I cannot just introduce a TBD generic.

Regards,
Boqun

> > To me, it's perfectly fine that Atomic{I32,I64} co-exist with Atomic<T>.
> > What's the downside? A bit specific example would help me understand
> > the real concern here.
> 
> I don't like that, why have two ways of doing the same thing? People
> will be confused whether they should use `AtomicI32` vs `Atomic<i32>`...
> 
> ---
> Cheers,
> Benno
> 

Re: [RFC 2/2] rust: sync: Add atomic support

[Benno Lossin]

On 16.06.24 16:08, Boqun Feng wrote:
> On Sun, Jun 16, 2024 at 09:46:45AM +0000, Benno Lossin wrote:
>> On 16.06.24 00:12, Boqun Feng wrote:
>>> On Sat, Jun 15, 2024 at 07:09:30AM +0000, Benno Lossin wrote:
>>>> On 15.06.24 03:33, Boqun Feng wrote:
>>>>> On Fri, Jun 14, 2024 at 09:22:24PM +0000, Benno Lossin wrote:
>>>>>> On 14.06.24 16:33, Boqun Feng wrote:
>>>>>>> On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
>>>>>>>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> Does this make sense?
>>>>>>>>
>>>>>>>> Implementation-wise, if you think it is simpler or more clear/elegant
>>>>>>>> to have the extra lower level layer, then that sounds fine.
>>>>>>>>
>>>>>>>> However, I was mainly talking about what we would eventually expose to
>>>>>>>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
>>>>>>>
>>>>>>> The truth is I don't know ;-) I don't have much data on which one is
>>>>>>> better. Personally, I think AtomicI32 and AtomicI64 make the users have
>>>>>>> to think about size, alignment, etc, and I think that's important for
>>>>>>> atomic users and people who review their code, because before one uses
>>>>>>> atomics, one should ask themselves: why don't I use a lock? Atomics
>>>>>>> provide the ablities to do low level stuffs and when doing low level
>>>>>>> stuffs, you want to be more explicit than ergonomic.
>>>>>>
>>>>>> How would this be different with `Atomic<i32>` and `Atomic<i64>`? Just
>>>>>
>>>>> The difference is that with Atomic{I32,I64} APIs, one has to choose (and
>>>>> think about) the size when using atomics, and cannot leave that option
>>>>> open. It's somewhere unconvenient, but as I said, atomics variables are
>>>>> different. For example, if someone is going to implement a reference
>>>>> counter struct, they can define as follow:
>>>>>
>>>>> 	struct Refcount<T> {
>>>>> 	    refcount: AtomicI32,
>>>>> 	    data: UnsafeCell<T>
>>>>> 	}
>>>>>
>>>>> but with atomic generic, people can leave that option open and do:
>>>>>
>>>>> 	struct Refcount<R, T> {
>>>>> 	    refcount: Atomic<R>,
>>>>> 	    data: UnsafeCell<T>
>>>>> 	}
>>>>>
>>>>> while it provides configurable options for experienced users, but it
>>>>> also provides opportunities for sub-optimal types, e.g. Refcount<u8, T>:
>>>>> on ll/sc architectures, because `data` and `refcount` can be in the same
>>>>> machine-word, the accesses of `refcount` are affected by the accesses of
>>>>> `data`.
>>>>
>>>> I think this is a non-issue. We have two options of counteracting this:
>>>> 1. We can just point this out in reviews and force people to use
>>>>    `Atomic<T>` with a concrete type. In cases where there really is the
>>>>    need to be generic, we can have it.
>>>> 2. We can add a private trait in the bounds for the generic, nobody
>>>>    outside of the module can access it and thus they need to use a
>>>>    concrete type:
>>>>
>>>>         // needs a better name
>>>>         trait Integer {}
>>>>         impl Integer for i32 {}
>>>>         impl Integer for i64 {}
>>>>
>>>>         pub struct Atomic<T: Integer> {
>>>>             /* ... */
>>>>         }
>>>>
>>>> And then in the other module, you can't do this (with compiler error):
>>>>
>>>>         pub struct Refcount<R: Integer, T> {
>>>>                             // ^^^^^^^ not found in this scope
>>>>                             // note: trait `crate::atomic::Integer` exists but is inaccessible
>>>>             refcount: Atomic<R>,
>>>>             data: UnsafeCell<T>,
>>>>         }
>>>>
>>>> I think that we can start with approach 2 and if we find a use-case
>>>> where generics are really unavoidable, we can either put it in the same
>>>> module as `Atomic<T>`, or change the access of `Integer`.
>>>>
>>>
>>> What's the issue of having AtomicI32 and AtomicI64 first then? We don't
>>> need to do 1 or 2 until the real users show up.
>>
>> Generics allow you to avoid code duplication (I don't think that you
>> want to create the `Atomic{I32,I64}` types via macros...). We would have
>> to do a lot of refactoring, when we want to introduce it. I don't see
> 
> You can simply do
> 
> 	type AtomicI32=Atomic<i32>;

Eh, I would think that we could just do a text replacement in this case.
Or if that doesn't work, Coccinelle should be able to do this...

> Plus, we always do refactoring in kernel, because it's impossible to get
> everything right at the first time. TBH, it's too confident to think one
> can.

I don't think that we're at the "let's just put it in" stage. This is an
RFC version, so it should be fine to completely change the approach.
I agree, that we can't get it 100% right the first time, but we should
at least strive to get a good version.

>> the harm of introducing generics from the get-go.
>>
>>> And I'd like also to point out that there are a few more trait bound
>>> designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
>>> have different sets of API (no inc_unless_negative() for u32).
>>
>> Sure, just like Gary said, you can just do:
>>
>>     impl Atomic<i32> {
>>         pub fn inc_unless_negative(&self, ordering: Ordering) -> bool;
>>     }
>>
>> Or add a `HasNegative` trait.
>>
>>> Don't make me wrong, I have no doubt we can handle this in the type
>>> system, but given the design work need, won't it make sense that we take
>>> baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
>>> already have real users, and then if there are some values of generic
>>> atomics, we introduce them and have proper discussion on design.
>>
>> I don't understand this point, why can't we put in the effort for a good
>> design? AFAIK we normally spend considerable time to get the API right
>> and I think in this case it would include making it generic.
>>
> 
> What's the design you propose here? Well, the conversation between us is
> only the design bit I saw, elsewhere it's all handwaving that "generics
> are overall really good". I'm happy to get the API right, and it's easy
> and simple to do on concrete types. But IIUC, Gary's suggestion is to
> only have Atomic<i32> and Atomic<i64> first, and do the design later,
> which I really don't like. It may not be a complete design, but I need
> to see the design now to understand whether we need to go to that
> direction. I cannot just introduce a TBD generic.

I don't think that the idea was to "do the design later". I don't even
know how you would do that, since you need the design to submit a patch.

I can't offer you a complete API description, since that would require
me writing it up myself. But I would recommend trying to get it to work
with generics. I got a few other comments:
- I don't think that we should resort to a script to generate the Rust
  code since it prevents adding good documentation & examples to the
  various methods. AFAIU you want to generate the functions from
  `scripts/atomic/atomics.tbl` to keep it in sync with the C side. I
  looked at the git log of that file and it hasn't been changed
  significantly since its inception. I don't think that there is any
  benefit to generating the functions from that file.
- most of the documented functions say "See `c_function`", I don't like
  this, can we either copy the C documentation (I imagine it not
  changing that often, or is that assumption wrong?) or write our own?
- we should try to use either const generic or normal parameters for the
  access ordering instead of putting it in the function name.
- why do we need both non-return and return variants?

I think it is probably a good idea to discuss this in our meeting.

---
Cheers,
Benno

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 03:06:36PM +0000, Benno Lossin wrote:
> On 16.06.24 16:08, Boqun Feng wrote:
> > On Sun, Jun 16, 2024 at 09:46:45AM +0000, Benno Lossin wrote:
> >> On 16.06.24 00:12, Boqun Feng wrote:
> >>> On Sat, Jun 15, 2024 at 07:09:30AM +0000, Benno Lossin wrote:
> >>>> On 15.06.24 03:33, Boqun Feng wrote:
> >>>>> On Fri, Jun 14, 2024 at 09:22:24PM +0000, Benno Lossin wrote:
> >>>>>> On 14.06.24 16:33, Boqun Feng wrote:
> >>>>>>> On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
> >>>>>>>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >>>>>>>>>
> >>>>>>>>> Does this make sense?
> >>>>>>>>
> >>>>>>>> Implementation-wise, if you think it is simpler or more clear/elegant
> >>>>>>>> to have the extra lower level layer, then that sounds fine.
> >>>>>>>>
> >>>>>>>> However, I was mainly talking about what we would eventually expose to
> >>>>>>>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> >>>>>>>
> >>>>>>> The truth is I don't know ;-) I don't have much data on which one is
> >>>>>>> better. Personally, I think AtomicI32 and AtomicI64 make the users have
> >>>>>>> to think about size, alignment, etc, and I think that's important for
> >>>>>>> atomic users and people who review their code, because before one uses
> >>>>>>> atomics, one should ask themselves: why don't I use a lock? Atomics
> >>>>>>> provide the ablities to do low level stuffs and when doing low level
> >>>>>>> stuffs, you want to be more explicit than ergonomic.
> >>>>>>
> >>>>>> How would this be different with `Atomic<i32>` and `Atomic<i64>`? Just
> >>>>>
> >>>>> The difference is that with Atomic{I32,I64} APIs, one has to choose (and
> >>>>> think about) the size when using atomics, and cannot leave that option
> >>>>> open. It's somewhere unconvenient, but as I said, atomics variables are
> >>>>> different. For example, if someone is going to implement a reference
> >>>>> counter struct, they can define as follow:
> >>>>>
> >>>>> 	struct Refcount<T> {
> >>>>> 	    refcount: AtomicI32,
> >>>>> 	    data: UnsafeCell<T>
> >>>>> 	}
> >>>>>
> >>>>> but with atomic generic, people can leave that option open and do:
> >>>>>
> >>>>> 	struct Refcount<R, T> {
> >>>>> 	    refcount: Atomic<R>,
> >>>>> 	    data: UnsafeCell<T>
> >>>>> 	}
> >>>>>
> >>>>> while it provides configurable options for experienced users, but it
> >>>>> also provides opportunities for sub-optimal types, e.g. Refcount<u8, T>:
> >>>>> on ll/sc architectures, because `data` and `refcount` can be in the same
> >>>>> machine-word, the accesses of `refcount` are affected by the accesses of
> >>>>> `data`.
> >>>>
> >>>> I think this is a non-issue. We have two options of counteracting this:
> >>>> 1. We can just point this out in reviews and force people to use
> >>>>    `Atomic<T>` with a concrete type. In cases where there really is the
> >>>>    need to be generic, we can have it.
> >>>> 2. We can add a private trait in the bounds for the generic, nobody
> >>>>    outside of the module can access it and thus they need to use a
> >>>>    concrete type:
> >>>>
> >>>>         // needs a better name
> >>>>         trait Integer {}
> >>>>         impl Integer for i32 {}
> >>>>         impl Integer for i64 {}
> >>>>
> >>>>         pub struct Atomic<T: Integer> {
> >>>>             /* ... */
> >>>>         }
> >>>>
> >>>> And then in the other module, you can't do this (with compiler error):
> >>>>
> >>>>         pub struct Refcount<R: Integer, T> {
> >>>>                             // ^^^^^^^ not found in this scope
> >>>>                             // note: trait `crate::atomic::Integer` exists but is inaccessible
> >>>>             refcount: Atomic<R>,
> >>>>             data: UnsafeCell<T>,
> >>>>         }
> >>>>
> >>>> I think that we can start with approach 2 and if we find a use-case
> >>>> where generics are really unavoidable, we can either put it in the same
> >>>> module as `Atomic<T>`, or change the access of `Integer`.
> >>>>
> >>>
> >>> What's the issue of having AtomicI32 and AtomicI64 first then? We don't
> >>> need to do 1 or 2 until the real users show up.
> >>
> >> Generics allow you to avoid code duplication (I don't think that you
> >> want to create the `Atomic{I32,I64}` types via macros...). We would have
> >> to do a lot of refactoring, when we want to introduce it. I don't see
> > 
> > You can simply do
> > 
> > 	type AtomicI32=Atomic<i32>;
> 
> Eh, I would think that we could just do a text replacement in this case.
> Or if that doesn't work, Coccinelle should be able to do this...
> 
> > Plus, we always do refactoring in kernel, because it's impossible to get
> > everything right at the first time. TBH, it's too confident to think one
> > can.
> 
> I don't think that we're at the "let's just put it in" stage. This is an
> RFC version, so it should be fine to completely change the approach.

I'm fine as well. I wasn't trying to rush anything, but as I mentioned
below, I need some more design from people who want it to understand
whether that's a good idea.

> I agree, that we can't get it 100% right the first time, but we should
> at least strive to get a good version.
> 
> >> the harm of introducing generics from the get-go.
> >>
> >>> And I'd like also to point out that there are a few more trait bound
> >>> designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
> >>> have different sets of API (no inc_unless_negative() for u32).
> >>
> >> Sure, just like Gary said, you can just do:
> >>
> >>     impl Atomic<i32> {
> >>         pub fn inc_unless_negative(&self, ordering: Ordering) -> bool;
> >>     }
> >>
> >> Or add a `HasNegative` trait.
> >>
> >>> Don't make me wrong, I have no doubt we can handle this in the type
> >>> system, but given the design work need, won't it make sense that we take
> >>> baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
> >>> already have real users, and then if there are some values of generic
> >>> atomics, we introduce them and have proper discussion on design.
> >>
> >> I don't understand this point, why can't we put in the effort for a good
> >> design? AFAIK we normally spend considerable time to get the API right
> >> and I think in this case it would include making it generic.
> >>
> > 
> > What's the design you propose here? Well, the conversation between us is
> > only the design bit I saw, elsewhere it's all handwaving that "generics
> > are overall really good". I'm happy to get the API right, and it's easy
> > and simple to do on concrete types. But IIUC, Gary's suggestion is to
> > only have Atomic<i32> and Atomic<i64> first, and do the design later,
> > which I really don't like. It may not be a complete design, but I need
> > to see the design now to understand whether we need to go to that
> > direction. I cannot just introduce a TBD generic.
> 
> I don't think that the idea was to "do the design later". I don't even
> know how you would do that, since you need the design to submit a patch.
> 

Then I might mis-understand Gary? He said:

"Can we avoid two types and use a generic `Atomic<T>` and then implement
on `Atomic<i32>` and `Atomic<i64>` instead?"

, which means just replace `impl AtomicI32` with `impl Atomic<i32>` to
me.

> I can't offer you a complete API description, since that would require
> me writing it up myself. But I would recommend trying to get it to work
> with generics. I got a few other comments:

We should work on things that are concrete, right? It's fine that the
design is not complete, and it's fine if you just recommend. But without
a somewhat concrete design (doesn't have to be complete), I cannot be
sure about whether we have the same vision of the future of generic
atomics (see my question to Gary), that's a bit hard for me to try to
work something out (plus I personally don't think it's a good idea, it's
OK to me, but not good). Anyway, I wasn't trying to refuse to do this
just based on personal reasons, but I do need to understand what you are
all proposing, because I don't have one myself.

> - I don't think that we should resort to a script to generate the Rust
>   code since it prevents adding good documentation & examples to the
>   various methods. AFAIU you want to generate the functions from
>   `scripts/atomic/atomics.tbl` to keep it in sync with the C side. I
>   looked at the git log of that file and it hasn't been changed
>   significantly since its inception. I don't think that there is any
>   benefit to generating the functions from that file.

I'll leave this to other atomic maintainers.

> - most of the documented functions say "See `c_function`", I don't like
>   this, can we either copy the C documentation (I imagine it not
>   changing that often, or is that assumption wrong?) or write our own?

You're not wrong. AN in C side, we do have some documentation template
to generate the comments (see scripts/atomic/kerneldoc). But first the
format is for doxygen(I guess?), and second as you just bring up, the
templates are tied with the bash script.

> - we should try to use either const generic or normal parameters for the
>   access ordering instead of putting it in the function name.

Why is it important? Keeping it in the current way brings the value that
it's not too much different than their C counterparts. Could you explain
a bit the pros and cons on suffix vs const generic approach?

> - why do we need both non-return and return variants?
> 

Historical reason I guess. Plus maybe some architectures have a better
implementation on non-return atomics IIRC.

Regards,
Boqun

> I think it is probably a good idea to discuss this in our meeting.
> 
> ---
> Cheers,
> Benno
> 

Re: [RFC 2/2] rust: sync: Add atomic support

[Benno Lossin]

On 16.06.24 17:34, Boqun Feng wrote:
> On Sun, Jun 16, 2024 at 03:06:36PM +0000, Benno Lossin wrote:
>> On 16.06.24 16:08, Boqun Feng wrote:
>>> On Sun, Jun 16, 2024 at 09:46:45AM +0000, Benno Lossin wrote:
>>>> On 16.06.24 00:12, Boqun Feng wrote:
>>>>> On Sat, Jun 15, 2024 at 07:09:30AM +0000, Benno Lossin wrote:
>>>>>> On 15.06.24 03:33, Boqun Feng wrote:
>>>>>>> On Fri, Jun 14, 2024 at 09:22:24PM +0000, Benno Lossin wrote:
>>>>>>>> On 14.06.24 16:33, Boqun Feng wrote:
>>>>>>>>> On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
>>>>>>>>>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Does this make sense?
>>>>>>>>>>
>>>>>>>>>> Implementation-wise, if you think it is simpler or more clear/elegant
>>>>>>>>>> to have the extra lower level layer, then that sounds fine.
>>>>>>>>>>
>>>>>>>>>> However, I was mainly talking about what we would eventually expose to
>>>>>>>>>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
>>>>>>>>>
>>>>>>>>> The truth is I don't know ;-) I don't have much data on which one is
>>>>>>>>> better. Personally, I think AtomicI32 and AtomicI64 make the users have
>>>>>>>>> to think about size, alignment, etc, and I think that's important for
>>>>>>>>> atomic users and people who review their code, because before one uses
>>>>>>>>> atomics, one should ask themselves: why don't I use a lock? Atomics
>>>>>>>>> provide the ablities to do low level stuffs and when doing low level
>>>>>>>>> stuffs, you want to be more explicit than ergonomic.
>>>>>>>>
>>>>>>>> How would this be different with `Atomic<i32>` and `Atomic<i64>`? Just
>>>>>>>
>>>>>>> The difference is that with Atomic{I32,I64} APIs, one has to choose (and
>>>>>>> think about) the size when using atomics, and cannot leave that option
>>>>>>> open. It's somewhere unconvenient, but as I said, atomics variables are
>>>>>>> different. For example, if someone is going to implement a reference
>>>>>>> counter struct, they can define as follow:
>>>>>>>
>>>>>>> 	struct Refcount<T> {
>>>>>>> 	    refcount: AtomicI32,
>>>>>>> 	    data: UnsafeCell<T>
>>>>>>> 	}
>>>>>>>
>>>>>>> but with atomic generic, people can leave that option open and do:
>>>>>>>
>>>>>>> 	struct Refcount<R, T> {
>>>>>>> 	    refcount: Atomic<R>,
>>>>>>> 	    data: UnsafeCell<T>
>>>>>>> 	}
>>>>>>>
>>>>>>> while it provides configurable options for experienced users, but it
>>>>>>> also provides opportunities for sub-optimal types, e.g. Refcount<u8, T>:
>>>>>>> on ll/sc architectures, because `data` and `refcount` can be in the same
>>>>>>> machine-word, the accesses of `refcount` are affected by the accesses of
>>>>>>> `data`.
>>>>>>
>>>>>> I think this is a non-issue. We have two options of counteracting this:
>>>>>> 1. We can just point this out in reviews and force people to use
>>>>>>    `Atomic<T>` with a concrete type. In cases where there really is the
>>>>>>    need to be generic, we can have it.
>>>>>> 2. We can add a private trait in the bounds for the generic, nobody
>>>>>>    outside of the module can access it and thus they need to use a
>>>>>>    concrete type:
>>>>>>
>>>>>>         // needs a better name
>>>>>>         trait Integer {}
>>>>>>         impl Integer for i32 {}
>>>>>>         impl Integer for i64 {}
>>>>>>
>>>>>>         pub struct Atomic<T: Integer> {
>>>>>>             /* ... */
>>>>>>         }
>>>>>>
>>>>>> And then in the other module, you can't do this (with compiler error):
>>>>>>
>>>>>>         pub struct Refcount<R: Integer, T> {
>>>>>>                             // ^^^^^^^ not found in this scope
>>>>>>                             // note: trait `crate::atomic::Integer` exists but is inaccessible
>>>>>>             refcount: Atomic<R>,
>>>>>>             data: UnsafeCell<T>,
>>>>>>         }
>>>>>>
>>>>>> I think that we can start with approach 2 and if we find a use-case
>>>>>> where generics are really unavoidable, we can either put it in the same
>>>>>> module as `Atomic<T>`, or change the access of `Integer`.
>>>>>>
>>>>>
>>>>> What's the issue of having AtomicI32 and AtomicI64 first then? We don't
>>>>> need to do 1 or 2 until the real users show up.
>>>>
>>>> Generics allow you to avoid code duplication (I don't think that you
>>>> want to create the `Atomic{I32,I64}` types via macros...). We would have
>>>> to do a lot of refactoring, when we want to introduce it. I don't see
>>>
>>> You can simply do
>>>
>>> 	type AtomicI32=Atomic<i32>;
>>
>> Eh, I would think that we could just do a text replacement in this case.
>> Or if that doesn't work, Coccinelle should be able to do this...
>>
>>> Plus, we always do refactoring in kernel, because it's impossible to get
>>> everything right at the first time. TBH, it's too confident to think one
>>> can.
>>
>> I don't think that we're at the "let's just put it in" stage. This is an
>> RFC version, so it should be fine to completely change the approach.
> 
> I'm fine as well. I wasn't trying to rush anything, but as I mentioned
> below, I need some more design from people who want it to understand
> whether that's a good idea.
> 
>> I agree, that we can't get it 100% right the first time, but we should
>> at least strive to get a good version.
>>
>>>> the harm of introducing generics from the get-go.
>>>>
>>>>> And I'd like also to point out that there are a few more trait bound
>>>>> designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
>>>>> have different sets of API (no inc_unless_negative() for u32).
>>>>
>>>> Sure, just like Gary said, you can just do:
>>>>
>>>>     impl Atomic<i32> {
>>>>         pub fn inc_unless_negative(&self, ordering: Ordering) -> bool;
>>>>     }
>>>>
>>>> Or add a `HasNegative` trait.
>>>>
>>>>> Don't make me wrong, I have no doubt we can handle this in the type
>>>>> system, but given the design work need, won't it make sense that we take
>>>>> baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
>>>>> already have real users, and then if there are some values of generic
>>>>> atomics, we introduce them and have proper discussion on design.
>>>>
>>>> I don't understand this point, why can't we put in the effort for a good
>>>> design? AFAIK we normally spend considerable time to get the API right
>>>> and I think in this case it would include making it generic.
>>>>
>>>
>>> What's the design you propose here? Well, the conversation between us is
>>> only the design bit I saw, elsewhere it's all handwaving that "generics
>>> are overall really good". I'm happy to get the API right, and it's easy
>>> and simple to do on concrete types. But IIUC, Gary's suggestion is to
>>> only have Atomic<i32> and Atomic<i64> first, and do the design later,
>>> which I really don't like. It may not be a complete design, but I need
>>> to see the design now to understand whether we need to go to that
>>> direction. I cannot just introduce a TBD generic.
>>
>> I don't think that the idea was to "do the design later". I don't even
>> know how you would do that, since you need the design to submit a patch.
>>
> 
> Then I might mis-understand Gary? He said:
> 
> "Can we avoid two types and use a generic `Atomic<T>` and then implement
> on `Atomic<i32>` and `Atomic<i64>` instead?"
> 
> , which means just replace `impl AtomicI32` with `impl Atomic<i32>` to
> me.

This is a fair interpretation, but what prevents you from merging the
impls of functions that can be? I assumed that you would do that
automatically.

>> I can't offer you a complete API description, since that would require
>> me writing it up myself. But I would recommend trying to get it to work
>> with generics. I got a few other comments:
> 
> We should work on things that are concrete, right? It's fine that the
> design is not complete, and it's fine if you just recommend. But without
> a somewhat concrete design (doesn't have to be complete), I cannot be
> sure about whether we have the same vision of the future of generic
> atomics (see my question to Gary), that's a bit hard for me to try to

Sorry, which question?
Also to be aligned on the vision, I think we should rather talk about
the vision and not the design, since the design that we want to have now
can be different from the vision. On that note, what do you envision the
future of the atomic API?

> work something out (plus I personally don't think it's a good idea, it's
> OK to me, but not good). Anyway, I wasn't trying to refuse to do this
> just based on personal reasons, but I do need to understand what you are
> all proposing, because I don't have one myself.

I went back through the thread and here is what I think your argument
against generics is: people should think about size and alignment when
using atomics, which is problematic when allowing users to leave the
atomic generic.
But as I argued before, this is not an issue. Have I overlooked another
argument? Because I don't see anything else.

>> - I don't think that we should resort to a script to generate the Rust
>>   code since it prevents adding good documentation & examples to the
>>   various methods. AFAIU you want to generate the functions from
>>   `scripts/atomic/atomics.tbl` to keep it in sync with the C side. I
>>   looked at the git log of that file and it hasn't been changed
>>   significantly since its inception. I don't think that there is any
>>   benefit to generating the functions from that file.
> 
> I'll leave this to other atomic maintainers.
> 
>> - most of the documented functions say "See `c_function`", I don't like
>>   this, can we either copy the C documentation (I imagine it not
>>   changing that often, or is that assumption wrong?) or write our own?
> 
> You're not wrong. AN in C side, we do have some documentation template
> to generate the comments (see scripts/atomic/kerneldoc). But first the
> format is for doxygen(I guess?), and second as you just bring up, the
> templates are tied with the bash script.

I see a bash script similarly to how Wedson sees proc macros ;)
We should try *hard* to avoid them and only use them when there is no
other way.

>> - we should try to use either const generic or normal parameters for the
>>   access ordering instead of putting it in the function name.
> 
> Why is it important? Keeping it in the current way brings the value that
> it's not too much different than their C counterparts. Could you explain
> a bit the pros and cons on suffix vs const generic approach?

Reduce code duplication, instead of 3 different variants, we can have
one. It allows people to build ergonomic APIs that allows the user to
decide which synchronization they use under the hood.

>> - why do we need both non-return and return variants?
>>
> 
> Historical reason I guess. Plus maybe some architectures have a better
> implementation on non-return atomics IIRC.

Could we get some more concrete arguments for why we would need these in
rust? If the reason is purely historical, then we shouldn't expose the
non-return variant IMO. If it is because of performance, then we can
only expose them in the respective arches.

---
Cheers,
Benno

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 03:55:12PM +0000, Benno Lossin wrote:
[...]
> >>
> >> I don't think that the idea was to "do the design later". I don't even
> >> know how you would do that, since you need the design to submit a patch.
> >>
> > 
> > Then I might mis-understand Gary? He said:
> > 
> > "Can we avoid two types and use a generic `Atomic<T>` and then implement
> > on `Atomic<i32>` and `Atomic<i64>` instead?"
> > 
> > , which means just replace `impl AtomicI32` with `impl Atomic<i32>` to
> > me.
> 
> This is a fair interpretation, but what prevents you from merging the
> impls of functions that can be? I assumed that you would do that
> automatically.
> 

I think you missed the point, Gary's suggestion at that time sounds
like: let's impl Atomic<i32> and Atomic<i64> first, and leave rest of
the work for later, that is a "do the design later" to me.

> >> I can't offer you a complete API description, since that would require
> >> me writing it up myself. But I would recommend trying to get it to work
> >> with generics. I got a few other comments:
> > 
> > We should work on things that are concrete, right? It's fine that the
> > design is not complete, and it's fine if you just recommend. But without
> > a somewhat concrete design (doesn't have to be complete), I cannot be
> > sure about whether we have the same vision of the future of generic
> > atomics (see my question to Gary), that's a bit hard for me to try to
> 
> Sorry, which question?

	https://lore.kernel.org/rust-for-linux/Zm7_XWe6ciy1yN-h@Boquns-Mac-mini.home/

> Also to be aligned on the vision, I think we should rather talk about
> the vision and not the design, since the design that we want to have now
> can be different from the vision. On that note, what do you envision the
> future of the atomic API?
> 

Mine is simple, just providing AtomicI32 and AtomicI64 first, since
there are immediate users right now, and should we learn more needs from
the users, we get more idea about what a good API is, and we evolve from
there.

> > work something out (plus I personally don't think it's a good idea, it's
> > OK to me, but not good). Anyway, I wasn't trying to refuse to do this
> > just based on personal reasons, but I do need to understand what you are
> > all proposing, because I don't have one myself.
> 
> I went back through the thread and here is what I think your argument
> against generics is: people should think about size and alignment when
> using atomics, which is problematic when allowing users to leave the
> atomic generic.
> But as I argued before, this is not an issue. Have I overlooked another

You mean you said it's a non-issue but gave me two counteract? If it's
really a non-issue, you won't need a couneraction, right? In other words
non-generic atomics do provide some value.

> argument? Because I don't see anything else.
> 
> >> - I don't think that we should resort to a script to generate the Rust
> >>   code since it prevents adding good documentation & examples to the
> >>   various methods. AFAIU you want to generate the functions from
> >>   `scripts/atomic/atomics.tbl` to keep it in sync with the C side. I
> >>   looked at the git log of that file and it hasn't been changed
> >>   significantly since its inception. I don't think that there is any
> >>   benefit to generating the functions from that file.
> > 
> > I'll leave this to other atomic maintainers.
> > 
> >> - most of the documented functions say "See `c_function`", I don't like
> >>   this, can we either copy the C documentation (I imagine it not
> >>   changing that often, or is that assumption wrong?) or write our own?
> > 
> > You're not wrong. AN in C side, we do have some documentation template
> > to generate the comments (see scripts/atomic/kerneldoc). But first the
> > format is for doxygen(I guess?), and second as you just bring up, the
> > templates are tied with the bash script.
> 
> I see a bash script similarly to how Wedson sees proc macros ;)
> We should try *hard* to avoid them and only use them when there is no
> other way.
> 

I will just start with the existing mechanism and try to evolve, whether
it's a script or proc macro, I don't mind, I want to get the work done
and most relevant people can understand in the way the they prefer and
step-by-step, move it to the place I think is the best for the project.

> >> - we should try to use either const generic or normal parameters for the
> >>   access ordering instead of putting it in the function name.
> > 
> > Why is it important? Keeping it in the current way brings the value that
> > it's not too much different than their C counterparts. Could you explain
> > a bit the pros and cons on suffix vs const generic approach?
> 
> Reduce code duplication, instead of 3 different variants, we can have
> one. It allows people to build ergonomic APIs that allows the user to
> decide which synchronization they use under the hood.
> 

I already mentioned why I think it's good in the current way, I will
defer it to others on their inputs.

> >> - why do we need both non-return and return variants?
> >>
> > 
> > Historical reason I guess. Plus maybe some architectures have a better
> > implementation on non-return atomics IIRC.
> 
> Could we get some more concrete arguments for why we would need these in
> rust? If the reason is purely historical, then we shouldn't expose the

Sure. Look like my memory is correct, at least on ARM64 they are
different instructions (see arch/arm64/include/asm/atomic_lse.h)

non-return atomics on ARM64:

	#define ATOMIC_OP(op, asm_op)						\
	static __always_inline void						\
	__lse_atomic_##op(int i, atomic_t *v)					\
	{									\
		asm volatile(							\
		__LSE_PREAMBLE							\
		"	" #asm_op "	%w[i], %[v]\n"				\
		: [v] "+Q" (v->counter)						\
		: [i] "r" (i));							\
	}

value-return atomics on ARM64:

	#define ATOMIC_FETCH_OP(name, mb, op, asm_op, cl...)			\
	static __always_inline int						\
	__lse_atomic_fetch_##op##name(int i, atomic_t *v)			\
	{									\
		int old;							\
										\
		asm volatile(							\
		__LSE_PREAMBLE							\
		"	" #asm_op #mb "	%w[i], %w[old], %[v]"			\
		: [v] "+Q" (v->counter),					\
		  [old] "=r" (old)						\
		: [i] "r" (i)							\
		: cl);								\
										\
		return old;							\
	}

It may not be easy to see the different instructions from the pasted
code, but you can find them in the header file, also you could notice
that the number of operands is different?

> non-return variant IMO. If it is because of performance, then we can
> only expose them in the respective arches.
> 

Hmm.. so we ask user to write arch-specific code like:

	pub fn increase_counter(&self) {
	    #[cfg(CONFIG_ARM64)]
	    self.counter.inc();

	    #[cfg(CONFIG_X86_64)]
	    let _ = self.counter.inc_return();
	}

are you sure it's a good idea?

Regards,
Boqun

> ---
> Cheers,
> Benno
> 

Re: [RFC 2/2] rust: sync: Add atomic support

[Benno Lossin]

On 16.06.24 18:30, Boqun Feng wrote:
> On Sun, Jun 16, 2024 at 03:55:12PM +0000, Benno Lossin wrote:
> [...]
>>>>
>>>> I don't think that the idea was to "do the design later". I don't even
>>>> know how you would do that, since you need the design to submit a patch.
>>>>
>>>
>>> Then I might mis-understand Gary? He said:
>>>
>>> "Can we avoid two types and use a generic `Atomic<T>` and then implement
>>> on `Atomic<i32>` and `Atomic<i64>` instead?"
>>>
>>> , which means just replace `impl AtomicI32` with `impl Atomic<i32>` to
>>> me.
>>
>> This is a fair interpretation, but what prevents you from merging the
>> impls of functions that can be? I assumed that you would do that
>> automatically.
>>
> 
> I think you missed the point, Gary's suggestion at that time sounds
> like: let's impl Atomic<i32> and Atomic<i64> first, and leave rest of
> the work for later, that is a "do the design later" to me.

Hmm, but wouldn't that just be less work for you?

>>>> I can't offer you a complete API description, since that would require
>>>> me writing it up myself. But I would recommend trying to get it to work
>>>> with generics. I got a few other comments:
>>>
>>> We should work on things that are concrete, right? It's fine that the
>>> design is not complete, and it's fine if you just recommend. But without
>>> a somewhat concrete design (doesn't have to be complete), I cannot be
>>> sure about whether we have the same vision of the future of generic
>>> atomics (see my question to Gary), that's a bit hard for me to try to
>>
>> Sorry, which question?
> 
> 	https://lore.kernel.org/rust-for-linux/Zm7_XWe6ciy1yN-h@Boquns-Mac-mini.home/
> 
>> Also to be aligned on the vision, I think we should rather talk about
>> the vision and not the design, since the design that we want to have now
>> can be different from the vision. On that note, what do you envision the
>> future of the atomic API?
>>
> 
> Mine is simple, just providing AtomicI32 and AtomicI64 first, since
> there are immediate users right now, and should we learn more needs from
> the users, we get more idea about what a good API is, and we evolve from
> there.

That is fine, but since we want to get generics in the future anyways, I
think it would be good to already implement them now to also gather
feedback on them.

>>> work something out (plus I personally don't think it's a good idea, it's
>>> OK to me, but not good). Anyway, I wasn't trying to refuse to do this
>>> just based on personal reasons, but I do need to understand what you are
>>> all proposing, because I don't have one myself.
>>
>> I went back through the thread and here is what I think your argument
>> against generics is: people should think about size and alignment when
>> using atomics, which is problematic when allowing users to leave the
>> atomic generic.
>> But as I argued before, this is not an issue. Have I overlooked another
> 
> You mean you said it's a non-issue but gave me two counteract? If it's
> really a non-issue, you won't need a couneraction, right? In other words
> non-generic atomics do provide some value.

The second counteractions would provide exactly the same API surface as
your non-generic version, so I don't see how going non-generic provides
any value over going generic.
The first approach was intended for a future in which we are not scared
of people using generic atomics in their structs. I don't know if we are
going to be in that future, so I think we should go with the second
approach for the time being.

>> argument? Because I don't see anything else.
>>
>>>> - I don't think that we should resort to a script to generate the Rust
>>>>   code since it prevents adding good documentation & examples to the
>>>>   various methods. AFAIU you want to generate the functions from
>>>>   `scripts/atomic/atomics.tbl` to keep it in sync with the C side. I
>>>>   looked at the git log of that file and it hasn't been changed
>>>>   significantly since its inception. I don't think that there is any
>>>>   benefit to generating the functions from that file.
>>>
>>> I'll leave this to other atomic maintainers.
>>>
>>>> - most of the documented functions say "See `c_function`", I don't like
>>>>   this, can we either copy the C documentation (I imagine it not
>>>>   changing that often, or is that assumption wrong?) or write our own?
>>>
>>> You're not wrong. AN in C side, we do have some documentation template
>>> to generate the comments (see scripts/atomic/kerneldoc). But first the
>>> format is for doxygen(I guess?), and second as you just bring up, the
>>> templates are tied with the bash script.
>>
>> I see a bash script similarly to how Wedson sees proc macros ;)
>> We should try *hard* to avoid them and only use them when there is no
>> other way.
>>
> 
> I will just start with the existing mechanism and try to evolve, whether
> it's a script or proc macro, I don't mind, I want to get the work done
> and most relevant people can understand in the way the they prefer and
> step-by-step, move it to the place I think is the best for the project.

I don't think that we need a script or a proc macro. A few declarative
macros probably suffice if we go the way of generics.

>>>> - we should try to use either const generic or normal parameters for the
>>>>   access ordering instead of putting it in the function name.
>>>
>>> Why is it important? Keeping it in the current way brings the value that
>>> it's not too much different than their C counterparts. Could you explain
>>> a bit the pros and cons on suffix vs const generic approach?
>>
>> Reduce code duplication, instead of 3 different variants, we can have
>> one. It allows people to build ergonomic APIs that allows the user to
>> decide which synchronization they use under the hood.
>>
> 
> I already mentioned why I think it's good in the current way, I will
> defer it to others on their inputs.
> 
>>>> - why do we need both non-return and return variants?
>>>>
>>>
>>> Historical reason I guess. Plus maybe some architectures have a better
>>> implementation on non-return atomics IIRC.
>>
>> Could we get some more concrete arguments for why we would need these in
>> rust? If the reason is purely historical, then we shouldn't expose the
> 
> Sure. Look like my memory is correct, at least on ARM64 they are
> different instructions (see arch/arm64/include/asm/atomic_lse.h)
> 
> non-return atomics on ARM64:
> 
> 	#define ATOMIC_OP(op, asm_op)						\
> 	static __always_inline void						\
> 	__lse_atomic_##op(int i, atomic_t *v)					\
> 	{									\
> 		asm volatile(							\
> 		__LSE_PREAMBLE							\
> 		"	" #asm_op "	%w[i], %[v]\n"				\
> 		: [v] "+Q" (v->counter)						\
> 		: [i] "r" (i));							\
> 	}
> 
> value-return atomics on ARM64:
> 
> 	#define ATOMIC_FETCH_OP(name, mb, op, asm_op, cl...)			\
> 	static __always_inline int						\
> 	__lse_atomic_fetch_##op##name(int i, atomic_t *v)			\
> 	{									\
> 		int old;							\
> 										\
> 		asm volatile(							\
> 		__LSE_PREAMBLE							\
> 		"	" #asm_op #mb "	%w[i], %w[old], %[v]"			\
> 		: [v] "+Q" (v->counter),					\
> 		  [old] "=r" (old)						\
> 		: [i] "r" (i)							\
> 		: cl);								\
> 										\
> 		return old;							\
> 	}
> 
> It may not be easy to see the different instructions from the pasted
> code, but you can find them in the header file, also you could notice
> that the number of operands is different?

This is not my expertise, so I believe you :)

>> non-return variant IMO. If it is because of performance, then we can
>> only expose them in the respective arches.
>>
> 
> Hmm.. so we ask user to write arch-specific code like:
> 
> 	pub fn increase_counter(&self) {
> 	    #[cfg(CONFIG_ARM64)]
> 	    self.counter.inc();
> 
> 	    #[cfg(CONFIG_X86_64)]
> 	    let _ = self.counter.inc_return();
> 	}
> 
> are you sure it's a good idea?

No that looks horrible.

Maybe there is something that we can do with generics, but I don't know
if it is worth it. I guess we can leave that open for the time being.

---
Cheers,
Benno

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Wed, Jun 19, 2024 at 09:09:46AM +0000, Benno Lossin wrote:
[...]
> >>> Then I might mis-understand Gary? He said:
> >>>
> >>> "Can we avoid two types and use a generic `Atomic<T>` and then implement
> >>> on `Atomic<i32>` and `Atomic<i64>` instead?"
> >>>
> >>> , which means just replace `impl AtomicI32` with `impl Atomic<i32>` to
> >>> me.
> >>
> >> This is a fair interpretation, but what prevents you from merging the
> >> impls of functions that can be? I assumed that you would do that
> >> automatically.
> >>
> > 
> > I think you missed the point, Gary's suggestion at that time sounds
> > like: let's impl Atomic<i32> and Atomic<i64> first, and leave rest of
> > the work for later, that is a "do the design later" to me.
> 
> Hmm, but wouldn't that just be less work for you?
> 

Not really ;-) Committing to a generic interface without a vision/design
is going to be a maintenace nightmare. I will need to deal the soundness
issues and different ideas about what can be put in <..>. So I'd suggest
we take great caution before we make our decision, and I may even ask
for a formal proof of the soundness of a generic interface for key
components such as atomics. Because last time I check, safety is what
Rust brings on the table, right?

Someone may say that non generic atomics is one of the biggest mistakes
that Rust ever made. But I disagree. In fact when I saw this, my first
impression was "these guys know their stuff": there are only a few types
and operations that make sense, so putting them in a generic interface
would be over-engineer (at least at the beginning). It's better correct
than convenient/popular/aesthetical.

So would you and Gary be interested at working on such a design and
proof? Maybe someone else Cced would also be interested. Eventually we
will need the tool of soundness proof for other types as well, that's
going to be very vaulable. And having that would be the real "less work
for me" ;-)

Right now, I still plan to do in the current way (i.e. non-generic
atomcis for i32, i64, isize) because there are already users [1], 
the correctness is easy to confirm and we won't confuse users with
half-baked generic design. But should we have a sound Atomic<T> design,
I'm happy to adopt it ;-)

[...]
> >> Also to be aligned on the vision, I think we should rather talk about
> >> the vision and not the design, since the design that we want to have now
> >> can be different from the vision. On that note, what do you envision the
> >> future of the atomic API?
> >>
> > 
> > Mine is simple, just providing AtomicI32 and AtomicI64 first, since
> > there are immediate users right now, and should we learn more needs from
> > the users, we get more idea about what a good API is, and we evolve from
> > there.
> 
> That is fine, but since we want to get generics in the future anyways, I
> think it would be good to already implement them now to also gather
> feedback on them.
> 
[...]
> > 
> > You mean you said it's a non-issue but gave me two counteract? If it's
> > really a non-issue, you won't need a couneraction, right? In other words
> > non-generic atomics do provide some value.
> 
> The second counteractions would provide exactly the same API surface as
> your non-generic version, so I don't see how going non-generic provides
> any value over going generic.
> The first approach was intended for a future in which we are not scared
> of people using generic atomics in their structs. I don't know if we are
> going to be in that future, so I think we should go with the second
> approach for the time being.
> 

Not going to reply the above right now, because I'm leaning more
torwards switching when we have a sound Atomic<T> design, but I want you
to know I appreciate your input and explanation.

> >> argument? Because I don't see anything else.
> >>
> >>>> - I don't think that we should resort to a script to generate the Rust
> >>>>   code since it prevents adding good documentation & examples to the
> >>>>   various methods. AFAIU you want to generate the functions from
> >>>>   `scripts/atomic/atomics.tbl` to keep it in sync with the C side. I
> >>>>   looked at the git log of that file and it hasn't been changed
> >>>>   significantly since its inception. I don't think that there is any
> >>>>   benefit to generating the functions from that file.
> >>>
> >>> I'll leave this to other atomic maintainers.
> >>>
> >>>> - most of the documented functions say "See `c_function`", I don't like
> >>>>   this, can we either copy the C documentation (I imagine it not
> >>>>   changing that often, or is that assumption wrong?) or write our own?
> >>>
> >>> You're not wrong. AN in C side, we do have some documentation template
> >>> to generate the comments (see scripts/atomic/kerneldoc). But first the
> >>> format is for doxygen(I guess?), and second as you just bring up, the
> >>> templates are tied with the bash script.
> >>
> >> I see a bash script similarly to how Wedson sees proc macros ;)
> >> We should try *hard* to avoid them and only use them when there is no
> >> other way.
> >>
> > 
> > I will just start with the existing mechanism and try to evolve, whether
> > it's a script or proc macro, I don't mind, I want to get the work done
> > and most relevant people can understand in the way the they prefer and
> > step-by-step, move it to the place I think is the best for the project.
> 
> I don't think that we need a script or a proc macro. A few declarative
> macros probably suffice if we go the way of generics.
> 

Ah right. And even without generics we could use some declarative
macros, and I'm happy to take a look at this and provide the alternative
approach so we can choose a better one.

[1]: https://lore.kernel.org/rust-for-linux/20240611114551.228679-2-nmi@metaspace.dk/

Regards,
Boqun

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 09:46:45AM +0000, Benno Lossin wrote:
[...]
> > To me, it's perfectly fine that Atomic{I32,I64} co-exist with Atomic<T>.
> > What's the downside? A bit specific example would help me understand
> > the real concern here.
> 
> I don't like that, why have two ways of doing the same thing? People
> will be confused whether they should use `AtomicI32` vs `Atomic<i32>`...
> 

BTW, we already have something similar like this in kernel, we have
SpinLock<T> and Lock<T, SpinLockBackend>, how should we do about this?

Regards,
Boqun

> ---
> Cheers,
> Benno
> 

Re: [RFC 2/2] rust: sync: Add atomic support

[Kent Overstreet]

On Sat, Jun 15, 2024 at 03:12:33PM -0700, Boqun Feng wrote:
> What's the issue of having AtomicI32 and AtomicI64 first then? We don't
> need to do 1 or 2 until the real users show up.
> 
> And I'd like also to point out that there are a few more trait bound
> designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
> have different sets of API (no inc_unless_negative() for u32).
> 
> Don't make me wrong, I have no doubt we can handle this in the type
> system, but given the design work need, won't it make sense that we take
> baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
> already have real users, and then if there are some values of generic
> atomics, we introduce them and have proper discussion on design.
> 
> To me, it's perfectly fine that Atomic{I32,I64} co-exist with Atomic<T>.
> What's the downside? A bit specific example would help me understand
> the real concern here.

Err, what?

Of course we want generic atomics, and we need that for properly
supporting cmpxchg.

Bogun, you've got all the rust guys pushing for doing this with
generics, I'm not sure why you're being stubborn here?

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 05:51:07AM -0400, Kent Overstreet wrote:
> On Sat, Jun 15, 2024 at 03:12:33PM -0700, Boqun Feng wrote:
> > What's the issue of having AtomicI32 and AtomicI64 first then? We don't
> > need to do 1 or 2 until the real users show up.
> > 
> > And I'd like also to point out that there are a few more trait bound
> > designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
> > have different sets of API (no inc_unless_negative() for u32).
> > 
> > Don't make me wrong, I have no doubt we can handle this in the type
> > system, but given the design work need, won't it make sense that we take
> > baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
> > already have real users, and then if there are some values of generic
> > atomics, we introduce them and have proper discussion on design.
> > 
> > To me, it's perfectly fine that Atomic{I32,I64} co-exist with Atomic<T>.
> > What's the downside? A bit specific example would help me understand
> > the real concern here.
> 
> Err, what?
> 
> Of course we want generic atomics, and we need that for properly
> supporting cmpxchg.
> 

Nope. Note this series only introduces the atomic types (atomic_ C
APIs), but cmpxchg C APIs (no atomic_ prefix) are probably presented via
a different API, where we need to make it easier to interact with normal
types, and we may use generic there.

> Bogun, you've got all the rust guys pushing for doing this with
> generics, I'm not sure why you're being stubborn here?

Hmm? Have you seen the email I replied to John, a broader Rust community
seems doesn't appreciate the idea of generic atomics.

Regards,
Boqun

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 07:16:30AM -0700, Boqun Feng wrote:
> On Sun, Jun 16, 2024 at 05:51:07AM -0400, Kent Overstreet wrote:
> > On Sat, Jun 15, 2024 at 03:12:33PM -0700, Boqun Feng wrote:
> > > What's the issue of having AtomicI32 and AtomicI64 first then? We don't
> > > need to do 1 or 2 until the real users show up.
> > > 
> > > And I'd like also to point out that there are a few more trait bound
> > > designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
> > > have different sets of API (no inc_unless_negative() for u32).
> > > 
> > > Don't make me wrong, I have no doubt we can handle this in the type
> > > system, but given the design work need, won't it make sense that we take
> > > baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
> > > already have real users, and then if there are some values of generic
> > > atomics, we introduce them and have proper discussion on design.
> > > 
> > > To me, it's perfectly fine that Atomic{I32,I64} co-exist with Atomic<T>.
> > > What's the downside? A bit specific example would help me understand
> > > the real concern here.
> > 
> > Err, what?
> > 
> > Of course we want generic atomics, and we need that for properly
> > supporting cmpxchg.
> > 
> 
> Nope. Note this series only introduces the atomic types (atomic_ C
> APIs), but cmpxchg C APIs (no atomic_ prefix) are probably presented via
> a different API, where we need to make it easier to interact with normal
> types, and we may use generic there.
> 

Or it could be a generic function instead of generic type like:

	pub unsafe fn cmpxchg<T>(ptr: * mut T, old: T, new T) -> T

the "unsafe" part is due to `ptr` may not be a valid pointer or this may
make normal accesses data race.

Regards,
Boqun

> > Bogun, you've got all the rust guys pushing for doing this with
> > generics, I'm not sure why you're being stubborn here?
> 
> Hmm? Have you seen the email I replied to John, a broader Rust community
> seems doesn't appreciate the idea of generic atomics.
> 
> Regards,
> Boqun

Re: [RFC 2/2] rust: sync: Add atomic support

[Miguel Ojeda]

On Sun, Jun 16, 2024 at 4:16 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Hmm? Have you seen the email I replied to John, a broader Rust community
> seems doesn't appreciate the idea of generic atomics.

I don't think we can easily draw that conclusion from those download
numbers / dependent crates.

portable-atomic may be more popular simply because it provides
features for platforms the standard library does not. The interface
being generic or not may have nothing to do with it. Or perhaps
because it has a 1.x version, while the other doesn't, etc.

In fact, the atomic crate is essentially about providing `Atomic<T>`,
so one could argue that all those downloads are precisely from people
that want a generic atomic.

Moreover, I noticed portable-atomic's issue #1 in GitHub is,
precisely, adding `Atomic<T>` support. The maintainer has a PR for
that updated over time, most recently a few hours ago.

There is also `AtomicCell<T>` from crossbeam, which is the first
feature listed in its docs.

Anyway...

The way I see it, both approaches seem similar (i.e. for what we are
going to use them for today, at least) and neither apparently has a
major downside today for those use cases (apart from needed refactors
later to go to another approach).

(By the "generic approach", by the way, I mean just providing
`Atomic<{i32,i64}>`, not a complex design)

So it is up to you on what you send for the non-RFC patches, of
course, and if nobody has the time / wants to do the work for the
"simple" generic approach, then we can just go ahead with this for the
moment. But I think it would be nice to at least consider the "simple"
generic approach to see how much worse it would be.

Other bits to consider, that perhaps give you arguments for one or the
other: consequences on the compilation time, on inlining, on the error
messages for new users, on the generated documentation, on how easy to
grep they are, etc.

Cheers,
Miguel

Re: [RFC 2/2] rust: sync: Add atomic support

[Kent Overstreet]

On Sun, Jun 16, 2024 at 05:14:56PM +0200, Miguel Ojeda wrote:
> On Sun, Jun 16, 2024 at 4:16 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Hmm? Have you seen the email I replied to John, a broader Rust community
> > seems doesn't appreciate the idea of generic atomics.
> 
> I don't think we can easily draw that conclusion from those download
> numbers / dependent crates.
> 
> portable-atomic may be more popular simply because it provides
> features for platforms the standard library does not. The interface
> being generic or not may have nothing to do with it. Or perhaps
> because it has a 1.x version, while the other doesn't, etc.
> 
> In fact, the atomic crate is essentially about providing `Atomic<T>`,
> so one could argue that all those downloads are precisely from people
> that want a generic atomic.
> 
> Moreover, I noticed portable-atomic's issue #1 in GitHub is,
> precisely, adding `Atomic<T>` support. The maintainer has a PR for
> that updated over time, most recently a few hours ago.
> 
> There is also `AtomicCell<T>` from crossbeam, which is the first
> feature listed in its docs.
> 
> Anyway...
> 
> The way I see it, both approaches seem similar (i.e. for what we are
> going to use them for today, at least) and neither apparently has a
> major downside today for those use cases (apart from needed refactors
> later to go to another approach).
> 
> (By the "generic approach", by the way, I mean just providing
> `Atomic<{i32,i64}>`, not a complex design)
> 
> So it is up to you on what you send for the non-RFC patches, of
> course, and if nobody has the time / wants to do the work for the
> "simple" generic approach, then we can just go ahead with this for the
> moment. But I think it would be nice to at least consider the "simple"
> generic approach to see how much worse it would be.
> 
> Other bits to consider, that perhaps give you arguments for one or the
> other: consequences on the compilation time, on inlining, on the error
> messages for new users, on the generated documentation, on how easy to
> grep they are, etc.

Yeah, rereading the thread - I'm with Miguel and Gary.

Generics are simply the correct way to do it, if the wider rust
community didn't do it that way I think that can be chalked up more to
historical baggage or needlessly copying the base integer type scheme.

Let's please do it right here, and generics are the correct approach.

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 11:32:31AM -0400, Kent Overstreet wrote:
> On Sun, Jun 16, 2024 at 05:14:56PM +0200, Miguel Ojeda wrote:
> > On Sun, Jun 16, 2024 at 4:16 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > Hmm? Have you seen the email I replied to John, a broader Rust community
> > > seems doesn't appreciate the idea of generic atomics.
> > 
> > I don't think we can easily draw that conclusion from those download
> > numbers / dependent crates.
> > 
> > portable-atomic may be more popular simply because it provides
> > features for platforms the standard library does not. The interface
> > being generic or not may have nothing to do with it. Or perhaps
> > because it has a 1.x version, while the other doesn't, etc.
> > 
> > In fact, the atomic crate is essentially about providing `Atomic<T>`,
> > so one could argue that all those downloads are precisely from people
> > that want a generic atomic.
> > 
> > Moreover, I noticed portable-atomic's issue #1 in GitHub is,
> > precisely, adding `Atomic<T>` support. The maintainer has a PR for
> > that updated over time, most recently a few hours ago.
> > 
> > There is also `AtomicCell<T>` from crossbeam, which is the first
> > feature listed in its docs.
> > 
> > Anyway...
> > 
> > The way I see it, both approaches seem similar (i.e. for what we are
> > going to use them for today, at least) and neither apparently has a
> > major downside today for those use cases (apart from needed refactors
> > later to go to another approach).
> > 
> > (By the "generic approach", by the way, I mean just providing
> > `Atomic<{i32,i64}>`, not a complex design)
> > 
> > So it is up to you on what you send for the non-RFC patches, of
> > course, and if nobody has the time / wants to do the work for the
> > "simple" generic approach, then we can just go ahead with this for the
> > moment. But I think it would be nice to at least consider the "simple"
> > generic approach to see how much worse it would be.
> > 
> > Other bits to consider, that perhaps give you arguments for one or the
> > other: consequences on the compilation time, on inlining, on the error
> > messages for new users, on the generated documentation, on how easy to
> > grep they are, etc.
> 
> Yeah, rereading the thread - I'm with Miguel and Gary.
> 
> Generics are simply the correct way to do it, if the wider rust
> community didn't do it that way I think that can be chalked up more to
> historical baggage or needlessly copying the base integer type scheme.
> 
> Let's please do it right here, and generics are the correct approach.

If so, maybe we should do u<Wide> instead of u8, u16, oh, and probably
just Integer<Sign, Wide> instead of i{8,16,32,64) and u{8,16,32,64} ;-)

Regards,
Boqun

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 08:54:09AM -0700, Boqun Feng wrote:
> On Sun, Jun 16, 2024 at 11:32:31AM -0400, Kent Overstreet wrote:
> > On Sun, Jun 16, 2024 at 05:14:56PM +0200, Miguel Ojeda wrote:
> > > On Sun, Jun 16, 2024 at 4:16 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > >
> > > > Hmm? Have you seen the email I replied to John, a broader Rust community
> > > > seems doesn't appreciate the idea of generic atomics.
> > > 
> > > I don't think we can easily draw that conclusion from those download
> > > numbers / dependent crates.
> > > 
> > > portable-atomic may be more popular simply because it provides
> > > features for platforms the standard library does not. The interface
> > > being generic or not may have nothing to do with it. Or perhaps
> > > because it has a 1.x version, while the other doesn't, etc.
> > > 
> > > In fact, the atomic crate is essentially about providing `Atomic<T>`,
> > > so one could argue that all those downloads are precisely from people
> > > that want a generic atomic.
> > > 
> > > Moreover, I noticed portable-atomic's issue #1 in GitHub is,
> > > precisely, adding `Atomic<T>` support. The maintainer has a PR for
> > > that updated over time, most recently a few hours ago.
> > > 
> > > There is also `AtomicCell<T>` from crossbeam, which is the first
> > > feature listed in its docs.
> > > 
> > > Anyway...
> > > 
> > > The way I see it, both approaches seem similar (i.e. for what we are
> > > going to use them for today, at least) and neither apparently has a
> > > major downside today for those use cases (apart from needed refactors
> > > later to go to another approach).
> > > 
> > > (By the "generic approach", by the way, I mean just providing
> > > `Atomic<{i32,i64}>`, not a complex design)
> > > 
> > > So it is up to you on what you send for the non-RFC patches, of
> > > course, and if nobody has the time / wants to do the work for the
> > > "simple" generic approach, then we can just go ahead with this for the
> > > moment. But I think it would be nice to at least consider the "simple"
> > > generic approach to see how much worse it would be.
> > > 
> > > Other bits to consider, that perhaps give you arguments for one or the
> > > other: consequences on the compilation time, on inlining, on the error
> > > messages for new users, on the generated documentation, on how easy to
> > > grep they are, etc.
> > 
> > Yeah, rereading the thread - I'm with Miguel and Gary.
> > 
> > Generics are simply the correct way to do it, if the wider rust
> > community didn't do it that way I think that can be chalked up more to
> > historical baggage or needlessly copying the base integer type scheme.
> > 
> > Let's please do it right here, and generics are the correct approach.
> 

I think the disagreement here is not non-generic atomic vs generic
atomic, it's pure generic atomic vs. AtomicI{32,64} etc + generic
atomic. I said multiple times that I'm OK with generic atomics if there
are real users, just I'm not sure it's something we want to do right now
(or we have enough information to go fully on that direction). And I
think it's fine to have non-generic atomic and generic atomic coexist.

Regards,
Boqun

> If so, maybe we should do u<Wide> instead of u8, u16, oh, and probably
> just Integer<Sign, Wide> instead of i{8,16,32,64) and u{8,16,32,64} ;-)
> 
> Regards,
> Boqun

Re: [RFC 2/2] rust: sync: Add atomic support

[Kent Overstreet]

On Sun, Jun 16, 2024 at 10:30:03AM -0700, Boqun Feng wrote:
> I think the disagreement here is not non-generic atomic vs generic
> atomic, it's pure generic atomic vs. AtomicI{32,64} etc + generic
> atomic. I said multiple times that I'm OK with generic atomics if there
> are real users, just I'm not sure it's something we want to do right now
> (or we have enough information to go fully on that direction). And I
> think it's fine to have non-generic atomic and generic atomic coexist.

Well, having the generic interface from the start does matter, that's
what we (myself included) want to code to.

No need to overcomplicate it, just
Atomic<u8>
Atomic<u16>

etc...

As long as that's available, the internal implementation shouldn't have
to change.

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 05:14:56PM +0200, Miguel Ojeda wrote:
> On Sun, Jun 16, 2024 at 4:16 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Hmm? Have you seen the email I replied to John, a broader Rust community
> > seems doesn't appreciate the idea of generic atomics.
> 
> I don't think we can easily draw that conclusion from those download
> numbers / dependent crates.
> 
> portable-atomic may be more popular simply because it provides
> features for platforms the standard library does not. The interface
> being generic or not may have nothing to do with it. Or perhaps
> because it has a 1.x version, while the other doesn't, etc.
> 

Totally agreed, but that was the information all I could get at that
time ;-)

> In fact, the atomic crate is essentially about providing `Atomic<T>`,
> so one could argue that all those downloads are precisely from people
> that want a generic atomic.
> 
> Moreover, I noticed portable-atomic's issue #1 in GitHub is,
> precisely, adding `Atomic<T>` support. The maintainer has a PR for
> that updated over time, most recently a few hours ago.
> 

Wait! Could it be because of me? Or I'm thinking too much about myself
;-)

> There is also `AtomicCell<T>` from crossbeam, which is the first
> feature listed in its docs.
> 
> Anyway...
> 
> The way I see it, both approaches seem similar (i.e. for what we are
> going to use them for today, at least) and neither apparently has a
> major downside today for those use cases (apart from needed refactors
> later to go to another approach).
> 
> (By the "generic approach", by the way, I mean just providing
> `Atomic<{i32,i64}>`, not a complex design)
> 
> So it is up to you on what you send for the non-RFC patches, of
> course, and if nobody has the time / wants to do the work for the
> "simple" generic approach, then we can just go ahead with this for the
> moment. But I think it would be nice to at least consider the "simple"
> generic approach to see how much worse it would be.
> 

What would work for me is a somewhat concrete design consensus (on what
sizes we are going to support, the expectation on how many traits we are
going to introduce, etc.) And then a simple generic approach is not a
problem. (But I remain my right to say "I told you so" ;-)) As I said,
we cannot just do generic because of generic, we have to have at least
some idea abou the things we are generify on (the scope and the cost in
term of how many more traits people need to understand).

> Other bits to consider, that perhaps give you arguments for one or the
> other: consequences on the compilation time, on inlining, on the error
> messages for new users, on the generated documentation, on how easy to
> grep they are, etc.
> 

These seem non-issues to me (except the grep part), but I'm relatively
more familiar with atomics and Rust, so it's good to hear others
thought, or wait the feedback until we have the patchset to review.

Regards,
Boqun

> Cheers,
> Miguel

Re: [RFC 2/2] rust: sync: Add atomic support

[Kent Overstreet]

On Sun, Jun 16, 2024 at 07:16:30AM -0700, Boqun Feng wrote:
> On Sun, Jun 16, 2024 at 05:51:07AM -0400, Kent Overstreet wrote:
> > On Sat, Jun 15, 2024 at 03:12:33PM -0700, Boqun Feng wrote:
> > > What's the issue of having AtomicI32 and AtomicI64 first then? We don't
> > > need to do 1 or 2 until the real users show up.
> > > 
> > > And I'd like also to point out that there are a few more trait bound
> > > designs needed for Atomic<T>, for example, Atomic<u32> and Atomic<i32>
> > > have different sets of API (no inc_unless_negative() for u32).
> > > 
> > > Don't make me wrong, I have no doubt we can handle this in the type
> > > system, but given the design work need, won't it make sense that we take
> > > baby steps on this? We can first introduce AtomicI32 and AtomicI64 which
> > > already have real users, and then if there are some values of generic
> > > atomics, we introduce them and have proper discussion on design.
> > > 
> > > To me, it's perfectly fine that Atomic{I32,I64} co-exist with Atomic<T>.
> > > What's the downside? A bit specific example would help me understand
> > > the real concern here.
> > 
> > Err, what?
> > 
> > Of course we want generic atomics, and we need that for properly
> > supporting cmpxchg.
> > 
> 
> Nope. Note this series only introduces the atomic types (atomic_ C
> APIs), but cmpxchg C APIs (no atomic_ prefix) are probably presented via
> a different API, where we need to make it easier to interact with normal
> types, and we may use generic there.
> 
> > Bogun, you've got all the rust guys pushing for doing this with
> > generics, I'm not sure why you're being stubborn here?
> 
> Hmm? Have you seen the email I replied to John, a broader Rust community
> seems doesn't appreciate the idea of generic atomics.

Apologies, I appear to have gotten things backwards in my pre-coffee
reading, I'll have to catch up on the whole thread :)

Re: [RFC 2/2] rust: sync: Add atomic support

[John Hubbard]

On 6/14/24 2:59 AM, Miguel Ojeda wrote:
> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>>
>> Does this make sense?
> 
> Implementation-wise, if you think it is simpler or more clear/elegant
> to have the extra lower level layer, then that sounds fine.
> 
> However, I was mainly talking about what we would eventually expose to
> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> then we could make the lower layer private already.
> 
> We can defer that extra layer/work if needed even if we go for
> `Atomic<T>`, but it would be nice to understand if we have consensus
> for an eventual user-facing API, or if someone has any other opinion
> or concerns on one vs. the other.

Well, here's one:

The reason that we have things like atomic64_read() in the C code is
because C doesn't have generics.

In Rust, we should simply move directly to Atomic<T>, as there are,
after all, associated benefits. And it's very easy to see the connection
between the C types and the Atomic<T> types.


thanks,
-- 
John Hubbard
NVIDIA

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Fri, Jun 14, 2024 at 06:03:37PM -0700, John Hubbard wrote:
> On 6/14/24 2:59 AM, Miguel Ojeda wrote:
> > On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > 
> > > Does this make sense?
> > 
> > Implementation-wise, if you think it is simpler or more clear/elegant
> > to have the extra lower level layer, then that sounds fine.
> > 
> > However, I was mainly talking about what we would eventually expose to
> > users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> > then we could make the lower layer private already.
> > 
> > We can defer that extra layer/work if needed even if we go for
> > `Atomic<T>`, but it would be nice to understand if we have consensus
> > for an eventual user-facing API, or if someone has any other opinion
> > or concerns on one vs. the other.
> 
> Well, here's one:
> 
> The reason that we have things like atomic64_read() in the C code is
> because C doesn't have generics.
> 
> In Rust, we should simply move directly to Atomic<T>, as there are,
> after all, associated benefits. And it's very easy to see the connection

What are the associated benefits you are referring to? Rust std doesn't
use Atomic<T>, that somewhat proves that we don't need it.

Regards,
Boqun

> between the C types and the Atomic<T> types.
> 
> 
> thanks,
> -- 
> John Hubbard
> NVIDIA
> 

Re: [RFC 2/2] rust: sync: Add atomic support

[John Hubbard]

On 6/14/24 6:24 PM, Boqun Feng wrote:
> On Fri, Jun 14, 2024 at 06:03:37PM -0700, John Hubbard wrote:
>> On 6/14/24 2:59 AM, Miguel Ojeda wrote:
>>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>>>>
>>>> Does this make sense?
>>>
>>> Implementation-wise, if you think it is simpler or more clear/elegant
>>> to have the extra lower level layer, then that sounds fine.
>>>
>>> However, I was mainly talking about what we would eventually expose to
>>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
>>> then we could make the lower layer private already.
>>>
>>> We can defer that extra layer/work if needed even if we go for
>>> `Atomic<T>`, but it would be nice to understand if we have consensus
>>> for an eventual user-facing API, or if someone has any other opinion
>>> or concerns on one vs. the other.
>>
>> Well, here's one:
>>
>> The reason that we have things like atomic64_read() in the C code is
>> because C doesn't have generics.
>>
>> In Rust, we should simply move directly to Atomic<T>, as there are,
>> after all, associated benefits. And it's very easy to see the connection
> 
> What are the associated benefits you are referring to? Rust std doesn't
> use Atomic<T>, that somewhat proves that we don't need it.
  
Just the stock things that a generic provides: less duplicated code,
automatic support for future types (although here it's really just
integer types we care about of course).


thanks,
-- 
John Hubbard
NVIDIA

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Fri, Jun 14, 2024 at 06:28:00PM -0700, John Hubbard wrote:
> On 6/14/24 6:24 PM, Boqun Feng wrote:
> > On Fri, Jun 14, 2024 at 06:03:37PM -0700, John Hubbard wrote:
> > > On 6/14/24 2:59 AM, Miguel Ojeda wrote:
> > > > On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > > > 
> > > > > Does this make sense?
> > > > 
> > > > Implementation-wise, if you think it is simpler or more clear/elegant
> > > > to have the extra lower level layer, then that sounds fine.
> > > > 
> > > > However, I was mainly talking about what we would eventually expose to
> > > > users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> > > > then we could make the lower layer private already.
> > > > 
> > > > We can defer that extra layer/work if needed even if we go for
> > > > `Atomic<T>`, but it would be nice to understand if we have consensus
> > > > for an eventual user-facing API, or if someone has any other opinion
> > > > or concerns on one vs. the other.
> > > 
> > > Well, here's one:
> > > 
> > > The reason that we have things like atomic64_read() in the C code is
> > > because C doesn't have generics.
> > > 
> > > In Rust, we should simply move directly to Atomic<T>, as there are,
> > > after all, associated benefits. And it's very easy to see the connection
> > 
> > What are the associated benefits you are referring to? Rust std doesn't
> > use Atomic<T>, that somewhat proves that we don't need it.
> Just the stock things that a generic provides: less duplicated code,

It's still a bit handwavy, sorry.

Admittedly, I haven't looked into too much Rust concurrent code, maybe
it's even true for C code ;-) So I took a look at the crate that Gary
mentioned (the one provides generic atomic APIs):

	https://crates.io/crates/atomic

there's a "Dependent" tab where you can see the other crates that
depends on it. With a quick look, I haven't found any Rust concurrent
project I'm aware of (no crossbeam, no tokio, no futures). On the other
hand, there is a non-generic based atomic library:

	https://crates.io/crates/portable-atomic

which has more projects depend on it, and there are some Rust concurrent
projects that I'm aware of: futures, async-task etc. Note that people
can get the non-generic based atomic API from Rust std library, and
the "portable-atomic" crate is only 2-year old, while "atomic" crate is
8-year old.

More interestingly, the same author of "atomic" crate, who is an expert
in concurrent areas, has another project (there are a lot projects from
the author, but this is the one I'm mostly aware of) "parking_lot",
which "provides implementations of Mutex, RwLock, Condvar and Once that
are smaller, faster and more flexible than those in the Rust standard
library, as well as a ReentrantMutex type which supports recursive
locking.", and it doesn't use the "atomic" crate either.

These data could mean nothing, there are multiple reasons affecting the
popularity of a library. But all the above seems to suggests that you
don't really need generic on atomic, at least for a lot of meaningful
concurent code.


So if we were to make a decision right now, I don't see that generic
atomics are winning. Of course, as I said previously, we can always add
them if we have learned more and have the consensus.


(Don't make me wrong, I love generic in general, I just want to avoid
the "I have a generic hammer and everything looks like generic nails"
situation.)

Regards,
Boqun

> automatic support for future types (although here it's really just
> integer types we care about of course).
> 
> 
> thanks,
> -- 
> John Hubbard
> NVIDIA
> 

Re: [RFC 2/2] rust: sync: Add atomic support

[John Hubbard]

On 6/14/24 7:39 PM, Boqun Feng wrote:
> On Fri, Jun 14, 2024 at 06:28:00PM -0700, John Hubbard wrote:
>> On 6/14/24 6:24 PM, Boqun Feng wrote:
>>> On Fri, Jun 14, 2024 at 06:03:37PM -0700, John Hubbard wrote:
>>>> On 6/14/24 2:59 AM, Miguel Ojeda wrote:
>>>>> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>>>>>>
>>>>>> Does this make sense?
>>>>>
>>>>> Implementation-wise, if you think it is simpler or more clear/elegant
>>>>> to have the extra lower level layer, then that sounds fine.
>>>>>
>>>>> However, I was mainly talking about what we would eventually expose to
>>>>> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
>>>>> then we could make the lower layer private already.
>>>>>
>>>>> We can defer that extra layer/work if needed even if we go for
>>>>> `Atomic<T>`, but it would be nice to understand if we have consensus
>>>>> for an eventual user-facing API, or if someone has any other opinion
>>>>> or concerns on one vs. the other.
>>>>
>>>> Well, here's one:
>>>>
>>>> The reason that we have things like atomic64_read() in the C code is
>>>> because C doesn't have generics.
>>>>
>>>> In Rust, we should simply move directly to Atomic<T>, as there are,
>>>> after all, associated benefits. And it's very easy to see the connection
>>>
>>> What are the associated benefits you are referring to? Rust std doesn't
>>> use Atomic<T>, that somewhat proves that we don't need it.
>> Just the stock things that a generic provides: less duplicated code,
> 
> It's still a bit handwavy, sorry.
> 
> Admittedly, I haven't looked into too much Rust concurrent code, maybe
> it's even true for C code ;-) So I took a look at the crate that Gary
> mentioned (the one provides generic atomic APIs):
> 
> 	https://crates.io/crates/atomic
> 
> there's a "Dependent" tab where you can see the other crates that
> depends on it. With a quick look, I haven't found any Rust concurrent
> project I'm aware of (no crossbeam, no tokio, no futures). On the other
> hand, there is a non-generic based atomic library:
> 
> 	https://crates.io/crates/portable-atomic
> 
> which has more projects depend on it, and there are some Rust concurrent
> projects that I'm aware of: futures, async-task etc. Note that people
> can get the non-generic based atomic API from Rust std library, and
> the "portable-atomic" crate is only 2-year old, while "atomic" crate is
> 8-year old.
> 
> More interestingly, the same author of "atomic" crate, who is an expert
> in concurrent areas, has another project (there are a lot projects from
> the author, but this is the one I'm mostly aware of) "parking_lot",
> which "provides implementations of Mutex, RwLock, Condvar and Once that
> are smaller, faster and more flexible than those in the Rust standard
> library, as well as a ReentrantMutex type which supports recursive
> locking.", and it doesn't use the "atomic" crate either.
> 
> These data could mean nothing, there are multiple reasons affecting the
> popularity of a library. But all the above seems to suggests that you
> don't really need generic on atomic, at least for a lot of meaningful
> concurent code.
> 
> 
> So if we were to make a decision right now, I don't see that generic
> atomics are winning. Of course, as I said previously, we can always add

That does seem to be the case: the non-generic flavor looks more
popular so far.

> them if we have learned more and have the consensus.

Yes, I suppose waiting might be better. I expected the Atomic<T> to
be more popular than it actually is...


thanks,
-- 
John Hubbard
NVIDIA

Re: [RFC 2/2] rust: sync: Add atomic support

[Gary Guo]

On Fri, 14 Jun 2024 19:39:27 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Fri, Jun 14, 2024 at 06:28:00PM -0700, John Hubbard wrote:
> > On 6/14/24 6:24 PM, Boqun Feng wrote:  
> > > On Fri, Jun 14, 2024 at 06:03:37PM -0700, John Hubbard wrote:  
> > > > On 6/14/24 2:59 AM, Miguel Ojeda wrote:  
> > > > > On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:  
> > > > > > 
> > > > > > Does this make sense?  
> > > > > 
> > > > > Implementation-wise, if you think it is simpler or more clear/elegant
> > > > > to have the extra lower level layer, then that sounds fine.
> > > > > 
> > > > > However, I was mainly talking about what we would eventually expose to
> > > > > users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> > > > > then we could make the lower layer private already.
> > > > > 
> > > > > We can defer that extra layer/work if needed even if we go for
> > > > > `Atomic<T>`, but it would be nice to understand if we have consensus
> > > > > for an eventual user-facing API, or if someone has any other opinion
> > > > > or concerns on one vs. the other.  
> > > > 
> > > > Well, here's one:
> > > > 
> > > > The reason that we have things like atomic64_read() in the C code is
> > > > because C doesn't have generics.
> > > > 
> > > > In Rust, we should simply move directly to Atomic<T>, as there are,
> > > > after all, associated benefits. And it's very easy to see the connection  
> > > 
> > > What are the associated benefits you are referring to? Rust std doesn't
> > > use Atomic<T>, that somewhat proves that we don't need it.  
> > Just the stock things that a generic provides: less duplicated code,  
> 
> It's still a bit handwavy, sorry.
> 
> Admittedly, I haven't looked into too much Rust concurrent code, maybe
> it's even true for C code ;-) So I took a look at the crate that Gary
> mentioned (the one provides generic atomic APIs):
> 
> 	https://crates.io/crates/atomic
> 
> there's a "Dependent" tab where you can see the other crates that
> depends on it. With a quick look, I haven't found any Rust concurrent
> project I'm aware of (no crossbeam, no tokio, no futures). On the other
> hand, there is a non-generic based atomic library:
> 
> 	https://crates.io/crates/portable-atomic
> 
> which has more projects depend on it, and there are some Rust concurrent
> projects that I'm aware of: futures, async-task etc. Note that people
> can get the non-generic based atomic API from Rust std library, and
> the "portable-atomic" crate is only 2-year old, while "atomic" crate is
> 8-year old.
> 
> More interestingly, the same author of "atomic" crate, who is an expert
> in concurrent areas, has another project (there are a lot projects from
> the author, but this is the one I'm mostly aware of) "parking_lot",
> which "provides implementations of Mutex, RwLock, Condvar and Once that
> are smaller, faster and more flexible than those in the Rust standard
> library, as well as a ReentrantMutex type which supports recursive
> locking.", and it doesn't use the "atomic" crate either.

Note that crossbeam's AtomicCell is also generic, and crossbeam is used
by tons of crates. As Miguel mentioned, I think it's very likely that in
the future we want be able to do atomics on new types (e.g. for
seqlocks perhaps). We probably don't need the non-lock-free fallback of
crossbeam's AtomicCell, but the lock-free subset with newtype support
is desirable.

People in general don't use the `atomic` crate because it provides no
additional feature compared to the standard library. But it doesn't
really mean that the standard library's atomic design is good.

People decided to use AtomicT and NonZeroT instead of Atomic<T> or
NonZero<T> long time ago, but many now thinks the decision was bad.
Introduction of NonZero<T> is a good example of it. NonZeroT are now
all type aliases of NonZero<T>.

I also don't see any downside in using generics. We can provide type
aliases so people can use `AtomicI32` and `AtomicI64` when they want
their code to be compatible with userspace Rust can still do so.

`Atomic<i32>` is also just aesthetically better than `AtomicI32` IMO.
When all other types look like `NonZero<i32>`, `Wrapping<i32>`, I don't
think we should have `AtomicI32` just because "it's done this way in
Rust std". Our alloc already deviates a lot from Rust std.

Best,
Gary

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 03:51:45PM +0100, Gary Guo wrote:
> On Fri, 14 Jun 2024 19:39:27 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> > On Fri, Jun 14, 2024 at 06:28:00PM -0700, John Hubbard wrote:
> > > On 6/14/24 6:24 PM, Boqun Feng wrote:  
> > > > On Fri, Jun 14, 2024 at 06:03:37PM -0700, John Hubbard wrote:  
> > > > > On 6/14/24 2:59 AM, Miguel Ojeda wrote:  
> > > > > > On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:  
> > > > > > > 
> > > > > > > Does this make sense?  
> > > > > > 
> > > > > > Implementation-wise, if you think it is simpler or more clear/elegant
> > > > > > to have the extra lower level layer, then that sounds fine.
> > > > > > 
> > > > > > However, I was mainly talking about what we would eventually expose to
> > > > > > users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> > > > > > then we could make the lower layer private already.
> > > > > > 
> > > > > > We can defer that extra layer/work if needed even if we go for
> > > > > > `Atomic<T>`, but it would be nice to understand if we have consensus
> > > > > > for an eventual user-facing API, or if someone has any other opinion
> > > > > > or concerns on one vs. the other.  
> > > > > 
> > > > > Well, here's one:
> > > > > 
> > > > > The reason that we have things like atomic64_read() in the C code is
> > > > > because C doesn't have generics.
> > > > > 
> > > > > In Rust, we should simply move directly to Atomic<T>, as there are,
> > > > > after all, associated benefits. And it's very easy to see the connection  
> > > > 
> > > > What are the associated benefits you are referring to? Rust std doesn't
> > > > use Atomic<T>, that somewhat proves that we don't need it.  
> > > Just the stock things that a generic provides: less duplicated code,  
> > 
> > It's still a bit handwavy, sorry.
> > 
> > Admittedly, I haven't looked into too much Rust concurrent code, maybe
> > it's even true for C code ;-) So I took a look at the crate that Gary
> > mentioned (the one provides generic atomic APIs):
> > 
> > 	https://crates.io/crates/atomic
> > 
> > there's a "Dependent" tab where you can see the other crates that
> > depends on it. With a quick look, I haven't found any Rust concurrent
> > project I'm aware of (no crossbeam, no tokio, no futures). On the other
> > hand, there is a non-generic based atomic library:
> > 
> > 	https://crates.io/crates/portable-atomic
> > 
> > which has more projects depend on it, and there are some Rust concurrent
> > projects that I'm aware of: futures, async-task etc. Note that people
> > can get the non-generic based atomic API from Rust std library, and
> > the "portable-atomic" crate is only 2-year old, while "atomic" crate is
> > 8-year old.
> > 
> > More interestingly, the same author of "atomic" crate, who is an expert
> > in concurrent areas, has another project (there are a lot projects from
> > the author, but this is the one I'm mostly aware of) "parking_lot",
> > which "provides implementations of Mutex, RwLock, Condvar and Once that
> > are smaller, faster and more flexible than those in the Rust standard
> > library, as well as a ReentrantMutex type which supports recursive
> > locking.", and it doesn't use the "atomic" crate either.
> 
> Note that crossbeam's AtomicCell is also generic, and crossbeam is used
> by tons of crates. As Miguel mentioned, I think it's very likely that in
> the future we want be able to do atomics on new types (e.g. for
> seqlocks perhaps). We probably don't need the non-lock-free fallback of

Good, another design bit, thank you!

What's our overall idea on sub-word types, like Atomic<u8> and
Atomic<u16>, do we plan to say no to them, or they could have a limited
APIs? IIUC, some operations on them are relatively sub-optimal on some
architectures, supporting the same set of API as i32 and i64 is probably
a bad idea.

Another thing in my mind is making `Atomic<T>`

	pub struct Atomic<T: Send + ...> { ... }

so that `Atomic<T>` will always be `Sync`, because quite frankly, an
atomic type that cannot `Sync` is pointless.

Regards,
Boqun

> crossbeam's AtomicCell, but the lock-free subset with newtype support
> is desirable.
> 
> People in general don't use the `atomic` crate because it provides no
> additional feature compared to the standard library. But it doesn't
> really mean that the standard library's atomic design is good.
> 
> People decided to use AtomicT and NonZeroT instead of Atomic<T> or
> NonZero<T> long time ago, but many now thinks the decision was bad.
> Introduction of NonZero<T> is a good example of it. NonZeroT are now
> all type aliases of NonZero<T>.
> 
> I also don't see any downside in using generics. We can provide type
> aliases so people can use `AtomicI32` and `AtomicI64` when they want
> their code to be compatible with userspace Rust can still do so.
> 
> `Atomic<i32>` is also just aesthetically better than `AtomicI32` IMO.
> When all other types look like `NonZero<i32>`, `Wrapping<i32>`, I don't
> think we should have `AtomicI32` just because "it's done this way in
> Rust std". Our alloc already deviates a lot from Rust std.
> 
> Best,
> Gary

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 08:06:05AM -0700, Boqun Feng wrote:
[...]
> > 
> > Note that crossbeam's AtomicCell is also generic, and crossbeam is used
> > by tons of crates. As Miguel mentioned, I think it's very likely that in
> > the future we want be able to do atomics on new types (e.g. for
> > seqlocks perhaps). We probably don't need the non-lock-free fallback of
> 
> Good, another design bit, thank you!
> 
> What's our overall idea on sub-word types, like Atomic<u8> and
> Atomic<u16>, do we plan to say no to them, or they could have a limited
> APIs? IIUC, some operations on them are relatively sub-optimal on some
> architectures, supporting the same set of API as i32 and i64 is probably
> a bad idea.
> 
> Another thing in my mind is making `Atomic<T>`
> 
> 	pub struct Atomic<T: Send + ...> { ... }
> 
> so that `Atomic<T>` will always be `Sync`, because quite frankly, an
> atomic type that cannot `Sync` is pointless.
> 

Also, how do we avoid this issue [1] in kernel?

`atomic_load()` in C is implemented as READ_ONCE() and it's, at most
time, a volatile read, so the eventual code is:

    let a: (u8, u16) = (1, 2);
    let b = unsafe { core::ptr::read_volatile::<i32>(&a as *const _ as *const i32) };

I know we probably ignore data race here and treat `read_volatile` as a
dependency read per LKMM [2]. But this is an using of uninitialized
data, so it's a bit different.

We can do what https://crates.io/crates/atomic does:

	pub struct Atomic<T: NoUninit + ..> { ... }

, where `NoUinit` means no internal padding bytes, but it loses the
ability to put a 

	#[repr(u32)]
	pub enum Foo { .. }

into `Atomic<T>`, right? Which is probably a case you want to support?

Regards,
Boqun

[1]: https://github.com/crossbeam-rs/crossbeam/issues/748#issuecomment-1133926617
[2]: tools/memory-model/Documentation/access-marking.txt

> Regards,
> Boqun
> 
> > crossbeam's AtomicCell, but the lock-free subset with newtype support
> > is desirable.
> > 
> > People in general don't use the `atomic` crate because it provides no
> > additional feature compared to the standard library. But it doesn't
> > really mean that the standard library's atomic design is good.
> > 
> > People decided to use AtomicT and NonZeroT instead of Atomic<T> or
> > NonZero<T> long time ago, but many now thinks the decision was bad.
> > Introduction of NonZero<T> is a good example of it. NonZeroT are now
> > all type aliases of NonZero<T>.
> > 
> > I also don't see any downside in using generics. We can provide type
> > aliases so people can use `AtomicI32` and `AtomicI64` when they want
> > their code to be compatible with userspace Rust can still do so.
> > 
> > `Atomic<i32>` is also just aesthetically better than `AtomicI32` IMO.
> > When all other types look like `NonZero<i32>`, `Wrapping<i32>`, I don't
> > think we should have `AtomicI32` just because "it's done this way in
> > Rust std". Our alloc already deviates a lot from Rust std.
> > 
> > Best,
> > Gary

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Sun, Jun 16, 2024 at 10:36:13PM -0700, Boqun Feng wrote:
> On Sun, Jun 16, 2024 at 08:06:05AM -0700, Boqun Feng wrote:
> [...]
> > > 
> > > Note that crossbeam's AtomicCell is also generic, and crossbeam is used
> > > by tons of crates. As Miguel mentioned, I think it's very likely that in
> > > the future we want be able to do atomics on new types (e.g. for
> > > seqlocks perhaps). We probably don't need the non-lock-free fallback of
> > 
> > Good, another design bit, thank you!
> > 
> > What's our overall idea on sub-word types, like Atomic<u8> and
> > Atomic<u16>, do we plan to say no to them, or they could have a limited
> > APIs? IIUC, some operations on them are relatively sub-optimal on some
> > architectures, supporting the same set of API as i32 and i64 is probably
> > a bad idea.
> > 
> > Another thing in my mind is making `Atomic<T>`
> > 
> > 	pub struct Atomic<T: Send + ...> { ... }
> > 
> > so that `Atomic<T>` will always be `Sync`, because quite frankly, an
> > atomic type that cannot `Sync` is pointless.
> > 
> 
> Also, how do we avoid this issue [1] in kernel?
> 
> `atomic_load()` in C is implemented as READ_ONCE() and it's, at most
> time, a volatile read, so the eventual code is:
> 
>     let a: (u8, u16) = (1, 2);
>     let b = unsafe { core::ptr::read_volatile::<i32>(&a as *const _ as *const i32) };
> 

^^^^ this line should really be:

	let b: (u8, u16) = unsafe { transmute_copy(&read_volatile::<i32>(&a as *const _ as *const i32)) };

but you get the idea.

Regards,
Boqun

> I know we probably ignore data race here and treat `read_volatile` as a
> dependency read per LKMM [2]. But this is an using of uninitialized
> data, so it's a bit different.
> 
> We can do what https://crates.io/crates/atomic does:
> 
> 	pub struct Atomic<T: NoUninit + ..> { ... }
> 
> , where `NoUinit` means no internal padding bytes, but it loses the
> ability to put a 
> 
> 	#[repr(u32)]
> 	pub enum Foo { .. }
> 
> into `Atomic<T>`, right? Which is probably a case you want to support?
> 
> Regards,
> Boqun
> 
> [1]: https://github.com/crossbeam-rs/crossbeam/issues/748#issuecomment-1133926617
> [2]: tools/memory-model/Documentation/access-marking.txt
> 
> > Regards,
> > Boqun
> > 
> > > crossbeam's AtomicCell, but the lock-free subset with newtype support
> > > is desirable.
> > > 
> > > People in general don't use the `atomic` crate because it provides no
> > > additional feature compared to the standard library. But it doesn't
> > > really mean that the standard library's atomic design is good.
> > > 
> > > People decided to use AtomicT and NonZeroT instead of Atomic<T> or
> > > NonZero<T> long time ago, but many now thinks the decision was bad.
> > > Introduction of NonZero<T> is a good example of it. NonZeroT are now
> > > all type aliases of NonZero<T>.
> > > 
> > > I also don't see any downside in using generics. We can provide type
> > > aliases so people can use `AtomicI32` and `AtomicI64` when they want
> > > their code to be compatible with userspace Rust can still do so.
> > > 
> > > `Atomic<i32>` is also just aesthetically better than `AtomicI32` IMO.
> > > When all other types look like `NonZero<i32>`, `Wrapping<i32>`, I don't
> > > think we should have `AtomicI32` just because "it's done this way in
> > > Rust std". Our alloc already deviates a lot from Rust std.
> > > 
> > > Best,
> > > Gary

Re: [RFC 2/2] rust: sync: Add atomic support

[Benno Lossin]

On 17.06.24 07:36, Boqun Feng wrote:
> On Sun, Jun 16, 2024 at 08:06:05AM -0700, Boqun Feng wrote:
> [...]
>>>
>>> Note that crossbeam's AtomicCell is also generic, and crossbeam is used
>>> by tons of crates. As Miguel mentioned, I think it's very likely that in
>>> the future we want be able to do atomics on new types (e.g. for
>>> seqlocks perhaps). We probably don't need the non-lock-free fallback of
>>
>> Good, another design bit, thank you!
>>
>> What's our overall idea on sub-word types, like Atomic<u8> and
>> Atomic<u16>, do we plan to say no to them, or they could have a limited
>> APIs? IIUC, some operations on them are relatively sub-optimal on some
>> architectures, supporting the same set of API as i32 and i64 is probably
>> a bad idea.
>>
>> Another thing in my mind is making `Atomic<T>`
>>
>> 	pub struct Atomic<T: Send + ...> { ... }
>>
>> so that `Atomic<T>` will always be `Sync`, because quite frankly, an
>> atomic type that cannot `Sync` is pointless.

That is true, but adding semantically "unnecessary" bounds can be bad.
This is because they infect everything that wants to use `Atomic<T>`,
since they also need to add that bound.

> Also, how do we avoid this issue [1] in kernel?

I think that we can first go the way of my second approach (ie adding a
private trait as a bound on `Atomic<T>` to prevent generic usage). And
only allow primitives.
If we then see that people would like to put their own (u8, u16) tuple
structs into `Atomic<T>`, we have multiple options:

1. Field projection:
   Only primitives can be `load`ed and `store`ed, to access the values
   of the tuple, one would need to project to each field and read them.

2. Disallow padding:
   We add an `unsafe` trait that asserts there are no padding bytes in
   there (like `NoUinit` from below) and also add a macro that
   implements the trait safely.

3. Use `MaybeUninit` under the hood:
   I don't know if this would fix the issue entirely, since that is what
   crossbeam currently uses (but the issue remains open).

But I don't think that we should encourage large structs to be put into
`Atomic<T>`, since that would be bad for perf, right? So I think that
going the way of 1 would be great (if we had FP, otherwise 2 seems fine).

> `atomic_load()` in C is implemented as READ_ONCE() and it's, at most
> time, a volatile read, so the eventual code is:
> 
>     let a: (u8, u16) = (1, 2);
>     let b = unsafe { core::ptr::read_volatile::<i32>(&a as *const _ as *const i32) };
> 
> I know we probably ignore data race here and treat `read_volatile` as a
> dependency read per LKMM [2]. But this is an using of uninitialized
> data, so it's a bit different.

But would we implement it this way? Or would it go through a C function?
If we entirely do it in Rust, then yes this is bad.

---
Cheers,
Benno

> We can do what https://crates.io/crates/atomic does:
> 
> 	pub struct Atomic<T: NoUninit + ..> { ... }
> 
> , where `NoUinit` means no internal padding bytes, but it loses the
> ability to put a
> 
> 	#[repr(u32)]
> 	pub enum Foo { .. }
> 
> into `Atomic<T>`, right? Which is probably a case you want to support?
> 
> Regards,
> Boqun
> 
> [1]: https://github.com/crossbeam-rs/crossbeam/issues/748#issuecomment-1133926617
> [2]: tools/memory-model/Documentation/access-marking.txt

Re: [RFC 2/2] rust: sync: Add atomic support

[Andrew Lunn]

On Fri, Jun 14, 2024 at 11:59:58AM +0200, Miguel Ojeda wrote:
> On Thu, Jun 13, 2024 at 9:05 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Does this make sense?
> 
> Implementation-wise, if you think it is simpler or more clear/elegant
> to have the extra lower level layer, then that sounds fine.
> 
> However, I was mainly talking about what we would eventually expose to
> users, i.e. do we want to provide `Atomic<T>` to begin with? If yes,
> then we could make the lower layer private already.
> 
> We can defer that extra layer/work if needed even if we go for
> `Atomic<T>`, but it would be nice to understand if we have consensus
> for an eventual user-facing API, or if someone has any other opinion
> or concerns on one vs. the other.

Since this is fully compatible to LKMM atomic operations, is there a
use case for C and Rust operating on the same atomic value? And then
you will need to specify the size, or odd things are likely to happen
if they disagree on size. With Atomic<T> can we easily say what type
the underlying implementation uses?

     Andrew

Re: [RFC 2/2] rust: sync: Add atomic support

[Peter Zijlstra]

On Thu, Jun 13, 2024 at 09:30:26AM -0700, Boqun Feng wrote:

> We can always add a layer on top of what we have here to provide the
> generic `Atomic<T>`. However, I personally don't think generic
> `Atomic<T>` is a good idea, for a few reasons:
> 
> *	I'm not sure it will bring benefits to users, the current atomic
> 	users in kernel are pretty specific on the size of atomic they
> 	use, so they want to directly use AtomicI32 or AtomicI64 in
> 	their type definitions rather than use a `Atomic<T>` where their
> 	users can provide type later.
> 
> *	I can also see the future where we have different APIs on
> 	different types of atomics, for example, we could have a:
> 
> 		impl AtomicI64 {
> 		    pub fn split(&self) -> (&AtomicI32, &AtomicI32)
> 		}
> 
> 	which doesn't exist for AtomicI32. Note this is not a UB because
> 	we write our atomic implementation in asm, so it's perfectly
> 	fine for mix-sized atomics.
> 
> So let's start with some basic and simple until we really have a need
> for generic `Atomic<T>`. Thoughts?

Not on the generic thing, but on the lack of long. atomic_long_t is
often used when we have pointers with extra bits on. Then you want a
number type in order to be able to manipulate the low bits.

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Fri, Jun 14, 2024 at 11:51:24AM +0200, Peter Zijlstra wrote:
> On Thu, Jun 13, 2024 at 09:30:26AM -0700, Boqun Feng wrote:
> 
> > We can always add a layer on top of what we have here to provide the
> > generic `Atomic<T>`. However, I personally don't think generic
> > `Atomic<T>` is a good idea, for a few reasons:
> > 
> > *	I'm not sure it will bring benefits to users, the current atomic
> > 	users in kernel are pretty specific on the size of atomic they
> > 	use, so they want to directly use AtomicI32 or AtomicI64 in
> > 	their type definitions rather than use a `Atomic<T>` where their
> > 	users can provide type later.
> > 
> > *	I can also see the future where we have different APIs on
> > 	different types of atomics, for example, we could have a:
> > 
> > 		impl AtomicI64 {
> > 		    pub fn split(&self) -> (&AtomicI32, &AtomicI32)
> > 		}
> > 
> > 	which doesn't exist for AtomicI32. Note this is not a UB because
> > 	we write our atomic implementation in asm, so it's perfectly
> > 	fine for mix-sized atomics.
> > 
> > So let's start with some basic and simple until we really have a need
> > for generic `Atomic<T>`. Thoughts?
> 
> Not on the generic thing, but on the lack of long. atomic_long_t is
> often used when we have pointers with extra bits on. Then you want a
> number type in order to be able to manipulate the low bits.

I mentioned my plan on AtomicPtr, but I think I should have clarified
this more. My plan is:

pub struct AtomicIsize {
    #[cfg(CONFIG_64BIT)]
    inner: AtomicI64
    #[cfg(not(CONFIG_64BIT))]
    inner: AtomicI32
}

i.e. building AtomicIsize (Rust's atomic_long_t) based on AtomicI64 and
AtomicI32. And we can a AtomicPtr type on it:

pub struct AtomicPtr<T> {
    inner: AtomicIsize,
    _type: PhantomData<*mut T>
}

Of course, I need to do some code generating work for AtomicIsize and
AtomicPtr, I plan to do that in Rust not in scripts, this will keep the
rust/kernel/sync/atomic/impl.rs relatively small (i.e. the Rust/C
interface is smaller). I can include this part in the next version, if
you want to see it.

Regards,
Boqun

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Wed, Jun 12, 2024 at 03:30:25PM -0700, Boqun Feng wrote:
[...]
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 48e7029f1054..99e6e2b2867f 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1601,6 +1601,8 @@ static bool
>  has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
>  {
>  	u64 val = read_scoped_sysreg(entry, scope);
> +	if (entry->capability == ARM64_HAS_LSE_ATOMICS)
> +		return false;
>  	return feature_matches(val, entry);
>  }
>  

Yeah, this part was mis-committed, will remove it in the next version.

Regards,
Boqun

[...]

Re: [RFC 2/2] rust: sync: Add atomic support

[Mark Rutland]

On Wed, Jun 12, 2024 at 03:30:25PM -0700, Boqun Feng wrote:
> Provide two atomic types: AtomicI32 and AtomicI64 with the existing
> implemenation of C atomics. These atomics have the same semantics of the
> corresponding LKMM C atomics, and using one memory (ordering) model
> certainly reduces the reasoning difficulty and potential bugs from the
> interaction of two different memory models.
> 
> Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
> my responsiblity on these Rust APIs.
> 
> Note that `Atomic*::new()`s are implemented vi open coding on struct
> atomic*_t. This allows `new()` being a `const` function, so that it can
> be used in constant contexts.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>

I have a few minor comments below.

> ---
>  MAINTAINERS                       |    4 +-
>  arch/arm64/kernel/cpufeature.c    |    2 +
>  rust/kernel/sync.rs               |    1 +
>  rust/kernel/sync/atomic.rs        |   63 ++
>  rust/kernel/sync/atomic/impl.rs   | 1375 +++++++++++++++++++++++++++++
>  scripts/atomic/gen-atomics.sh     |    1 +
>  scripts/atomic/gen-rust-atomic.sh |  136 +++
>  7 files changed, 1581 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/sync/atomic.rs
>  create mode 100644 rust/kernel/sync/atomic/impl.rs
>  create mode 100755 scripts/atomic/gen-rust-atomic.sh
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d6c90161c7bf..a8528d27b260 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3458,7 +3458,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
> @@ -3467,6 +3467,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/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 48e7029f1054..99e6e2b2867f 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1601,6 +1601,8 @@ static bool
>  has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
>  {
>  	u64 val = read_scoped_sysreg(entry, scope);
> +	if (entry->capability == ARM64_HAS_LSE_ATOMICS)
> +		return false;
>  	return feature_matches(val, entry);
>  }

As per other replies, this'll obviously need to go.

> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> new file mode 100644
> index 000000000000..b0f852cf1741
> --- /dev/null
> +++ b/rust/kernel/sync/atomic.rs
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Atomic primitives.
> +//!
> +//! These primitives have the same semantics as their C counterparts, for precise definitions of
> +//! the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory (Consistency)
> +//! Model is the only model for Rust development in kernel right now, please avoid to use Rust's
> +//! own atomics.
> +
> +use crate::bindings::{atomic64_t, atomic_t};

As with the last patch, why no atomic_long_t?

[...]

> +#gen_proto_order_variant(meta, pfx, name, sfx, order, atomic, ty, int, raw, 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 ty="$1"; shift
> +	local int="$1"; shift
> +	local raw="$1"; shift
> +
> +	local fn_name="${raw}${pfx}${name}${sfx}${order}"
> +	local atomicname="${raw}${atomic}_${pfx}${name}${sfx}${order}"
> +
> +	local ret="$(gen_ret_type "${meta}" "${int}")"
> +	local params="$(gen_params "${int}" $@)"
> +	local args="$(gen_args "$@")"
> +	local retstmt="$(gen_ret_stmt "${meta}")"
> +
> +cat <<EOF
> +    /// See \`${atomicname}\`.
> +    #[inline(always)]
> +    pub fn ${fn_name}(&self${params}) ${ret}{
> +        // SAFETY:\`self.0.get()\` is a valid pointer.
> +        unsafe {
> +            ${retstmt}${atomicname}(${args});
> +        }
> +    }
> +EOF
> +}

AFAICT the 'ty' argument (AtomicI32/AtomicI64) isn't used and can be
removed.

Likewise for 'raw'.

> +
> +cat << EOF
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Generated by $0
> +//! DO NOT MODIFY THIS FILE DIRECTLY
> +
> +use super::*;
> +use crate::bindings::*;
> +
> +impl AtomicI32 {
> +EOF
> +
> +grep '^[a-z]' "$1" | while read name meta args; do
> +	gen_proto "${meta}" "${name}" "atomic" "AtomicI32" "i32" "" ${args}

With 'ty' and 'raw' gone, this'd be:

	gen_proto "${meta}" "${name}" "atomic" "i32" ${args}

> +done
> +
> +cat << EOF
> +}
> +
> +impl AtomicI64 {
> +EOF
> +
> +grep '^[a-z]' "$1" | while read name meta args; do
> +	gen_proto "${meta}" "${name}" "atomic64" "AtomicI64" "i64" "" ${args}

With 'ty' and 'raw' gone, this'd be:

	gen_proto "${meta}" "${name}" "atomic64" "i64" ${args}

Mark.

> +done
> +
> +cat << EOF
> +}
> +
> +EOF
> -- 
> 2.45.2
> 

Re: [RFC 2/2] rust: sync: Add atomic support

[Boqun Feng]

On Fri, Jun 14, 2024 at 11:40:47AM +0100, Mark Rutland wrote:
[...]
> > +#gen_proto_order_variant(meta, pfx, name, sfx, order, atomic, ty, int, raw, 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 ty="$1"; shift
> > +	local int="$1"; shift
> > +	local raw="$1"; shift
> > +
> > +	local fn_name="${raw}${pfx}${name}${sfx}${order}"
> > +	local atomicname="${raw}${atomic}_${pfx}${name}${sfx}${order}"
> > +
> > +	local ret="$(gen_ret_type "${meta}" "${int}")"
> > +	local params="$(gen_params "${int}" $@)"
> > +	local args="$(gen_args "$@")"
> > +	local retstmt="$(gen_ret_stmt "${meta}")"
> > +
> > +cat <<EOF
> > +    /// See \`${atomicname}\`.
> > +    #[inline(always)]
> > +    pub fn ${fn_name}(&self${params}) ${ret}{
> > +        // SAFETY:\`self.0.get()\` is a valid pointer.
> > +        unsafe {
> > +            ${retstmt}${atomicname}(${args});
> > +        }
> > +    }
> > +EOF
> > +}
> 
> AFAICT the 'ty' argument (AtomicI32/AtomicI64) isn't used and can be
> removed.
> 

Good catch.

> Likewise for 'raw'.
> 
> > +
> > +cat << EOF
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Generated by $0
> > +//! DO NOT MODIFY THIS FILE DIRECTLY
> > +
> > +use super::*;
> > +use crate::bindings::*;
> > +
> > +impl AtomicI32 {
> > +EOF
> > +
> > +grep '^[a-z]' "$1" | while read name meta args; do
> > +	gen_proto "${meta}" "${name}" "atomic" "AtomicI32" "i32" "" ${args}
> 
> With 'ty' and 'raw' gone, this'd be:
> 
> 	gen_proto "${meta}" "${name}" "atomic" "i32" ${args}
> 
> > +done
> > +
> > +cat << EOF
> > +}
> > +
> > +impl AtomicI64 {
> > +EOF
> > +
> > +grep '^[a-z]' "$1" | while read name meta args; do
> > +	gen_proto "${meta}" "${name}" "atomic64" "AtomicI64" "i64" "" ${args}
> 
> With 'ty' and 'raw' gone, this'd be:
> 
> 	gen_proto "${meta}" "${name}" "atomic64" "i64" ${args}
> 

All fixed locally, thanks!

Regards,
Boqun

> Mark.
> 
> > +done
> > +
> > +cat << EOF
> > +}
> > +
> > +EOF
> > -- 
> > 2.45.2
> >