Re: RFC abstracting atomics

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Jan 10, 2023 at 12:10:33PM -0800, Tyler Retzlaff wrote:
> On Tue, Jan 10, 2023 at 09:16:48AM +0000, Bruce Richardson wrote:
> > 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.
> > > 
> > > 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?
> 
> that is a discussion for the community i think on a per-platform /
> per-toolchain basis. there is likely to be resistance which is why i'm
> favoring the opt-in if you want model.
> 
> some potential arguments against switching.
> 
> * it's an abi break there is no way around it.
> * old compiler x stdatomics implementation is less optimal than
>   old compiler x __atomics (potential argument).
> * there's some "mixed" use of variables in the tree where sometimes we
>   operate on them as if they are atomic and sometimes not. the std
>   atomics apis doesn't support this sometimes atomic codegen you either
>   get atomic or you don't.
> * direct use of atomics default to seq_cst ordering if the strengthening
>   of the ordering is not desired each variable needs to be hunted down
>   and explicitly returned to relaxed ordering access.
> 
> > 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?
> 
> the abstraction as i have prototyped it is a set of conditionally
> compiled macros where the macros mirror the standard c atomics for
> naming and have been placed in include/generic/rte_atomics.h
> 
> i've no problem splitting it out into a separate library and then have
> EAL depend on it, but it is currently header only so i'm not sure if it
> is worth doing for that.
> 
> we can chew on that more in a couple days when i submit the base series
> if you like.
> 
Thanks.

One additional point that just became clear to me when I started thinking
about upping our DPDK C-standard-baseline. We need to be careful what we
are considering when we up our C baseline. We can mandate a specific
compiler minimum and C version for compiling up DPDK itself, but I think we
should not mandate that for the end applications.

That means that our header files, such as atomics, should not require C99
or C11 even if the build of DPDK itself does. More specifically, even if we
bump DPDK minimum to C11, we should still allow apps to build using older
compiler settings.

Therefore, we probably need to maintain non-C11 atomics code paths in
headers beyond the point at which DPDK itself uses C11 as a code baseline.

/Bruce