RE: [RFC v7 3/6] eal: add exactly-once bit access functions

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Mattias Rönnblom [mailto:hofors@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:hofors@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.ronnblom@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.

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.ronnblom@ericsson.com>
> >>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >>>> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >>>> ---
> >>>