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

[Joel Fernandes <joelagnelf@nvidia.com>]

On 4/4/2026 8:43 AM, Gary Guo wrote:
> 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?

You are conflating lots of terms here, a "full memory barrier" is much stronger,
has stronger cumulativity properties etc (even stronger than Acquire/Release).

And I do think it is too much, because you are using the name of existing memory
barrier types (Acquire/Release) with specific definition, but saying "This is an
acquire lets pretend its a full memory barrier".  So you are in effect defining
new types IMO.

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

Sure that's certainly better, once the Release stuff that you're sort of
re-defining is accepted by LKMM maintainers and others, it can be replaced for
sure. But I wouldn't even want to mention a comment until this type of
"Standalone Release" is something everyone (LKMM reviewers) agree with and is
upstream-bound.  Just split the patch series into "Bug fix" and "New memory
barrier things".

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

My gut feeling is, by calling it a Release (something that already has specific
definition), the marketing of this is probably going to be one of any hurdles.
:) But I'm curious about the take of others on this thread.

thanks,

--
Joel Fernandes