The cited commit introduced functions with 'int memory_order' argument. The C11 standard section 7.17.1.4 defines 'memory_order' as the "enumerated type whose enumerators identify memory ordering constraints". Applications that use the standard enum (includes stdatomic.h), will fail compilation with: error: declaration of 'memory_order' shadows a global declaration [-Werror=shadow] rte_atomic_thread_fence(int memory_order) Fix it by changing the argument name 'memory_order' to 'memorder'. Fixes: 672a15056380 ("eal: add wrapper for C11 atomic thread fence") Signed-off-by: Eli Britstein <elibr@nvidia.com> --- lib/librte_eal/arm/include/rte_atomic_32.h | 4 ++-- lib/librte_eal/arm/include/rte_atomic_64.h | 4 ++-- lib/librte_eal/include/generic/rte_atomic.h | 2 +- lib/librte_eal/ppc/include/rte_atomic.h | 4 ++-- lib/librte_eal/x86/include/rte_atomic.h | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/librte_eal/arm/include/rte_atomic_32.h b/lib/librte_eal/arm/include/rte_atomic_32.h index 9d0568d497..fe48ab428e 100644 --- a/lib/librte_eal/arm/include/rte_atomic_32.h +++ b/lib/librte_eal/arm/include/rte_atomic_32.h @@ -34,9 +34,9 @@ extern "C" { #define rte_io_rmb() rte_rmb() static __rte_always_inline void -rte_atomic_thread_fence(int memory_order) +rte_atomic_thread_fence(int memorder) { - __atomic_thread_fence(memory_order); + __atomic_thread_fence(memorder); } #ifdef __cplusplus diff --git a/lib/librte_eal/arm/include/rte_atomic_64.h b/lib/librte_eal/arm/include/rte_atomic_64.h index c518559bc9..20dd6c75dd 100644 --- a/lib/librte_eal/arm/include/rte_atomic_64.h +++ b/lib/librte_eal/arm/include/rte_atomic_64.h @@ -38,9 +38,9 @@ extern "C" { #define rte_io_rmb() rte_rmb() static __rte_always_inline void -rte_atomic_thread_fence(int memory_order) +rte_atomic_thread_fence(int memorder) { - __atomic_thread_fence(memory_order); + __atomic_thread_fence(memorder); } /*------------------------ 128 bit atomic operations -------------------------*/ diff --git a/lib/librte_eal/include/generic/rte_atomic.h b/lib/librte_eal/include/generic/rte_atomic.h index d1255b2d8c..276272f40b 100644 --- a/lib/librte_eal/include/generic/rte_atomic.h +++ b/lib/librte_eal/include/generic/rte_atomic.h @@ -122,7 +122,7 @@ static inline void rte_io_rmb(void); /** * Synchronization fence between threads based on the specified memory order. */ -static inline void rte_atomic_thread_fence(int memory_order); +static inline void rte_atomic_thread_fence(int memorder); /*------------------------- 16 bit atomic operations -------------------------*/ diff --git a/lib/librte_eal/ppc/include/rte_atomic.h b/lib/librte_eal/ppc/include/rte_atomic.h index a91989930b..6a7e65210c 100644 --- a/lib/librte_eal/ppc/include/rte_atomic.h +++ b/lib/librte_eal/ppc/include/rte_atomic.h @@ -37,9 +37,9 @@ extern "C" { #define rte_io_rmb() rte_rmb() static __rte_always_inline void -rte_atomic_thread_fence(int memory_order) +rte_atomic_thread_fence(int memorder) { - __atomic_thread_fence(memory_order); + __atomic_thread_fence(memorder); } /*------------------------- 16 bit atomic operations -------------------------*/ diff --git a/lib/librte_eal/x86/include/rte_atomic.h b/lib/librte_eal/x86/include/rte_atomic.h index b7d6b06ddf..915afd9d27 100644 --- a/lib/librte_eal/x86/include/rte_atomic.h +++ b/lib/librte_eal/x86/include/rte_atomic.h @@ -87,12 +87,12 @@ rte_smp_mb(void) * used instead. */ static __rte_always_inline void -rte_atomic_thread_fence(int memory_order) +rte_atomic_thread_fence(int memorder) { - if (memory_order == __ATOMIC_SEQ_CST) + if (memorder == __ATOMIC_SEQ_CST) rte_smp_mb(); else - __atomic_thread_fence(memory_order); + __atomic_thread_fence(memorder); } /*------------------------- 16 bit atomic operations -------------------------*/ -- 2.28.0.546.g385c171
>-----Original Message----- >From: dev <dev-bounces@dpdk.org> On Behalf Of Eli Britstein >Sent: Wednesday, October 14, 2020 9:19 AM >To: dev@dpdk.org >Cc: Eli Britstein <elibr@nvidia.com> >Subject: [dpdk-dev] [PATCH] eal: fix build with conflicting libc variable >memory_order > >The cited commit introduced functions with 'int memory_order' argument. >The C11 standard section 7.17.1.4 defines 'memory_order' as the >"enumerated type whose enumerators identify memory ordering >constraints". >Applications that use the standard enum (includes stdatomic.h), will fail >compilation with: >error: declaration of 'memory_order' shadows a global declaration > [-Werror=shadow] > rte_atomic_thread_fence(int memory_order) Fix it by changing the >argument name 'memory_order' to 'memorder'. > >Fixes: 672a15056380 ("eal: add wrapper for C11 atomic thread fence") >Signed-off-by: Eli Britstein <elibr@nvidia.com> Reviewed-by: Asaf Penso <asafp@nvidia.com> >--- > lib/librte_eal/arm/include/rte_atomic_32.h | 4 ++-- >lib/librte_eal/arm/include/rte_atomic_64.h | 4 ++-- >lib/librte_eal/include/generic/rte_atomic.h | 2 +- > lib/librte_eal/ppc/include/rte_atomic.h | 4 ++-- > lib/librte_eal/x86/include/rte_atomic.h | 6 +++--- > 5 files changed, 10 insertions(+), 10 deletions(-) > >diff --git a/lib/librte_eal/arm/include/rte_atomic_32.h >b/lib/librte_eal/arm/include/rte_atomic_32.h >index 9d0568d497..fe48ab428e 100644 >--- a/lib/librte_eal/arm/include/rte_atomic_32.h >+++ b/lib/librte_eal/arm/include/rte_atomic_32.h >@@ -34,9 +34,9 @@ extern "C" { > #define rte_io_rmb() rte_rmb() > > static __rte_always_inline void >-rte_atomic_thread_fence(int memory_order) >+rte_atomic_thread_fence(int memorder) > { >- __atomic_thread_fence(memory_order); >+ __atomic_thread_fence(memorder); > } > > #ifdef __cplusplus >diff --git a/lib/librte_eal/arm/include/rte_atomic_64.h >b/lib/librte_eal/arm/include/rte_atomic_64.h >index c518559bc9..20dd6c75dd 100644 >--- a/lib/librte_eal/arm/include/rte_atomic_64.h >+++ b/lib/librte_eal/arm/include/rte_atomic_64.h >@@ -38,9 +38,9 @@ extern "C" { > #define rte_io_rmb() rte_rmb() > > static __rte_always_inline void >-rte_atomic_thread_fence(int memory_order) >+rte_atomic_thread_fence(int memorder) > { >- __atomic_thread_fence(memory_order); >+ __atomic_thread_fence(memorder); > } > > /*------------------------ 128 bit atomic operations -------------------------*/ diff -- >git a/lib/librte_eal/include/generic/rte_atomic.h >b/lib/librte_eal/include/generic/rte_atomic.h >index d1255b2d8c..276272f40b 100644 >--- a/lib/librte_eal/include/generic/rte_atomic.h >+++ b/lib/librte_eal/include/generic/rte_atomic.h >@@ -122,7 +122,7 @@ static inline void rte_io_rmb(void); > /** > * Synchronization fence between threads based on the specified memory >order. > */ >-static inline void rte_atomic_thread_fence(int memory_order); >+static inline void rte_atomic_thread_fence(int memorder); > > /*------------------------- 16 bit atomic operations -------------------------*/ > >diff --git a/lib/librte_eal/ppc/include/rte_atomic.h >b/lib/librte_eal/ppc/include/rte_atomic.h >index a91989930b..6a7e65210c 100644 >--- a/lib/librte_eal/ppc/include/rte_atomic.h >+++ b/lib/librte_eal/ppc/include/rte_atomic.h >@@ -37,9 +37,9 @@ extern "C" { > #define rte_io_rmb() rte_rmb() > > static __rte_always_inline void >-rte_atomic_thread_fence(int memory_order) >+rte_atomic_thread_fence(int memorder) > { >- __atomic_thread_fence(memory_order); >+ __atomic_thread_fence(memorder); > } > > /*------------------------- 16 bit atomic operations -------------------------*/ diff -- >git a/lib/librte_eal/x86/include/rte_atomic.h >b/lib/librte_eal/x86/include/rte_atomic.h >index b7d6b06ddf..915afd9d27 100644 >--- a/lib/librte_eal/x86/include/rte_atomic.h >+++ b/lib/librte_eal/x86/include/rte_atomic.h >@@ -87,12 +87,12 @@ rte_smp_mb(void) > * used instead. > */ > static __rte_always_inline void >-rte_atomic_thread_fence(int memory_order) >+rte_atomic_thread_fence(int memorder) > { >- if (memory_order == __ATOMIC_SEQ_CST) >+ if (memorder == __ATOMIC_SEQ_CST) > rte_smp_mb(); > else >- __atomic_thread_fence(memory_order); >+ __atomic_thread_fence(memorder); > } > > /*------------------------- 16 bit atomic operations -------------------------*/ >-- >2.28.0.546.g385c171
14/10/2020 08:19, Eli Britstein: > The cited commit introduced functions with 'int memory_order' argument. > The C11 standard section 7.17.1.4 defines 'memory_order' as the > "enumerated type whose enumerators identify memory ordering constraints". > Applications that use the standard enum (includes stdatomic.h), will > fail compilation with: > error: declaration of 'memory_order' shadows a global declaration > [-Werror=shadow] > rte_atomic_thread_fence(int memory_order) > Fix it by changing the argument name 'memory_order' to 'memorder'. Not clear why it builds fine with most compilers, but the fix does not hurt. > Fixes: 672a15056380 ("eal: add wrapper for C11 atomic thread fence") A blank line should be inserted here. > Signed-off-by: Eli Britstein <elibr@nvidia.com> Acked-by: Thomas Monjalon <thomas@monjalon.net>
On Thu, Oct 15, 2020 at 1:16 AM Thomas Monjalon <thomas@monjalon.net> wrote: > > 14/10/2020 08:19, Eli Britstein: > > The cited commit introduced functions with 'int memory_order' argument. > > The C11 standard section 7.17.1.4 defines 'memory_order' as the > > "enumerated type whose enumerators identify memory ordering constraints". > > Applications that use the standard enum (includes stdatomic.h), will > > fail compilation with: Simply including stdatomic.h does not trigger the problem. Can you rework this commitlog with below info? > > error: declaration of 'memory_order' shadows a global declaration > > [-Werror=shadow] > > rte_atomic_thread_fence(int memory_order) > > Fix it by changing the argument name 'memory_order' to 'memorder'. > > Not clear why it builds fine with most compilers, I can reproduce in two cases: - with a gcc that provides a stdatomic.h header + passing -Wsystem-headers in the CFLAGS, - with a compiler that does not provide stdatomic.h and you redefine memory_order in your code (like OVS does), > but the fix does not hurt. Otherwise, yes, lgtm. > > Fixes: 672a15056380 ("eal: add wrapper for C11 atomic thread fence") > > A blank line should be inserted here. > > > Signed-off-by: Eli Britstein <elibr@nvidia.com> > > Acked-by: Thomas Monjalon <thomas@monjalon.net> Acked-by: David Marchand <david.marchand@redhat.com> -- David Marchand