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 754A4428B9; Tue, 4 Apr 2023 04:24:15 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C6FF4067E; Tue, 4 Apr 2023 04:24:15 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 2FD4040141 for ; Tue, 4 Apr 2023 04:24:13 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 50EC7210DC6B; Mon, 3 Apr 2023 19:24:12 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 50EC7210DC6B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1680575052; bh=ahBF8s1PgDQ8egxy2d8vepRombFhev0P/X4ZNdy97qI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=NzC1g6iQTeCmul4x8FBUKWsprr8H8A41LoRY1mWtP/MFki/GciKZubc53Uh8gSVZZ S9Z42i/eNOlf6bQTqN/EbVTP9uNhnURXnG9BhiP+SP6VP4TAYAzq9SOYRzFN0P5p6e XqE8ZlncAqVgfVZlqvRwbegXEuxIoT3wiovqWPo8= Date: Mon, 3 Apr 2023 19:24:12 -0700 From: Tyler Retzlaff To: Mattias =?iso-8859-1?Q?R=F6nnblom?= Cc: dev@dpdk.org, 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 Message-ID: <20230404022412.GA21454@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> 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> <5c2ab2bd-6a73-6c58-e789-83326d7ee129@lysator.liu.se> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <5c2ab2bd-6a73-6c58-e789-83326d7ee129@lysator.liu.se> User-Agent: Mutt/1.5.21 (2010-09-15) 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 On Mon, Apr 03, 2023 at 11:11:35PM +0200, Mattias Rönnblom wrote: > 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 > >--- > > 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 > >+#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? This series has been marked obsolete/abandoned. As per technical board DPDK will move straight to standard C atomics without transitioning with these macros. Thanks