[dpdk-dev] [PATCH v6 1/6] lib/eal: implement the family of rte bit operation APIs

[Honnappa Nagarahalli]

<snip>
> >
> > > > >
> > > > > >
> > > > > > On Sat, 21 Dec 2019 16:07:23 +0000 Honnappa Nagarahalli
> > > > > > <Honnappa.Nagarahalli at arm.com> wrote:
> > > > > >
> > > > > > > Converting these into macros will help remove the size based
> > > > > > > duplication
> > > > of
> > > > > > APIs. I came up with the following macro:
> > > > > > >
> > > > > > > #define RTE_GET_BIT(nr, var, ret, memorder) \ ({ \
> > > > > > >     if (sizeof(var) == sizeof(uint32_t)) { \
> > > > > > >         uint32_t mask1 = 1U << (nr)%32; \
> > > > > > >         ret = __atomic_load_n(&var, (memorder)) & mask1;\
> > > > > > >     } \
> > > > > > >     else {\
> > > > > > >         uint64_t mask2 = 1UL << (nr)%64;\
> > > > > > >         ret = __atomic_load_n(&var, (memorder)) & mask2;\
> > > > > > >     } \
> > > > > > > })
> > > > > >
> > > > > > Macros are more error prone. Especially because this is in
> > > > > > exposed header
> > > > file
> > > > > That's another question I have. Why do we need to have these
> > > > > APIs in a
> > > > public header file? These will add to the ABI burden as well.
> > > > These APIs should be in a common-but-not-public header file. I am
> > > > also not sure how helpful these APIs are for applications as these
> > > > APIs seem to have considered requirements only from the PMDs.
> > > >
> > > > Why do we have to wrap every C atomic builtin? What value is there in
> that?
> > >
> > > The wrapping is aimed to reduce code duplication, on average 3 lines
> > > cut down to 1 line for a single core.
> > > Overall I am thinking this bitops APIs are targeted for use by PMDs
> > > only, applications can use C11 freely.
> > > The initial thought for the new APIs came from the idea of
> > > consolidating the scattered bit operations all over the PMDs. It is
> > > unwise to expanding to applications or libraries, as different
> > > memory orderings are required and complexity generate.
> > >
> > > If the use cases are limited to PMDs, a 'volatile' or a compiler
> > > barrier is sufficient therefore the number of APIs can be saved by half.
> > >
> http://inbox.dpdk.org/dev/VI1PR08MB53766C30B5CDA00FB9FCE9678F2E0
> > > @VI1PR08MB5376.eurprd08.prod.outlook.com/
> > >
> > > Any thoughts and comments are welcome!
> > I would prefer that the APIs/Macros just address PMD's requirements.
> These also should be kept private (through naming conventions?). Given that
> the current PMDs are not using C11, we can skip using C11 atomics in these
> APIs.
> 
> Not in favor, just use existing Gcc/clang/icc atomics instead of creating
> unnecessary bloat wrappers.
I thought, you blessed this patch [1]. 

[1] http://mails.dpdk.org/archives/dev/2019-October/147297.html