Re: [PATCH v5 03/10] rust: sync: atomic: Add ordering annotation types

Boqun Feng
2025-06-23T02:48:13Z

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