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

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

and acquire users would do:

	let r = x.load(Acquire);

relaxed users:

	let r = x.load(Relaxed);

doing the following:

	let r = x.load(Release);

will cause a compiler error.

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

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/sync/atomic.rs          |   3 +
 rust/kernel/sync/atomic/ordering.rs | 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,
+}
+
+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;
+}
+
+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 {}
-- 
2.39.5 (Apple Git-154)

"Boqun Feng" <boqun.feng@gmail.com> writes:

> Preparation for atomic primitives. Instead of a suffix like _acquire, a
> method parameter along with the corresponding generic parameter will be
> used to specify the ordering of an atomic operations. For example,
> atomic load() can be defined as:
>
> 	impl<T: ...> Atomic<T> {
> 	    pub fn load<O: AcquireOrRelaxed>(&self, _o: O) -> T { ... }
> 	}
>
> and acquire users would do:
>
> 	let r = x.load(Acquire);
>
> relaxed users:
>
> 	let r = x.load(Relaxed);
>
> doing the following:
>
> 	let r = x.load(Release);
>
> will cause a compiler error.
>
> Compared to suffixes, it's easier to tell what ordering variants an
> operation has, and it also make it easier to unify the implementation of
> all ordering variants in one method via generic. The `IS_RELAXED` and
> `TYPE` associate consts are for generic function to pick up the
> particular implementation specified by an ordering annotation.
>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  rust/kernel/sync/atomic.rs          |   3 +
>  rust/kernel/sync/atomic/ordering.rs | 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,
> +}
> +
> +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 {}

I think I would prefer `Any` rather than `All` here. Because it is "any
of", not "all of them at once".


Best regards,
Andreas Hindborg

On Thu, Jun 26, 2025 at 02:36:50PM +0200, Andreas Hindborg wrote:
[...]
> > +/// The trait bound for annotating operations that should support all orderings.
> > +pub trait All: internal::OrderingUnit {}
> 
> I think I would prefer `Any` rather than `All` here. Because it is "any
> of", not "all of them at once".
> 

Good idea! Changed. Thanks!

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
>

On Fri, Jun 27, 2025 at 07:34:46AM -0700, Boqun Feng wrote:
> On Thu, Jun 26, 2025 at 02:36:50PM +0200, Andreas Hindborg wrote:
> [...]
> > > +/// The trait bound for annotating operations that should support all orderings.
> > > +pub trait All: internal::OrderingUnit {}
> > 
> > I think I would prefer `Any` rather than `All` here. Because it is "any
> > of", not "all of them at once".
> > 
> 
> Good idea! Changed. Thanks!
> 

And I realized I can unify `Any` with `OrderingUnit`, here is the what I
have now:

mod internal {
    /// Sealed trait, can be only implemented inside atomic mod.
    pub trait Sealed {}

    impl Sealed for super::Relaxed {}
    impl Sealed for super::Acquire {}
    impl Sealed for super::Release {}
    impl Sealed for super::Full {}
}

/// The trait bound for annotating operations that support any ordering.
pub trait Any: internal::Sealed {
    /// Describes the exact memory ordering.
    const TYPE: OrderingType;
}

impl Any for Relaxed {
    const TYPE: OrderingType = OrderingType::Relaxed;
}

impl Any for Acquire {
    const TYPE: OrderingType = OrderingType::Acquire;
}

impl Any for Release {
    const TYPE: OrderingType = OrderingType::Release;
}

impl Any for Full {
    const TYPE: OrderingType = OrderingType::Full;
}

Better than what I had before, thanks!

Regards,
Boqun

On Wed, 18 Jun 2025 09:49:27 -0700
Boqun Feng <boqun.feng@gmail.com> 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.

> 
> 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 <boqun.feng@gmail.com>
> ---
>  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).

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

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

On Sat, Jun 21, 2025 at 12:18:42PM +0100, Gary Guo wrote:
[...]
> > +
> > +/// 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.
> 

I would like to make it public so that users can define their own method
with ordering out of atomic mod (even out of kernel crate):

    pub fn my_ordering_func<Ordering: All>(..., o: Ordering) {
        match Ordering::TYPE {

	}
    }

I just realized to do so I need to make OrderingUnit pub too (with a
sealed supertrait of course).

