From: "Mattias Rönnblom" <hofors@lysator.liu.se>
To: Tyler Retzlaff <roretzla@linux.microsoft.com>, dev@dpdk.org
Cc: david.marchand@redhat.com, thomas@monjalon.net,
mb@smartsharesystems.com, Honnappa.Nagarahalli@arm.com,
bruce.richardson@intel.com
Subject: Re: [PATCH v2] eal: introduce atomics abstraction
Date: Mon, 3 Apr 2023 23:11:35 +0200 [thread overview]
Message-ID: <5c2ab2bd-6a73-6c58-e789-83326d7ee129@lysator.liu.se> (raw)
In-Reply-To: <1675892618-31755-2-git-send-email-roretzla@linux.microsoft.com>
On 2023-02-08 22:43, Tyler Retzlaff wrote:
> Introduce atomics abstraction that permits optional use of standard C11
> atomics when meson is provided the new enable_stdatomics=true option.
>
Terminology nitpicking: I don't think these functions provide any
abstraction at all. They are just wrappers.
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> 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(-)
>
> 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')
>
> +stdc_atomics_enabled = get_option('enable_stdatomics')
> +dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
> +
> +if stdc_atomics_enabled
> +if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
> + add_project_arguments('-std=gnu11', language: 'c')
> +else
> + add_project_arguments('-std=c11', 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()
>
> 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
> }
>
> #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()
>
> 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
> }
>
> /*------------------------ 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 @@
>
> #endif /* __DOXYGEN__ */
>
> +#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 <stdatomic.h>
> +#endif
> +
> +#define __rte_atomic _Atomic
> +
> +typedef int 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
> +
Would this be better of as an enum, rather than a typedef? If typedef,
it should have the "_t" postfix. Also, the #define should be all-caps.
> +#define rte_atomic_store_explicit(obj, desired, order) \
> + atomic_store_explicit(obj, desired, order)
> +
Drop "explicit" from all the names. It's just noise. Also, the memory
orders have very long names.
We haven't even move all DPDK code over from the old API, to using GCC
C11 built-ins, and now we are switching to a new API?
> +#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
> +
> +#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)
> +
> +#define rte_atomic_compare_exchange_weak_explicit(obj, expected, desired, success, fail) \
> + __atomic_compare_exchange_n(obj, expected, desired, 1, success, fail)
> +
> +#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);
>
> /*------------------------- 16 bit atomic operations -------------------------*/
>
> 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()
>
> 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
> }
>
> #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()
>
> 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
> }
>
> /*------------------------- 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")
>
> 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
> }
>
> #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 == __ATOMIC_SEQ_CST)
> + if (memorder == rte_memory_order_seq_cst)
> rte_smp_mb();
> else
> +#ifdef RTE_STDC_ATOMICS
> + atomic_thread_fence(memorder);
> +#else
> __atomic_thread_fence(memorder);
> +#endif
> }
>
> /*------------------------- 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:
next prev parent reply other threads:[~2023-04-03 21:11 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-12 21:26 [PATCH] eal: abstract compiler atomics Tyler Retzlaff
2023-01-12 21:26 ` [PATCH] eal: introduce atomics abstraction Tyler Retzlaff
2023-01-31 22:42 ` Thomas Monjalon
2023-02-01 1:07 ` Honnappa Nagarahalli
2023-02-01 8:09 ` Morten Brørup
2023-02-01 21:41 ` Tyler Retzlaff
2023-02-02 8:43 ` Morten Brørup
2023-02-02 19:00 ` Tyler Retzlaff
2023-02-02 20:44 ` Morten Brørup
2023-02-03 13:56 ` Bruce Richardson
2023-02-03 14:25 ` Morten Brørup
2023-02-03 12:19 ` Bruce Richardson
2023-02-03 20:49 ` Tyler Retzlaff
2023-02-07 15:16 ` Morten Brørup
2023-02-07 21:58 ` Tyler Retzlaff
2023-02-07 23:34 ` Honnappa Nagarahalli
2023-02-08 1:20 ` Tyler Retzlaff
2023-02-08 8:31 ` Morten Brørup
2023-02-08 16:35 ` Tyler Retzlaff
2023-02-09 0:16 ` Honnappa Nagarahalli
2023-02-09 8:34 ` Morten Brørup
2023-02-09 17:30 ` Tyler Retzlaff
2023-02-10 5:30 ` Honnappa Nagarahalli
2023-02-10 20:30 ` Tyler Retzlaff
2023-02-13 5:04 ` Honnappa Nagarahalli
2023-02-13 15:28 ` Ben Magistro
2023-02-13 15:55 ` Bruce Richardson
2023-02-13 16:46 ` Ben Magistro
2023-02-13 17:49 ` Morten Brørup
2023-02-13 23:18 ` Tyler Retzlaff
2023-01-31 21:33 ` [PATCH] eal: abstract compiler atomics Tyler Retzlaff
2023-02-08 21:43 ` [PATCH v2] " Tyler Retzlaff
2023-02-08 21:43 ` [PATCH v2] eal: introduce atomics abstraction Tyler Retzlaff
2023-02-09 8:05 ` Morten Brørup
2023-02-09 18:15 ` Tyler Retzlaff
2023-02-09 19:19 ` Morten Brørup
2023-02-09 22:04 ` Tyler Retzlaff
2023-04-03 21:17 ` Mattias Rönnblom
2023-02-09 9:04 ` Bruce Richardson
2023-02-09 12:53 ` Ferruh Yigit
2023-02-09 17:40 ` Tyler Retzlaff
2023-02-09 22:13 ` Ferruh Yigit
2023-02-10 0:36 ` Tyler Retzlaff
2023-02-09 17:38 ` Tyler Retzlaff
2023-04-03 21:32 ` Mattias Rönnblom
2023-04-03 21:11 ` Mattias Rönnblom [this message]
2023-04-03 21:25 ` Honnappa Nagarahalli
2023-04-04 2:24 ` Tyler Retzlaff
2023-02-22 18:09 ` [PATCH v2] eal: abstract compiler atomics Tyler Retzlaff
2023-02-22 20:07 ` Honnappa Nagarahalli
2023-02-23 19:11 ` Tyler Retzlaff
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5c2ab2bd-6a73-6c58-e789-83326d7ee129@lysator.liu.se \
--to=hofors@lysator.liu.se \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=mb@smartsharesystems.com \
--cc=roretzla@linux.microsoft.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).