Re: RFC abstracting atomics

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Tue, Jan 10, 2023 at 12:45:05PM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Tuesday, 10 January 2023 10.17
> > 
> > On Mon, Jan 09, 2023 at 02:56:04PM -0800, Tyler Retzlaff wrote:
> > > hi folks,
> > >
> > > i would like to introduce a layer of abstraction that would allow
> > > optional use of standard C11 atomics when the platform / toolchain
> > > combination has them available.
> 
> It would be great if that abstraction layer could expose the exact same (or very similar) interface as standard C11 atomics, so the API is given in advance, and developers don't have to learn the API of yet another shim. It would also make it easier getting rid of the abstraction layer at a later time.

the patch series i have prototype exactly mirrors the stdandard atomics
from C11. it requires a few fixups to fill convenience builtins that the
standard doesn't provide e.g. `__atomic_add_fetch(&o, v, order)` becomes
`__atomic_fetch_add(&o, v, order) + v;` sort of stuff, mostly minor.

> 
> > >
> > > making the option usable would be a phased approach intended to focus
> > > review and minimize dealing with churn on such a broad change.
> > >
> > > 1. provide an initial series to add the abstraction and the ability
> > >    control enablement with a meson option enable_stdatomics=false
> > will
> > >    be the default.
> > >
> > >    for all existing platform / toolchain combinations the default
> > would
> > >    remain false. i.e. i have no plans to enable it for existing
> > platforms
> > >    toolchain combinations but leaves a change of default open to the
> > >    community as a future discussion if it is desired.
> > >
> > > 2. once the initial abstraction is integrated a series will be
> > introduced to
> > >    port the tree to the abstraction with enable_stdatomics=false. the
> > goal
> > >    being low or no change to the current use of gcc builtin C++11
> > memory
> > >    model atomics.
> > >
> > > 3. once the tree is ported a final series will be introduced to
> > introduce
> > >    the remaining change to allow the use of enable_stdatomics=true.
> > >
> > > would appreciate any assistance / suggestions you can provide to
> > > introduce the abstraction smoothly.
> > >
> > 
> > Plan generally sounds ok. However, beyond point #3, would there then be
> > plans to remove the option and always use stdatomics in future?
> 
> We are already trying to migrate atomics in DPDK from using __sync built-ins (used in rte_atomic) to using __atomic built-ins; e.g. new contributions are recommended to use the latter (and not rte_atomic). When DPDK migrates to C11, I assume stdatomics will become the new standard for atomics in DPDK, and the process for migrating to stdatomics will resemble the currently ongoing process for migrating from rte_atomic to __atomic built-ins, which seems to be relatively uncomplicated (but slow).

the few __sync left in use are easily converted. there might be some
argument against converting __sync -> __atomic because of sub-optimal
codegen on older gcc for the semantically equivalent replacements.

the only comment i have on current/existing rte_atomic is they're
annoyingly stealing a little bit of the rte_ namespace that would allow
convenient alignment with the standard names. also some of them provide
only signed or unsigned variations which forces kind of non-standardy
things to be done in the code but are perfectly acceptable when using
the gcc implementation.

with the set of macros i'm introducing migrating to direct use of C11
atomics would likely be just a big search and replace (or that is my
goal).

> Another thing: The proposed layer of abstraction should only live until DPDK supports C11, which is hopefully relatively soon. So is such a (hopefully) short-lived abstraction layer worth the effort?

i kept hoping we would just baseline on C11 but after prototyping the
abstraction i realize there may be some valid arguments for not moving
to stdatomics now or possibly ever for some platform/toolchain
combinations (so long as they are declared supported). given the drawn
out discussion about RHEL-7 has shown that the migration path may be
long term. more possible headwinds were listed in my reply to Bruce.

> 
> > 
> > To slightly expand the scope of the discussion - would it be worthwhile
> > putting these abstractions in a new library in DPDK other than EAL, to
> > start the process of splitting out some of the lower-level material
> > from
> > that library?
> 
> While I applaud this idea, I think we first need to discuss in detail how to split up EAL, and especially what new structure we want to end up with instead. E.g. we already have architecture specific code in various libraries and drivers, which proves that architecture specific stuff does not necessarily need to go into the EAL. We also need to discuss challenges/barriers to separating stuff from the EAL, and how we can solve those challenges; e.g. some parts are inside the EAL only due to startup sequence requirements, which might be solvable by offering better startup sequencing support for libraries in the EAL. Perhaps the EAL has even outlived itself, and could be replaced by set of DPDK Core libraries instead - some could be optional (at build time). But let's make this a separate discussion.

i gave some detail about this to my reply with Bruce. i think when i
post the series we can discuss it more. but for atomics i expect it
isn't worth separating out.