RE: [PATCH] eal: introduce atomics abstraction

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

<snip>

> > > > >
> > > > > >
> > > > > > For environments where stdatomics are not supported, we could
> > > have a
> > > > > stdatomic.h in DPDK implementing the same APIs (we have to
> > > > > support
> > > only
> > > > > _explicit APIs). This allows the code to use stdatomics APIs and
> > > when we move
> > > > > to minimum supported standard C11, we just need to get rid of
> > > > > the
> > > file in DPDK
> > > > > repo.
> > > > >
> > > > > my concern with this is that if we provide a stdatomic.h or
> > > introduce names
> > > > > from stdatomic.h it's a violation of the C standard.
> > > > >
> > > > > references:
> > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > >  * GNU libc manual
> > > > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > > Names.html
> > > > >
> > > > > in effect the header, the names and in some instances namespaces
> > > introduced
> > > > > are reserved by the implementation. there are several reasons in
> > > the GNU libc
> > > > Wouldn't this apply only after the particular APIs were introduced?
> > > i.e. it should not apply if the compiler does not support stdatomics.
> > >
> > > yeah, i agree they're being a bit wishy washy in the wording, but
> > > i'm not convinced glibc folks are documenting this as permissive
> > > guidance against.
> > >
> > > >
> > > > > manual that explain the justification for these reservations and
> > > > > if
> > > if we think
> > > > > about ODR and ABI compatibility we can conceive of others.
> > > > >
> > > > > i'll also remark that the inter-mingling of names from the POSIX
> > > standard
> > > > > implicitly exposed as a part of the EAL public API has been
> > > problematic for
> > > > > portability.
> > > > These should be exposed as EAL APIs only when compiled with a
> > > compiler that does not support stdatomics.
> > >
> > > you don't necessarily compile dpdk, the application or its other
> > > dynamically linked dependencies with the same compiler at the same
> > > time.
> > > i.e. basically the model of any dpdk-dev package on any linux
> > > distribution.
> > >
> > > if dpdk is built without real stdatomic types but the application
> > > has to interoperate with a different kit or library that does they
> > > would be forced to dance around dpdk with their own version of a
> > > shim to hide our faked up stdatomics.
> > >
> >
> > So basically, if we want a binary DPDK distribution to be compatible with a
> separate application build environment, they both have to implement atomics
> the same way, i.e. agree on the ABI for atomics.
> >
> > Summing up, this leaves us with only two realistic options:
> >
> > 1. Go all in on C11 stdatomics, also requiring the application build
> environment to support C11 stdatomics.
> > 2. Provide our own DPDK atomics library.
> >
> > (As mentioned by Tyler, the third option - using C11 stdatomics inside
> > DPDK, and requiring a build environment without C11 stdatomics to
> > implement a shim - is not realistic!)
> >
> > I strongly want atomics to be available for use across inline and compiled
> code; i.e. it must be possible for both compiled DPDK functions and inline
> functions to perform atomic transactions on the same atomic variable.
> 
> i consider it a mandatory requirement. i don't see practically how we could
> withdraw existing use and even if we had clean way i don't see why we would
> want to. so this item is defintely settled if you were concerned.
I think I agree here.

> 
> >
> > So either we upgrade the DPDK build requirements to support C11 (including
> the optional stdatomics), or we provide our own DPDK atomics.
> 
> i think the issue of requiring a toolchain conformant to a specific standard is a
> separate matter because any adoption of C11 standard atomics is a potential
> abi break from the current use of intrinsics.
I am not sure why you are calling it as ABI break. Referring to [1], I just see wrappers around intrinsics (though [2] does not use the intrinsics).

[1] https://github.com/gcc-mirror/gcc/blob/master/gcc/ginclude/stdatomic.h
[2] https://github.com/llvm-mirror/clang/blob/master/lib/Headers/stdatomic.h

> 
> the abstraction (whatever namespace it resides) allows the existing
> toolchain/platform combinations to maintain compatibility by defaulting to
> current non-standard intrinsics.
How about using the intrinsics (__atomic_xxx) name space for abstraction? This covers the GCC and Clang compilers.
If there is another platform that uses the same name space for something else, I think DPDK should not be supporting that platform.
What problems do you see?

> 
> once in place it provides an opportunity to introduce new toolchain/platform
> combinations and enables an opt-in capability to use stdatomics on existing
> toolchain/platform combinations subject to community discussion on
> how/if/when.
> 
> it would be good to get more participants into the discussion so i'll cc techboard
> for some attention. i feel like the only area that isn't decided is to do or not do
> this in rte_ namespace.
> 
> i'm strongly in favor of rte_ namespace after discussion, mainly due to to
> disadvantages of trying to overlap with the standard namespace while not
> providing a compatible api/abi and because it provides clear disambiguation of
> that difference in semantics and compatibility with the standard api.
> 
> so far i've noted the following
> 
> * we will not provide the non-explicit apis.
+1

> * we will make no attempt to support operate on struct/union atomics
>   with our apis.
+1

> * we will mirror the standard api potentially in the rte_ namespace to
>   - reference the standard api documentation.
>   - assume compatible semantics (sans exceptions from first 2 points).
> 
> my vote is to remove 'potentially' from the last point above for reasons
> previously discussed in postings to the mail thread.
> 
> thanks all for the discussion, i'll send up a patch removing non-explicit apis for
> viewing.
> 
> ty