> 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).
> 

Seem reasonable.

> > +
> > +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;
> > +}
[...]
> > +
> > +/// 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`.
> 

Right, I somehow missed how monomorphization works. I can drop this.
Thanks!

> > +}
> > +
[...]
> > +/// 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

Mostly for documentation purpose, i.e. users can figure out the ordering
from the trait bounds of the function. I will say we can probably drop
it when we find a Release-only or Acquire-only function, but even then
the current definition won't affect users, so I lean torwards keeping
it.

Regards,
Boqun

> function that takes a `Relaxed` directly?
>

On Wed, Jun 18, 2025 at 09:49:27AM -0700, Boqun Feng wrote:

> +//! 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.

So I've no clue what the Rust memory model states, and I'm assuming
it is very similar to the C11 model. I have also forgotten what LKMM
states :/

Do they all agree on what RELEASE+ACQUIRE makes?

> +//! - [`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()`).

s/strong/strength/ ?

> +//! - [`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

On Thu, Jun 19, 2025 at 12:31:55PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 18, 2025 at 09:49:27AM -0700, Boqun Feng wrote:
> 
> > +//! 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.
> 
> So I've no clue what the Rust memory model states, and I'm assuming
> it is very similar to the C11 model. I have also forgotten what LKMM
> states :/
> 
> Do they all agree on what RELEASE+ACQUIRE makes?
> 

I think the question is irrelevant here, because we are implementing
LKMM atomics in Rust using primitives from C, so no C11/Rust memory
model in the picture for kernel Rust.

But I think they do. I assume you mostly ask whether RELEASE(a) +
ACQUIRE(b) (i.e. release and acquire on different variables) makes a TSO
barrier [1]? We don't make it a TSO barrier in LKMM either (only
unlock(a)+lock(b) is a TSO barrier) and neither does C11/Rust memory
model.

[1]: https://lore.kernel.org/lkml/20211202005053.3131071-1-paulmck@kernel.org/

> > +//! - [`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()`).
> 
> s/strong/strength/ ?
> 

Fixed locally.

Regards,
Boqun

> > +//! - [`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

On Thu, Jun 19, 2025 at 06:29:29AM -0700, Boqun Feng wrote:
> On Thu, Jun 19, 2025 at 12:31:55PM +0200, Peter Zijlstra wrote:
> > On Wed, Jun 18, 2025 at 09:49:27AM -0700, Boqun Feng wrote:
> > 
> > > +//! 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.
> > 
> > So I've no clue what the Rust memory model states, and I'm assuming
> > it is very similar to the C11 model. I have also forgotten what LKMM
> > states :/
> > 
> > Do they all agree on what RELEASE+ACQUIRE makes?
> > 
> 
> I think the question is irrelevant here, because we are implementing
> LKMM atomics in Rust using primitives from C, so no C11/Rust memory
> model in the picture for kernel Rust.

The question is relevant in so far that the comment refers to them; and
if their behaviour is different in any way, this is confusing.

> But I think they do. I assume you mostly ask whether RELEASE(a) +
> ACQUIRE(b) (i.e. release and acquire on different variables) makes a TSO
> barrier [1]? We don't make it a TSO barrier in LKMM either (only
> unlock(a)+lock(b) is a TSO barrier) and neither does C11/Rust memory
> model.
> 
> [1]: https://lore.kernel.org/lkml/20211202005053.3131071-1-paulmck@kernel.org/

Right, that!

So given we build locks from atomics, this has to come from somewhere.

The simplest lock -- TAS -- is: rmw.acquire + store.release.

So while plain store.release + load.acquire might not make TSO (although
IIRC ARM added variants that do just that in an effort to aid x86
emulation); store.release + rmw.acquire must, otherwise we cannot
satisfy that unlock+lock.

