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(-) > > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > index ae91ae2..8a456e5 100644 > --- a/lib/librte_mbuf/rte_mbuf.c > +++ b/lib/librte_mbuf/rte_mbuf.c > @@ -22,7 +22,6 @@ > #include <rte_eal.h> > #include <rte_per_lcore.h> > #include <rte_lcore.h> > -#include <rte_atomic.h> > #include <rte_branch_prediction.h> > #include <rte_mempool.h> > #include <rte_mbuf.h> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index f8e492e..c1c0956 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -37,7 +37,6 @@ > #include <rte_config.h> > #include <rte_mempool.h> > #include <rte_memory.h> > -#include <rte_atomic.h> > #include <rte_prefetch.h> > #include <rte_branch_prediction.h> > #include <rte_byteorder.h> > @@ -365,7 +364,7 @@ rte_pktmbuf_priv_flags(struct rte_mempool *mp) > static inline uint16_t > rte_mbuf_refcnt_read(const struct rte_mbuf *m) > { > - return (uint16_t)(rte_atomic16_read(&m->refcnt_atomic)); > + return __atomic_load_n(&m->refcnt, __ATOMIC_RELAXED); > } > > /** > @@ -378,14 +377,15 @@ rte_mbuf_refcnt_read(const struct rte_mbuf *m) > static inline void > rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value) > { > - rte_atomic16_set(&m->refcnt_atomic, (int16_t)new_value); > + __atomic_store_n(&m->refcnt, new_value, __ATOMIC_RELAXED); > } > > /* internal */ > static inline uint16_t > __rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value) > { > - return (uint16_t)(rte_atomic16_add_return(&m->refcnt_atomic, value)); > + return __atomic_add_fetch(&m->refcnt, (uint16_t)value, > + __ATOMIC_ACQ_REL); > } > > /** > @@ -466,7 +466,7 @@ rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t > new_value) > static inline uint16_t > rte_mbuf_ext_refcnt_read(const struct rte_mbuf_ext_shared_info *shinfo) > { > - return (uint16_t)(rte_atomic16_read(&shinfo->refcnt_atomic)); > + return __atomic_load_n(&shinfo->refcnt_atomic, __ATOMIC_RELAXED); > } > > /** > @@ -481,7 +481,7 @@ static inline void > rte_mbuf_ext_refcnt_set(struct rte_mbuf_ext_shared_info *shinfo, > uint16_t new_value) > { > - rte_atomic16_set(&shinfo->refcnt_atomic, (int16_t)new_value); > + __atomic_store_n(&shinfo->refcnt_atomic, new_value, __ATOMIC_RELAXED); > } > > /** > @@ -505,7 +505,8 @@ rte_mbuf_ext_refcnt_update(struct > rte_mbuf_ext_shared_info *shinfo, > return (uint16_t)value; > } > > - return (uint16_t)rte_atomic16_add_return(&shinfo->refcnt_atomic, value); > + return __atomic_add_fetch(&shinfo->refcnt_atomic, (uint16_t)value, > + __ATOMIC_ACQ_REL); > } > > /** Mbuf prefetch */ > @@ -1304,8 +1305,8 @@ static inline int > __rte_pktmbuf_pinned_extbuf_decref(struct rte_mbuf *m) > * Direct usage of add primitive to avoid > * duplication of comparing with one. > */ > - if (likely(rte_atomic16_add_return > - (&shinfo->refcnt_atomic, -1))) > + if (likely(__atomic_add_fetch(&shinfo->refcnt_atomic, (uint16_t)-1, > + __ATOMIC_ACQ_REL))) > return 1; > > /* 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. Olivier