Re: [PATCH 2/3] rust: sync: generic memory barriers

On Fri Apr 3, 2026 at 10:33 PM BST, Joel Fernandes wrote:
>
>
> On 4/2/2026 8:07 PM, Gary Guo wrote:
>> On Thu Apr 2, 2026 at 10:49 PM BST, Joel Fernandes wrote:
>
> [...]
>
>>> See also in Documentation/memory-barriers.txt, ACQUIRE and RELEASE are =
defined as being
>>> tied to specific memory operations.
>>=20
>> That's what we have today, hence the implementation upgrades them to ful=
l memory
>> barriers. But ACQUIRE and RELEASE orderings doesn't *need* to be tied to
>> specific memory operations and they can still make conceptual sense as b=
arriers.
>>=20
>> C11 memory model defines Acquire and Release fences, and it looks to me =
it's
>> relatively easy to add it to LKMM. I was playing with Herd7 and I think =
I've got
>> it working, see the attached diff.
>>=20
>> Another thing that I'd like to note is that in all architectures that we=
 have
>> today except ARM and PARISC, the smp_load_acquire and smp_store_release =
are
>> actually implemented as READ_ONCE + ACQUIRE barrier and RELEASE barrier =
+
>> WRITE_ONCE. I'm planning to propose C API and corresponding memory model=
 change
>> too, but I want to gather some more concrete numbers (the performance be=
nefit of
>> having dma_mb_acquire/dma_mb_release compared to full dma_mb) before pro=
posing so.
>>=20
>> Note that marked dma_load_acquire/dma_store_release (and their mandatory
>> versions) don't make too much sense, as AFAIK no architectures have inst=
ructions
>> for them so you're implementing these as fence instructions anyway.
>
> Ah, so you're proposing new memory barrier types that don't exist anywher=
e
> in the kernel yet. I thought you were just wrapping existing ones so it g=
ot
> a bit confusing.

Well, I am not proposing new memory barrier types in this change, hence I
mentioned that these are annotations about intention and documentation and =
not
about their *semantics* currently. Perhaps I didn't phrase this clear enoug=
h?

How about change the wording in documentation like this:

    /// `Acquire` is a full memory barrier, but caller indicates that it on=
ly
    /// uses it to provide LOAD->{LOAD,STORE} ordering.=20

or do you think even that is too much?

>
> My suggestion: fix the nova-core issue in patch 3 using dma_mb() directly=
,
> without the new barrier types. Adding a new standalone release/acquire
> fence semantic to the kernel is something that will need broad consensus
> from the LKMM maintainers IMO, independent of this series. It's a bigger
> conversation that could delay your nova bug fix.

Something like this?

    // TODO: Replace with `dma_mb(Release)` when it's available.
    dma_mb(Full);

Or not even mentioning that? I really like the Acquire/Release because they
serve themselves a documentation on why the barrier is there to exist.

When I read a full barrier, I always need to think whether the code really
relies on STORE->LOAD ordering or a weaker ordering that could be used.
There's a conventional wisdom in userspace C/Rust where when you see seq_cs=
t,
99% of the time it's wrong.

>
> Once smp_mb_release() / smp_mb_acquire() (or the DMA equivalents) land
> properly, updating nova to use them will be trivial. But the correctness =
of
> the fix shouldn't depend on semantics that don't exist in the kernel yet,
> even if they currently degrade to a full barrier.
>
> That said, I find the idea genuinely interesting, and thanks for keeping =
me
> in the loop. Standalone acquire/release fences that are formally modeled =
in
> LKMM and A-cumulative is interesting as you showed in the herd7 code. You
> would want to update Documentation/memory-barriers.txt too.

Yeah, there're more works to do on changing the semantics, hence I want to =
limit
this to not be about semantics.

Best,
Gary