diff mbox series

[6/6] devtools: forbid new direct use of GCC atomic builtins

Message ID 1691717521-1025-7-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series RFC optional rte optional stdatomics API |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS

Commit Message

Tyler Retzlaff Aug. 11, 2023, 1:32 a.m. UTC 
  Refrain from using compiler __atomic_xxx builtins DPDK now requires
the use of rte_atomic_<op>_explicit macros when operating on DPDK
atomic variables.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 devtools/checkpatches.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Bruce Richardson Aug. 11, 2023, 8:57 a.m. UTC | #1 
On Thu, Aug 10, 2023 at 06:32:01PM -0700, Tyler Retzlaff wrote:
> Refrain from using compiler __atomic_xxx builtins DPDK now requires
> the use of rte_atomic_<op>_explicit macros when operating on DPDK
> atomic variables.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Morten Brørup Aug. 11, 2023, 9:51 a.m. UTC | #2 
> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Friday, 11 August 2023 03.32
> 
> Refrain from using compiler __atomic_xxx builtins DPDK now requires
> the use of rte_atomic_<op>_explicit macros when operating on DPDK
> atomic variables.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>

The Acked-by should have been:
Suggested-by: Morten Brørup <mb@smartsharesystems.com>

> ---
>  devtools/checkpatches.sh | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index 43f5e36..a32f02e 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -102,6 +102,14 @@ check_forbidden_additions() { # <patch>
>  		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk
> \
>  		"$1" || res=1
> 
> +	# refrain from using compiler __atomic_xxx builtins
> +	awk -v FOLDERS="lib drivers app examples" \
> +		-v EXPRESSIONS="__atomic_.*\\\(" \

This expression is a superset of other expressions in checkpatches (search for "__atomic" in the checkpatches, and you'll find them). Perhaps they can be removed?

> +		-v RET_ON_FAIL=1 \
> +		-v MESSAGE='Using __atomic_xxx builtins' \
> +		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk
> \
> +		"$1" || res=1
> +
>  	# refrain from using compiler __atomic_thread_fence()
>  	# It should be avoided on x86 for SMP case.
>  	awk -v FOLDERS="lib drivers app examples" \
> --
> 1.8.3.1
Tyler Retzlaff Aug. 11, 2023, 3:56 p.m. UTC | #3 
On Fri, Aug 11, 2023 at 11:51:17AM +0200, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Friday, 11 August 2023 03.32
> > 
> > Refrain from using compiler __atomic_xxx builtins DPDK now requires
> > the use of rte_atomic_<op>_explicit macros when operating on DPDK
> > atomic variables.
> > 
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 
> The Acked-by should have been:
> Suggested-by: Morten Brørup <mb@smartsharesystems.com>

ooh, did i make a mistake? i was carrying the ack from my abandoned
series (or i thought you had acked this patch on that series sorry).

i'll change it to suggested-by.

thanks!

> 
> > ---
> >  devtools/checkpatches.sh | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> > index 43f5e36..a32f02e 100755
> > --- a/devtools/checkpatches.sh
> > +++ b/devtools/checkpatches.sh
> > @@ -102,6 +102,14 @@ check_forbidden_additions() { # <patch>
> >  		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk
> > \
> >  		"$1" || res=1
> > 
> > +	# refrain from using compiler __atomic_xxx builtins
> > +	awk -v FOLDERS="lib drivers app examples" \
> > +		-v EXPRESSIONS="__atomic_.*\\\(" \
> 
> This expression is a superset of other expressions in checkpatches (search for "__atomic" in the checkpatches, and you'll find them). Perhaps they can be removed?

yes, seems like a good idea.

v2

> 
> > +		-v RET_ON_FAIL=1 \
> > +		-v MESSAGE='Using __atomic_xxx builtins' \
> > +		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk
> > \
> > +		"$1" || res=1
> > +
> >  	# refrain from using compiler __atomic_thread_fence()
> >  	# It should be avoided on x86 for SMP case.
> >  	awk -v FOLDERS="lib drivers app examples" \
> > --
> > 1.8.3.1
>
Morten Brørup Aug. 14, 2023, 6:37 a.m. UTC | #4 
> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Friday, 11 August 2023 17.57
> 
> On Fri, Aug 11, 2023 at 11:51:17AM +0200, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Friday, 11 August 2023 03.32
> > >
> > > Refrain from using compiler __atomic_xxx builtins DPDK now requires
> > > the use of rte_atomic_<op>_explicit macros when operating on DPDK
> > > atomic variables.
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >
> > The Acked-by should have been:
> > Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> 
> ooh, did i make a mistake? i was carrying the ack from my abandoned
> series (or i thought you had acked this patch on that series sorry).
> 
> i'll change it to suggested-by.
> 
> thanks!

No problem. Both tags mean that I approve of the concept anyway.

Minor mistakes are bound to come with big sets like this. Better in the comments than in the code. :-)

> 
> >
> > > ---
> > >  devtools/checkpatches.sh | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> > > index 43f5e36..a32f02e 100755
> > > --- a/devtools/checkpatches.sh
> > > +++ b/devtools/checkpatches.sh
> > > @@ -102,6 +102,14 @@ check_forbidden_additions() { # <patch>
> > >  		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk
> > > \
> > >  		"$1" || res=1
> > >
> > > +	# refrain from using compiler __atomic_xxx builtins
> > > +	awk -v FOLDERS="lib drivers app examples" \
> > > +		-v EXPRESSIONS="__atomic_.*\\\(" \
> >
> > This expression is a superset of other expressions in checkpatches (search
> for "__atomic" in the checkpatches, and you'll find them). Perhaps they can be
> removed?
> 
> yes, seems like a good idea.
> 
> v2
> 
> >
> > > +		-v RET_ON_FAIL=1 \
> > > +		-v MESSAGE='Using __atomic_xxx builtins' \
> > > +		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk
> > > \
> > > +		"$1" || res=1
> > > +
> > >  	# refrain from using compiler __atomic_thread_fence()
> > >  	# It should be avoided on x86 for SMP case.
> > >  	awk -v FOLDERS="lib drivers app examples" \
> > > --
> > > 1.8.3.1
> >
diff mbox series

Patch

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 43f5e36..a32f02e 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -102,6 +102,14 @@  check_forbidden_additions() { # <patch>
 		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
 		"$1" || res=1
 
+	# refrain from using compiler __atomic_xxx builtins
+	awk -v FOLDERS="lib drivers app examples" \
+		-v EXPRESSIONS="__atomic_.*\\\(" \
+		-v RET_ON_FAIL=1 \
+		-v MESSAGE='Using __atomic_xxx builtins' \
+		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
+		"$1" || res=1
+
 	# refrain from using compiler __atomic_thread_fence()
 	# It should be avoided on x86 for SMP case.
 	awk -v FOLDERS="lib drivers app examples" \