From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 20F1B41C49; Thu, 9 Feb 2023 09:05:53 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 026F84113D; Thu, 9 Feb 2023 09:05:53 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 0DABA410EA for ; Thu, 9 Feb 2023 09:05:51 +0100 (CET) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v2] eal: introduce atomics abstraction Date: Thu, 9 Feb 2023 09:05:46 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87718@smartserver.smartshare.dk> In-Reply-To: <1675892618-31755-2-git-send-email-roretzla@linux.microsoft.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v2] eal: introduce atomics abstraction Thread-Index: Adk8Bm6VHvKu3YTSQUuRvTQJx0tZCwAVCLqQ References: <1673558785-24992-1-git-send-email-roretzla@linux.microsoft.com> <1675892618-31755-1-git-send-email-roretzla@linux.microsoft.com> <1675892618-31755-2-git-send-email-roretzla@linux.microsoft.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Tyler Retzlaff" , Cc: , , , X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > Sent: Wednesday, 8 February 2023 22.44 >=20 > Introduce atomics abstraction that permits optional use of standard = C11 > atomics when meson is provided the new enable_stdatomics=3Dtrue = option. >=20 > Signed-off-by: Tyler Retzlaff > --- Looks good. A few minor suggestions about implementation only. With or without suggested modifications, Acked-by: Morten Br=F8rup > config/meson.build | 11 ++++ > lib/eal/arm/include/rte_atomic_32.h | 6 ++- > lib/eal/arm/include/rte_atomic_64.h | 6 ++- > lib/eal/include/generic/rte_atomic.h | 96 > +++++++++++++++++++++++++++++++++- > lib/eal/loongarch/include/rte_atomic.h | 6 ++- > lib/eal/ppc/include/rte_atomic.h | 6 ++- > lib/eal/riscv/include/rte_atomic.h | 6 ++- > lib/eal/x86/include/rte_atomic.h | 8 ++- > meson_options.txt | 2 + > 9 files changed, 139 insertions(+), 8 deletions(-) >=20 > diff --git a/config/meson.build b/config/meson.build > index 26f3168..25dd628 100644 > --- a/config/meson.build > +++ b/config/meson.build > @@ -255,6 +255,17 @@ endif > # add -include rte_config to cflags > add_project_arguments('-include', 'rte_config.h', language: 'c') >=20 > +stdc_atomics_enabled =3D get_option('enable_stdatomics') > +dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled) > + > +if stdc_atomics_enabled > +if cc.get_id() =3D=3D 'gcc' or cc.get_id() =3D=3D 'clang' > + add_project_arguments('-std=3Dgnu11', language: 'c') > +else > + add_project_arguments('-std=3Dc11', language: 'c') > +endif > +endif > + > # enable extra warnings and disable any unwanted warnings > # -Wall is added by default at warning level 1, and -Wextra > # at warning level 2 (DPDK default) > diff --git a/lib/eal/arm/include/rte_atomic_32.h > b/lib/eal/arm/include/rte_atomic_32.h > index c00ab78..7088a12 100644 > --- a/lib/eal/arm/include/rte_atomic_32.h > +++ b/lib/eal/arm/include/rte_atomic_32.h > @@ -34,9 +34,13 @@ > #define rte_io_rmb() rte_rmb() >=20 > static __rte_always_inline void > -rte_atomic_thread_fence(int memorder) > +rte_atomic_thread_fence(rte_memory_order memorder) > { > +#ifdef RTE_STDC_ATOMICS > + atomic_thread_fence(memorder); > +#else > __atomic_thread_fence(memorder); > +#endif > } >=20 > #ifdef __cplusplus > diff --git a/lib/eal/arm/include/rte_atomic_64.h > b/lib/eal/arm/include/rte_atomic_64.h > index 6047911..7f02c57 100644 > --- a/lib/eal/arm/include/rte_atomic_64.h > +++ b/lib/eal/arm/include/rte_atomic_64.h > @@ -38,9 +38,13 @@ > #define rte_io_rmb() rte_rmb() >=20 > static __rte_always_inline void > -rte_atomic_thread_fence(int memorder) > +rte_atomic_thread_fence(rte_memory_order memorder) > { > +#ifdef RTE_STDC_ATOMICS > + atomic_thread_fence(memorder); > +#else > __atomic_thread_fence(memorder); > +#endif > } >=20 > /*------------------------ 128 bit atomic operations = ----------------- > --------*/ > diff --git a/lib/eal/include/generic/rte_atomic.h > b/lib/eal/include/generic/rte_atomic.h > index f5c49a9..392d928 100644 > --- a/lib/eal/include/generic/rte_atomic.h > +++ b/lib/eal/include/generic/rte_atomic.h > @@ -110,6 +110,100 @@ >=20 > #endif /* __DOXYGEN__ */ >=20 > +#ifdef RTE_STDC_ATOMICS > + > +#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L || > defined(__STDC_NO_ATOMICS__) > +#error compiler does not support C11 standard atomics > +#else > +#include > +#endif > + > +#define __rte_atomic _Atomic > + > +typedef int rte_memory_order; I would prefer enum for rte_memory_order: typedef enum { rte_memory_order_relaxed =3D memory_order_relaxed, rte_memory_order_consume =3D memory_order_consume, rte_memory_order_acquire =3D memory_order_acquire, rte_memory_order_release =3D memory_order_release, rte_memory_order_acq_rel =3D memory_order_acq_rel, rte_memory_order_seq_cst =3D memory_order_seq_cst } rte_memory_order; > + > +#define rte_memory_order_relaxed memory_order_relaxed > +#define rte_memory_order_consume memory_order_consume > +#define rte_memory_order_acquire memory_order_acquire > +#define rte_memory_order_release memory_order_release > +#define rte_memory_order_acq_rel memory_order_acq_rel > +#define rte_memory_order_seq_cst memory_order_seq_cst > + > +#define rte_atomic_store_explicit(obj, desired, order) \ > + atomic_store_explicit(obj, desired, order) > + > +#define rte_atomic_load_explicit(obj, order) \ > + atomic_load_explicit(obj, order) > + > +#define rte_atomic_exchange_explicit(obj, desired, order) \ > + atomic_exchange_explicit(obj, desired, order) > + > +#define rte_atomic_compare_exchange_strong_explicit(obj, expected, > desired, success, fail) \ > + atomic_compare_exchange_strong_explicit(obj, expected, desired, > success, fail) > + > +#define rte_atomic_compare_exchange_weak_explicit(obj, expected, > desired, success, fail) \ > + atomic_compare_exchange_weak_explicit(obj, expected, desired, > success, fail) > + > +#define rte_atomic_fetch_add_explicit(obj, arg, order) \ > + atomic_fetch_add_explicit(obj, arg, order) > + > +#define rte_atomic_fetch_sub_explicit(obj, arg, order) \ > + atomic_fetch_sub_explicit(obj, arg, order) > + > +#define rte_atomic_fetch_or_explicit(obj, arg, order) \ > + atomic_fetch_or_explicit(obj, arg, order) > + > +#define rte_atomic_fetch_xor_explicit(obj, arg, order) \ > + atomic_fetch_xor_explicit(obj, arg, order) > + > +#define rte_atomic_fetch_and_explicit(obj, arg, order) \ > + atomic_fetch_and_explicit(obj, arg, order) > + > +#else > + > +#define __rte_atomic > + > +typedef int rte_memory_order; > + > +#define rte_memory_order_relaxed __ATOMIC_RELAXED > +#define rte_memory_order_consume __ATOMIC_CONSUME > +#define rte_memory_order_acquire __ATOMIC_ACQUIRE > +#define rte_memory_order_release __ATOMIC_RELEASE > +#define rte_memory_order_acq_rel __ATOMIC_ACQ_REL > +#define rte_memory_order_seq_cst __ATOMIC_SEQ_CST Prefer enum for rte_memory_order: typedef enum { rte_memory_order_relaxed =3D __ATOMIC_RELAXED, rte_memory_order_consume =3D __ATOMIC_CONSUME, rte_memory_order_acquire =3D __ATOMIC_ACQUIRE, rte_memory_order_release =3D __ATOMIC_RELEASE, rte_memory_order_acq_rel =3D __ATOMIC_ACQ_REL, rte_memory_order_seq_cst =3D __ATOMIC_SEQ_CST } rte_memory_order; > + > +#define rte_atomic_store_explicit(obj, desired, order) \ > + __atomic_store_n(obj, desired, order) > + > +#define rte_atomic_load_explicit(obj, order) \ > + __atomic_load_n(obj, order) > + > +#define rte_atomic_exchange_explicit(obj, desired, order) \ > + __atomic_exchange_n(obj, desired, order) > + > +#define rte_atomic_compare_exchange_strong_explicit(obj, expected, > desired, success, fail) \ > + __atomic_compare_exchange_n(obj, expected, desired, 0, success, > fail) The type of the "weak" parameter to __atomic_compare_exchange_n() is = bool, not int, so use "false" instead of 0. There is probably no = practical difference, so I'll leave it up to you. You might need to include for this... I haven't checked. > + > +#define rte_atomic_compare_exchange_weak_explicit(obj, expected, > desired, success, fail) \ > + __atomic_compare_exchange_n(obj, expected, desired, 1, success, > fail) Same as above: Use "true" instead of 1. > + > +#define rte_atomic_fetch_add_explicit(obj, arg, order) \ > + __atomic_fetch_add(obj, arg, order) > + > +#define rte_atomic_fetch_sub_explicit(obj, arg, order) \ > + __atomic_fetch_sub(obj, arg, order) > + > +#define rte_atomic_fetch_or_explicit(obj, arg, order) \ > + __atomic_fetch_or(obj, arg, order) > + > +#define rte_atomic_fetch_xor_explicit(obj, arg, order) \ > + __atomic_fetch_xor(obj, arg, order) > + > +#define rte_atomic_fetch_and_explicit(obj, arg, order) \ > + __atomic_fetch_and(obj, arg, order) > + > +#endif > + > /** > * Compiler barrier. > * > @@ -123,7 +217,7 @@ > /** > * Synchronization fence between threads based on the specified = memory > order. > */ > -static inline void rte_atomic_thread_fence(int memorder); > +static inline void rte_atomic_thread_fence(rte_memory_order = memorder); >=20 > /*------------------------- 16 bit atomic operations = ----------------- > --------*/ >=20 > diff --git a/lib/eal/loongarch/include/rte_atomic.h > b/lib/eal/loongarch/include/rte_atomic.h > index 3c82845..66aa0c8 100644 > --- a/lib/eal/loongarch/include/rte_atomic.h > +++ b/lib/eal/loongarch/include/rte_atomic.h > @@ -35,9 +35,13 @@ > #define rte_io_rmb() rte_mb() >=20 > static __rte_always_inline void > -rte_atomic_thread_fence(int memorder) > +rte_atomic_thread_fence(rte_memory_order memorder) > { > +#ifdef RTE_STDC_ATOMICS > + atomic_thread_fence(memorder); > +#else > __atomic_thread_fence(memorder); > +#endif > } >=20 > #ifdef __cplusplus > diff --git a/lib/eal/ppc/include/rte_atomic.h > b/lib/eal/ppc/include/rte_atomic.h > index 663b4d3..a428a83 100644 > --- a/lib/eal/ppc/include/rte_atomic.h > +++ b/lib/eal/ppc/include/rte_atomic.h > @@ -38,9 +38,13 @@ > #define rte_io_rmb() rte_rmb() >=20 > static __rte_always_inline void > -rte_atomic_thread_fence(int memorder) > +rte_atomic_thread_fence(rte_memory_order memorder) > { > +#ifdef RTE_STDC_ATOMICS > + atomic_thread_fence(memorder); > +#else > __atomic_thread_fence(memorder); > +#endif > } >=20 > /*------------------------- 16 bit atomic operations = ----------------- > --------*/ > diff --git a/lib/eal/riscv/include/rte_atomic.h > b/lib/eal/riscv/include/rte_atomic.h > index 4b4633c..3c203a9 100644 > --- a/lib/eal/riscv/include/rte_atomic.h > +++ b/lib/eal/riscv/include/rte_atomic.h > @@ -40,9 +40,13 @@ > #define rte_io_rmb() asm volatile("fence ir, ir" : : : "memory") >=20 > static __rte_always_inline void > -rte_atomic_thread_fence(int memorder) > +rte_atomic_thread_fence(rte_memory_order memorder) > { > +#ifdef RTE_STDC_ATOMICS > + atomic_thread_fence(memorder); > +#else > __atomic_thread_fence(memorder); > +#endif > } >=20 > #ifdef __cplusplus > diff --git a/lib/eal/x86/include/rte_atomic.h > b/lib/eal/x86/include/rte_atomic.h > index f2ee1a9..02d8b12 100644 > --- a/lib/eal/x86/include/rte_atomic.h > +++ b/lib/eal/x86/include/rte_atomic.h > @@ -87,12 +87,16 @@ > * used instead. > */ > static __rte_always_inline void > -rte_atomic_thread_fence(int memorder) > +rte_atomic_thread_fence(rte_memory_order memorder) > { > - if (memorder =3D=3D __ATOMIC_SEQ_CST) > + if (memorder =3D=3D rte_memory_order_seq_cst) > rte_smp_mb(); > else > +#ifdef RTE_STDC_ATOMICS > + atomic_thread_fence(memorder); > +#else > __atomic_thread_fence(memorder); > +#endif > } >=20 > /*------------------------- 16 bit atomic operations = ----------------- > --------*/ > diff --git a/meson_options.txt b/meson_options.txt > index 0852849..acbcbb8 100644 > --- a/meson_options.txt > +++ b/meson_options.txt > @@ -46,6 +46,8 @@ option('mbuf_refcnt_atomic', type: 'boolean', value: > true, description: > 'Atomically access the mbuf refcnt.') > option('platform', type: 'string', value: 'native', description: > 'Platform to build, either "native", "generic" or a SoC. = Please > refer to the Linux build guide for more information.') > +option('enable_stdatomics', type: 'boolean', value: false, > description: > + 'enable use of standard C11 atomics.') > option('enable_trace_fp', type: 'boolean', value: false, description: > 'enable fast path trace points.') > option('tests', type: 'boolean', value: true, description: > -- > 1.8.3.1 >=20