Hi,
On Tue, Jul 07, 2020 at 06:10:33PM +0800, Phil Yang wrote:
> Use C11 atomics 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>
> ---
> 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 | 11 +++--------
> 3 files changed, 13 insertions(+), 18 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..4a7a98c 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 (uint16_t)(__atomic_add_fetch((int16_t *)&m->refcnt, 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 (uint16_t)(__atomic_add_fetch((int16_t *)&shinfo->refcnt_atomic,
> + 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((int *)&shinfo->refcnt_atomic, -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..806313a 100644
> --- a/lib/librte_mbuf/rte_mbuf_core.h
> +++ b/lib/librte_mbuf/rte_mbuf_core.h
> @@ -18,7 +18,6 @@
>
> #include <stdint.h>
> #include <rte_compat.h>
> -#include <generic/rte_atomic.h>
>
> #ifdef __cplusplus
> extern "C" {
> @@ -495,12 +494,8 @@ struct rte_mbuf {
> * or non-atomic) is controlled by the CONFIG_RTE_MBUF_REFCNT_ATOMIC
> * config option.
> */
> - RTE_STD_C11
> - union {
> - rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
> - /** Non-atomically accessed refcnt */
> - uint16_t refcnt;
> - };
> + uint16_t refcnt;
> +
It seems this patch does 2 things:
- remove refcnt_atomic
- use C11 atomics
The first change is an API break. I think it should be announced in a
deprecation
notice. The one about atomic does not talk about it.
So I suggest to keep refcnt_atomic until next version.
> uint16_t nb_segs; /**< Number of segments. */
>
> /** Input port (16 bits to support more than 256 virtual ports).
> @@ -679,7 +674,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 */
> };
>
> /**< Maximum number of nb_segs allowed. */
> --
> 2.7.4
>
