On 2024-05-08 10:11, Morten Brørup wrote:
From: Mattias Rönnblom [mailto:hof...@lysator.liu.se]
Sent: Wednesday, 8 May 2024 10.00

On 2024-05-08 09:33, Morten Brørup wrote:
From: Mattias Rönnblom [mailto:hof...@lysator.liu.se]
Sent: Wednesday, 8 May 2024 08.47

On 2024-05-07 21:17, Morten Brørup wrote:
From: Mattias Rönnblom [mailto:mattias.ronnb...@ericsson.com]
Sent: Sunday, 5 May 2024 10.38

Add test/set/clear/assign/flip functions which prevents certain
compiler optimizations and guarantees that program-level memory loads
and/or stores will actually occur.

These functions are useful when interacting with memory-mapped
hardware devices.

The "once" family of functions does not promise atomicity and provides
no memory ordering guarantees beyond the C11 relaxed memory model.

In another thread, Stephen referred to the extended discussion on memory
models in Linux kernel documentation:
https://www.kernel.org/doc/html/latest/core-api/wrappers/memory-
barriers.html

Unlike the "once" family of functions in this RFC, the "once" family of
functions in the kernel also guarantee memory ordering, specifically for
memory-mapped hardware devices. The document describes the rationale with
examples.


What more specifically did you have in mind? READ_ONCE() and
WRITE_ONCE()? They give almost no guarantees. Very much relaxed.

The way I read it, they do provide memory ordering guarantees.


Sure. All types memory operations comes with some kind guarantees. A
series of non-atomic, non-volatile stores issued by a particular thread
are guaranteed to happen in program order, from the point of view of
that thread, for example. Would be hard to write a program if that
wasn't true.

"This macro does not give any guarantees in regards to memory ordering /../"

This is not true. I will rephrase to "any *additional* guarantees" for
both plain and "once" family documentation.

Consider code like this:
set_once(HW_START_BIT);
while (!get_once(HW_DONE_BIT)) /*busy wait*/;

If the "once" functions are used for hardware access, they must guarantee that 
HW_START_BIT has been written before HW_DONE_BIT is read.


Provided bits reside in the same word, there is (or at least, should be) such guarantee, and otherwise, you'll need a barrier.

I'm guessing in most cases the requirements are actually not as strict as you pose them: DONE starts as 0, so it may actually be read before START is written to, but not all DONE reads can be reordered ahead of the single START write. In that case, a compiler barrier between set and the get loop should suffice. Otherwise, you need a full barrier, or an I/O barrier.

Anyway, since the exact purpose of the "once" type bit operations is unclear, maybe I should drop them from the patch set.

Now, they are much like the Linux kernel's __set_bit(), but for hardware access, maybe they should be more like writel().

The documentation must reflect this ordering guarantee.


Ignore that the kernel's "once" functions operates on words and this RFC
operates on bits, the behavior is the same. Either there are memory ordering
guarantees, or there are not.


I've read that document.

What you should keep in mind if you read that document, is that DPDK
doesn't use the kernel's memory model, and doesn't have the kernel's
barrier and atomics APIs. What we have are an obsolete, miniature
look-alike in <rte_atomic.h> and something C11-like in <rte_stdatomic.h>.

My general impression is that DPDK was moving in the C11 direction
memory model-wise, which is not the model the kernel uses.

I think you and I agree that using legacy methods only because "the kernel
does it that way" would not be the optimal roadmap for DPDK.

We should keep moving in the C11 direction memory model-wise.
I consider it more descriptive, and thus expect compilers to eventually
produce better optimized code.


It makes me think that DPDK "once" family of functions should behave
similarly.

I think they do already.

I haven't looked deep into it, but the RFC's documentation says otherwise:
The "once" family of functions does not promise atomicity and provides *no
memory ordering* guarantees beyond the C11 relaxed memory model.


Also, rte_bit_once_set() works as the kernel's __set_bit().

Alternatively, if the "once" family of functions cannot be generically
implemented with a memory ordering that is optimal for all use cases, drop
this family of functions, and instead rely on the "atomic" family of
functions
for interacting with memory-mapped hardware devices.


RFC v7:
    * Fix various minor issues in documentation.

RFC v6:
    * Have rte_bit_once_test() accept const-marked bitsets.

RFC v3:
    * Work around lack of C++ support for _Generic (Tyler Retzlaff).

Signed-off-by: Mattias Rönnblom <mattias.ronnb...@ericsson.com>
Acked-by: Morten Brørup <m...@smartsharesystems.com>
Acked-by: Tyler Retzlaff <roret...@linux.microsoft.com>
---

Reply via email to