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