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

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 7 Jan 2020 00:44:51 +0000
Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:

> <snip>
> 
> > > >  
> > > > >
> > > > > On Sat, 21 Dec 2019 16:07:23 +0000 Honnappa Nagarahalli
> > > > > <Honnappa.Nagarahalli@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.