On Thu, Jun 19, 2025 at 04:32:14PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 19, 2025 at 06:29:29AM -0700, Boqun Feng wrote:
> > On Thu, Jun 19, 2025 at 12:31:55PM +0200, Peter Zijlstra wrote:
> > > On Wed, Jun 18, 2025 at 09:49:27AM -0700, Boqun Feng wrote:
> > > 
> > > > +//! 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.
> > > 
> > > So I've no clue what the Rust memory model states, and I'm assuming
> > > it is very similar to the C11 model. I have also forgotten what LKMM
> > > states :/
> > > 
> > > Do they all agree on what RELEASE+ACQUIRE makes?
> > > 
> > 
> > I think the question is irrelevant here, because we are implementing
> > LKMM atomics in Rust using primitives from C, so no C11/Rust memory
> > model in the picture for kernel Rust.
> 
> The question is relevant in so far that the comment refers to them; and
> if their behaviour is different in any way, this is confusing.
> 

I did use the word "similar" and before that I said "The semantics of
these orderings follows the [`LKMM`] definitions and rules." The
referring was merely to avoid repeating the part like:

- [`Acquire`] orders the load part of the operation against all
  following memory operations.

- [`Release`] orders the store part of the operation against all
  preceding memory operations.
  
because of this part, both models agree. But if you think this way is
better, I could change it.

> > But I think they do. I assume you mostly ask whether RELEASE(a) +
> > ACQUIRE(b) (i.e. release and acquire on different variables) makes a TSO
> > barrier [1]? We don't make it a TSO barrier in LKMM either (only
> > unlock(a)+lock(b) is a TSO barrier) and neither does C11/Rust memory
> > model.
> > 
> > [1]: https://lore.kernel.org/lkml/20211202005053.3131071-1-paulmck@kernel.org/
> 
> Right, that!
> 
> So given we build locks from atomics, this has to come from somewhere.
> 
> The simplest lock -- TAS -- is: rmw.acquire + store.release.
> 
> So while plain store.release + load.acquire might not make TSO (although
> IIRC ARM added variants that do just that in an effort to aid x86
> emulation); store.release + rmw.acquire must, otherwise we cannot
> satisfy that unlock+lock.
> 

Make sense, so something like this in the model should work:

diff --git a/tools/memory-model/linux-kernel.cat b/tools/memory-model/linux-kernel.cat
index d7e7bf13c831..90cb6db6e335 100644
--- a/tools/memory-model/linux-kernel.cat
+++ b/tools/memory-model/linux-kernel.cat
@@ -27,7 +27,7 @@ include "lock.cat"
 (* Release Acquire *)
 let acq-po = [Acquire] ; po ; [M]
 let po-rel = [M] ; po ; [Release]
-let po-unlock-lock-po = po ; [UL] ; (po|rf) ; [LKR] ; po
+let po-unlock-lock-po = po ; (([UL] ; (po|rf) ; [LKR]) | ([Release]; (po;rf); [Acquire & RMW])) ; po

 (* Fences *)
 let R4rmb = R \ Noreturn       (* Reads for which rmb works *)


although I'm not sure whether there will be actual users that use this
ordering.

Regards,
Boqun

On Thu, Jun 19, 2025 at 08:00:30AM -0700, Boqun Feng wrote:
> Make sense, so something like this in the model should work:
> 
> diff --git a/tools/memory-model/linux-kernel.cat b/tools/memory-model/linux-kernel.cat
> index d7e7bf13c831..90cb6db6e335 100644
> --- a/tools/memory-model/linux-kernel.cat
> +++ b/tools/memory-model/linux-kernel.cat
> @@ -27,7 +27,7 @@ include "lock.cat"
>  (* Release Acquire *)
>  let acq-po = [Acquire] ; po ; [M]
>  let po-rel = [M] ; po ; [Release]
> -let po-unlock-lock-po = po ; [UL] ; (po|rf) ; [LKR] ; po
> +let po-unlock-lock-po = po ; (([UL] ; (po|rf) ; [LKR]) | ([Release]; (po;rf); [Acquire & RMW])) ; po
> 
>  (* Fences *)
>  let R4rmb = R \ Noreturn       (* Reads for which rmb works *)
> 
> 
> although I'm not sure whether there will be actual users that use this
> ordering.

If we do end up making a change like this then we should also start 
keeping careful track of the parts of the LKMM that are not justified by 
the operational model (and vice versa), perhaps putting something about 
them into the documentation.  As far as I can remember, 
po-unlock-lock-po is the only current example, but my memory isn't 
always the greatest -- just one reason why it would be good to have 
these things written down in an organized manner.

Alan

On Thu, Jun 19, 2025 at 08:00:30AM -0700, Boqun Feng wrote:

