Re: [PATCH v6 3/9] rust: sync: atomic: Add ordering annotation types
Andreas Hindborg <[email protected]> Thu, 10 Jul 2025 14:00:59 +0200
| Newsgroups | dev.linux.lists.lkmm,org.kernel.vger.linux-arch,org.kernel.vger.linux-kernel,org.kernel.vger.rust-for-linux |
|---|---|
| Message-ID | <[email protected]> |
"Benno Lossin" <[email protected]> writes: > On Thu Jul 10, 2025 at 8:00 AM CEST, Boqun Feng 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. >> >> 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 `TYPE` associate >> const is for generic function to pick up the particular implementation >> specified by an ordering annotation. >> >> Reviewed-by: Alice Ryhl <[email protected]> >> Signed-off-by: Boqun Feng <[email protected]> > > One naming comment below, with that fixed: > > Reviewed-by: Benno Lossin <[email protected]> > >> --- >> rust/kernel/sync/atomic.rs | 3 + >> rust/kernel/sync/atomic/ordering.rs | 97 +++++++++++++++++++++++++++++ >> 2 files changed, 100 insertions(+) >> create mode 100644 rust/kernel/sync/atomic/ordering.rs > >> +/// The trait bound for annotating operations that support any ordering. >> +pub trait Any: internal::Sealed { > > I don't like the name `Any`, how about `AnyOrdering`? Otherwise we > should require people to write `ordering::Any` because otherwise it's > pretty confusing. I agree with this observation. Best regards, Andreas Hindborg