On Fri, Dec 13, 2024 at 03:37:13PM +0100, Alice Ryhl wrote:
[...]
quoted
quoted
quoted
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Atomic primitives.
+//!
+//! These primitives have the same semantics as their C counterparts: and the precise definitions of
+//! semantics can be found at [`LKMM`]. Note that Linux Kernel Memory (Consistency) Model is the
+//! only model for Rust code in kernel, and Rust's own atomics should be avoided.
+//!
+//! # Data races
+//!
+//! [`LKMM`] atomics have different rules regarding data races:
+//!
+//! - A normal read doesn't data-race with an atomic read.
This was fixed:
https://github.com/rust-lang/rust/pull/128778
Yeah, I was aware of that effort, and good to know it's finally merged.
Thanks!
This will be in 1.83, right? If so, we will still need the above until
we bump up the minimal rustc version to 1.83 or beyond. I will handle
this properly with the minimal rustc 1.83 (i.e. if this goes in first,
will send a follow up patch). I will also mention in the above that this
has been changed in 1.83.
This also reminds that I should add that LKMM allows mixed-size atomic
accesses (as non data race), I will add that in the version.
This is just documentation. I don't think you need to do any special
The PR also contained miri changes, so the same code will be reported
differently by miri. That was what I was thinking of. However, now think
about it, we are not going to use Rust atomics, so this difference
shouldn't affect us. Therefore I agree, I will drop this.
MSRV handling.
quoted
quoted
quoted
+mod private {
+ /// Sealed trait marker to disable customized impls on atomic implementation traits.
+ pub trait Sealed {}
+}
Just make the trait unsafe?
And make the safety requirement of `AtomicImpl` something like:
The type must have the implementation for atomic operations.
? Hmm.. I don't think that's a good safety requirement TBH. Actually the
reason that we need to restrict `AtomicImpl` types is more of an
iplementation issue (the implementation need to be done if we want to
support i8 or i16) rather than safety issue. So a sealed trait is proper
here. Does this make sense? Or am I missing something?
Where is the AtomicImpl trait used?
It's used when `impl`ing an `AllowAtomic` type, `AllowAtomic` has an
associate type named `Repr` which must be an `AtomicImpl`, i.e. each
type that has atomic operation support must select the underlying
implementation types (currently we only have i32 and i64 from C side
APIs). Using a sealed trait is appropriate in this case, because unless
you are adding atomic support for different sizes of data, you shouldn't
impl `AtomicImpl`.
Regards,
Boqun
Alice