> > So given we build locks from atomics, this has to come from somewhere.
> > 
> > The simplest lock -- TAS -- is: rmw.acquire + store.release.
> > 
> > So while plain store.release + load.acquire might not make TSO (although
> > IIRC ARM added variants that do just that in an effort to aid x86
> > emulation); store.release + rmw.acquire must, otherwise we cannot
> > satisfy that unlock+lock.
> 
> Make sense, so something like this in the model should work:
> 
> diff --git a/tools/memory-model/linux-kernel.cat b/tools/memory-model/linux-kernel.cat
> index d7e7bf13c831..90cb6db6e335 100644
> --- a/tools/memory-model/linux-kernel.cat
> +++ b/tools/memory-model/linux-kernel.cat
> @@ -27,7 +27,7 @@ include "lock.cat"
>  (* Release Acquire *)
>  let acq-po = [Acquire] ; po ; [M]
>  let po-rel = [M] ; po ; [Release]
> -let po-unlock-lock-po = po ; [UL] ; (po|rf) ; [LKR] ; po
> +let po-unlock-lock-po = po ; (([UL] ; (po|rf) ; [LKR]) | ([Release]; (po;rf); [Acquire & RMW])) ; po
> 
>  (* Fences *)
>  let R4rmb = R \ Noreturn       (* Reads for which rmb works *)
> 

I am forever struggling with cats, but that does look about right :-)

> although I'm not sure whether there will be actual users that use this
> ordering.

include/asm-generic/ticket_spinlock.h comes to mind, as I thing would
kernel/locking/qspinlock.*, no?

On Thu, Jun 19, 2025 at 05:10:50PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 19, 2025 at 08:00:30AM -0700, Boqun Feng wrote:
> 
> > > So given we build locks from atomics, this has to come from somewhere.
> > > 
> > > The simplest lock -- TAS -- is: rmw.acquire + store.release.
> > > 
> > > So while plain store.release + load.acquire might not make TSO (although
> > > IIRC ARM added variants that do just that in an effort to aid x86
> > > emulation); store.release + rmw.acquire must, otherwise we cannot
> > > satisfy that unlock+lock.
> > 
> > Make sense, so something like this in the model should work:
> > 
> > diff --git a/tools/memory-model/linux-kernel.cat b/tools/memory-model/linux-kernel.cat
> > index d7e7bf13c831..90cb6db6e335 100644
> > --- a/tools/memory-model/linux-kernel.cat
> > +++ b/tools/memory-model/linux-kernel.cat
> > @@ -27,7 +27,7 @@ include "lock.cat"
> >  (* Release Acquire *)
> >  let acq-po = [Acquire] ; po ; [M]
> >  let po-rel = [M] ; po ; [Release]
> > -let po-unlock-lock-po = po ; [UL] ; (po|rf) ; [LKR] ; po
> > +let po-unlock-lock-po = po ; (([UL] ; (po|rf) ; [LKR]) | ([Release]; (po;rf); [Acquire & RMW])) ; po
> > 
> >  (* Fences *)
> >  let R4rmb = R \ Noreturn       (* Reads for which rmb works *)
> > 
> 
> I am forever struggling with cats, but that does look about right :-)
> 

;-) ;-) ;-)

> > although I'm not sure whether there will be actual users that use this
> > ordering.
> 
> include/asm-generic/ticket_spinlock.h comes to mind, as I thing would
> kernel/locking/qspinlock.*, no?
> 

Ah, right. Although I thought users outside lock implementation would be
nice, but you're right, we do have users. Previously our reasoning for
correctness of this particular locking ordering kinda depends on
per-architecture memory model reasoning, so modeling this does make
sense.

Regards,
Boqun

>

On Thu, Jun 19, 2025 at 12:31 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Jun 18, 2025 at 09:49:27AM -0700, Boqun Feng wrote:
>
> > +//! 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.
>
> So I've no clue what the Rust memory model states, and I'm assuming
> it is very similar to the C11 model. I have also forgotten what LKMM
> states :/
>
> Do they all agree on what RELEASE+ACQUIRE makes?

Rust just uses the C11 model outright, so yes it's the same. There's
no separate Rust memory model as far as atomics are concerned.

Alice