Hi Oliver,
> -----Original Message-----
> From: Olivier Matz <olivier.m...@6wind.com>
> Sent: Thursday, July 9, 2020 7:04 PM
> To: Phil Yang <phil.y...@arm.com>
> Cc: dev@dpdk.org; step...@networkplumber.org;
> david.march...@redhat.com; d...@linux.vnet.ibm.com; Honnappa
> Nagarahalli <honnappa.nagaraha...@arm.com>; Ruifeng Wang
> <ruifeng.w...@arm.com>; nd <n...@arm.com>
> Subject: Re: [PATCH v3] mbuf: use C11 atomic built-ins for refcnt operations
>
> Hi Phil,
>
> On Thu, Jul 09, 2020 at 06:10:42PM +0800, Phil Yang wrote:
> > Use C11 atomic built-ins with explicit ordering instead of rte_atomic
> > ops which enforce unnecessary barriers on aarch64.
> >
> > Signed-off-by: Phil Yang <phil.y...@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.w...@arm.com>
> > ---
> > v3:
> > 1.Fix ABI breakage.
> > 2.Simplify data type cast.
> >
> > v2:
> > Fix ABI issue: revert the rte_mbuf_ext_shared_info struct refcnt field
> > to refcnt_atomic.
> >
> > lib/librte_mbuf/rte_mbuf.c | 1 -
> > lib/librte_mbuf/rte_mbuf.h | 19 ++++++++++---------
> > lib/librte_mbuf/rte_mbuf_core.h | 2 +-
> > 3 files changed, 11 insertions(+), 11 deletions(-)
> >
<snip>
> >
> > /* Reinitialize counter before mbuf freeing. */
> > diff --git a/lib/librte_mbuf/rte_mbuf_core.h
> b/lib/librte_mbuf/rte_mbuf_core.h
> > index 16600f1..d65d1c8 100644
> > --- a/lib/librte_mbuf/rte_mbuf_core.h
> > +++ b/lib/librte_mbuf/rte_mbuf_core.h
> > @@ -679,7 +679,7 @@ typedef void
> (*rte_mbuf_extbuf_free_callback_t)(void *addr, void *opaque);
> > struct rte_mbuf_ext_shared_info {
> > rte_mbuf_extbuf_free_callback_t free_cb; /**< Free callback
> function */
> > void *fcb_opaque; /**< Free callback argument */
> > - rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
> > + uint16_t refcnt_atomic; /**< Atomically accessed refcnt */
> > };
>
> To avoid an API breakage (i.e. currently, an application that accesses
> to refcnt_atomic expects that its type is rte_atomic16_t), I suggest to
> do the same than in the mbuf struct:
>
> union {
> rte_atomic16_t refcnt_atomic;
> uint16_t refcnt;
> };
>
> I hope the ABI checker won't complain.
>
> It will also be better for 20.11 when the deprecated fields will be
> renamed: the remaining one will be called 'refcnt' in both mbuf and
> mbuf_ext_shared_info.
Got it. I agree with you.
It should work. In my local test machine, the ABI checker happy with this
approach.
Once the test is done, I will upstream the new patch.
Appreciate your comments.
Thanks,
Phil
