On Wed, 18 Jun 2025 09:49:27 -0700
Boqun Feng [off-list ref] wrote:
Preparation for atomic primitives. Instead of a suffix like _acquire, a
method parameter along with the corresponding generic parameter will be
used to specify the ordering of an atomic operations. For example,
atomic load() can be defined as:
impl<T: ...> Atomic<T> {
pub fn load<O: AcquireOrRelaxed>(&self, _o: O) -> T { ... }
}
and acquire users would do:
let r = x.load(Acquire);
relaxed users:
let r = x.load(Relaxed);
doing the following:
let r = x.load(Release);
will cause a compiler error.
I quite like the design. Minor comments inline below.
quoted hunk ↗ jump to hunk
Compared to suffixes, it's easier to tell what ordering variants an
operation has, and it also make it easier to unify the implementation of
all ordering variants in one method via generic. The `IS_RELAXED` and
`TYPE` associate consts are for generic function to pick up the
particular implementation specified by an ordering annotation.
Signed-off-by: Boqun Feng <redacted>
---
rust/kernel/sync/atomic.rs | 3 +
rust/kernel/sync/atomic/ordering.rs | 106 ++++++++++++++++++++++++++++
2 files changed, 109 insertions(+)
create mode 100644 rust/kernel/sync/atomic/ordering.rs
diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
index 65e41dba97b7..9fe5d81fc2a9 100644
--- a/rust/kernel/sync/atomic.rs
+++ b/rust/kernel/sync/atomic.rs
@@ -17,3 +17,6 @@
//! [`LKMM`]: srctree/tools/memory-mode/
pub mod ops;
+pub mod ordering;
+
+pub use ordering::{Acquire, Full, Relaxed, Release};diff --git a/rust/kernel/sync/atomic/ordering.rs b/rust/kernel/sync/atomic/ordering.rs
new file mode 100644
index 000000000000..96757574ed7d
--- /dev/null
+++ b/rust/kernel/sync/atomic/ordering.rs
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Memory orderings.
+//!
+//! The semantics of these orderings follows the [`LKMM`] definitions and rules.
+//!
+//! - [`Acquire`] and [`Release`] are similar to their counterpart in Rust memory model.
+//! - [`Full`] means "fully-ordered", that is:
+//! - It provides ordering between all the preceding memory accesses and the annotated operation.
+//! - It provides ordering between the annotated operation and all the following memory accesses.
+//! - It provides ordering between all the preceding memory accesses and all the fllowing memory
+//! accesses.
+//! - All the orderings are the same strong as a full memory barrier (i.e. `smp_mb()`).
+//! - [`Relaxed`] is similar to the counterpart in Rust memory model, except that dependency
+//! orderings are also honored in [`LKMM`]. Dependency orderings are described in "DEPENDENCY
+//! RELATIONS" in [`LKMM`]'s [`explanation`].
+//!
+//! [`LKMM`]: srctree/tools/memory-model/
+//! [`explanation`]: srctree/tools/memory-model/Documentation/explanation.txt
+
+/// The annotation type for relaxed memory ordering.
+pub struct Relaxed;
+
+/// The annotation type for acquire memory ordering.
+pub struct Acquire;
+
+/// The annotation type for release memory ordering.
+pub struct Release;
+
+/// The annotation type for fully-order memory ordering.
+pub struct Full;
+
+/// Describes the exact memory ordering.
+pub enum OrderingType {
+ /// Relaxed ordering.
+ Relaxed,
+ /// Acquire ordering.
+ Acquire,
+ /// Release ordering.
+ Release,
+ /// Fully-ordered.
+ Full,
+}
Does this need to be public? I think this can cause a confusion on what
this is in the rendered documentation.
IIUC this is for internal atomic impl only
and this is not useful otherwise. This can be moved into `internal` and
then `pub(super) use internal::OrderingType` to stop exposing it.
(Or, just `#[doc(hidden)]` so it doesn't show in the docs).
quoted hunk ↗ jump to hunk
+
+mod internal {
+ /// Unit types for ordering annotation.
+ ///
+ /// Sealed trait, can be only implemented inside atomic mod.
+ pub trait OrderingUnit {
+ /// Describes the exact memory ordering.
+ const TYPE: super::OrderingType;
+ }
+}
+
+impl internal::OrderingUnit for Relaxed {
+ const TYPE: OrderingType = OrderingType::Relaxed;
+}
+
+impl internal::OrderingUnit for Acquire {
+ const TYPE: OrderingType = OrderingType::Acquire;
+}
+
+impl internal::OrderingUnit for Release {
+ const TYPE: OrderingType = OrderingType::Release;
+}
+
+impl internal::OrderingUnit for Full {
+ const TYPE: OrderingType = OrderingType::Full;
+}
+
+/// The trait bound for annotating operations that should support all orderings.
+pub trait All: internal::OrderingUnit {}
+
+impl All for Relaxed {}
+impl All for Acquire {}
+impl All for Release {}
+impl All for Full {}
+
+/// The trait bound for operations that only support acquire or relaxed ordering.
+pub trait AcquireOrRelaxed: All {
+ /// Describes whether an ordering is relaxed or not.
+ const IS_RELAXED: bool = false;
This should not be needed. I'd prefer to the use site to just match on
`TYPE`.
quoted hunk ↗ jump to hunk
+}
+
+impl AcquireOrRelaxed for Acquire {}
+
+impl AcquireOrRelaxed for Relaxed {
+ const IS_RELAXED: bool = true;
+}
+
+/// The trait bound for operations that only support release or relaxed ordering.
+pub trait ReleaseOrRelaxed: All {
+ /// Describes whether an ordering is relaxed or not.
+ const IS_RELAXED: bool = false;
+}
+
+impl ReleaseOrRelaxed for Release {}
+
+impl ReleaseOrRelaxed for Relaxed {
+ const IS_RELAXED: bool = true;
+}
+
+/// The trait bound for operations that only support relaxed ordering.
+pub trait RelaxedOnly: AcquireOrRelaxed + ReleaseOrRelaxed + All {}
+
+impl RelaxedOnly for Relaxed {}
Any reason that this is needed at all? Should just be a non-generic
function that takes a `Relaxed` directly?