Hi Stephen, Honnappa,
> -----Original Message-----
> From: Stephen Hemminger <step...@networkplumber.org>
> Sent: Tuesday, December 24, 2019 12:37 AM
> To: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>
> Cc: Joyce Kong <joyce.k...@arm.com>; tho...@monjalon.net;
> david.march...@redhat.com; m...@smartsharesystems.com;
> jer...@marvell.com; bruce.richard...@intel.com; ravi1.ku...@amd.com;
> rm...@marvell.com; shsha...@marvell.com; xuanziya...@huawei.com;
> cloud.wangxiao...@huawei.com; zhouguoy...@huawei.com; Phil Yang
> <phil.y...@arm.com>; Gavin Hu <gavin...@arm.com>; nd <n...@arm.com>;
> dev@dpdk.org
> Subject: Re: [PATCH v6 1/6] lib/eal: implement the family of rte bit operation
> APIs
>
> On Mon, 23 Dec 2019 05:04:12 +0000
> Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> wrote:
>
> > <snip>
> >
> > >
> > > On Sat, 21 Dec 2019 16:07:23 +0000
> > > Honnappa Nagarahalli <honnappa.nagaraha...@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/vi1pr08mb53766c30b5cda00fb9fce9678f...@vi1pr08mb5376.eurprd08.prod.outlook.com/
Any thoughts and comments are welcome!
