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

[Gary Guo]

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.
>> 
>> That's what we have today, hence the implementation upgrades them to full 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 barriers.
>> 
>> 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.
>> 
>> 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 benefit of
>> having dma_mb_acquire/dma_mb_release compared to full dma_mb) before proposing so.
>> 
>> Note that marked dma_load_acquire/dma_store_release (and their mandatory
>> versions) don't make too much sense, as AFAIK no architectures have instructions
>> for them so you're implementing these as fence instructions anyway.
>
> Ah, so you're proposing new memory barrier types that don't exist anywhere
> in the kernel yet. I thought you were just wrapping existing ones so it got
> 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 enough?

How about change the wording in documentation like this:

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

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_cst,
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