diff mbox series

tools: Rename __fallthrough to fallthrough

Message ID 20221125154947.2163498-1-Liam.Howlett@oracle.com
State New
Headers
Series tools: Rename __fallthrough to fallthrough |

Commit Message

Liam R. Howlett Nov. 25, 2022, 3:50 p.m. UTC 
  Rename the fallthrough attribute to better align with the kernel
version.  Copy the definition from include/linux/compiler_attributes.h
including the #else clause.  Adding the #else clause allows the tools
compiler.h header to drop the check for a definition entirely and keeps
both definitions together.

Change any __fallthrough statements to fallthrough anywhere it was used
within perf.

This allows other tools to use the same key word as the kernel.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Tom Rix <trix@redhat.com>
Signed-off-by: Liam Howlett <Liam.Howlett@oracle.com>
---
 tools/include/linux/compiler-gcc.h                      | 6 ++++--
 tools/include/linux/compiler.h                          | 4 ----
 tools/perf/builtin-probe.c                              | 2 +-
 tools/perf/builtin-stat.c                               | 2 +-
 tools/perf/builtin-top.c                                | 2 +-
 tools/perf/util/callchain.c                             | 4 ++--
 tools/perf/util/intel-pt-decoder/intel-pt-decoder.c     | 6 +++---
 tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c | 2 +-
 tools/perf/util/ordered-events.c                        | 2 +-
 tools/perf/util/strfilter.c                             | 2 +-
 tools/perf/util/string.c                                | 2 +-
 11 files changed, 16 insertions(+), 18 deletions(-)

Comments

Miguel Ojeda Nov. 25, 2022, 6:50 p.m. UTC | #1 
On Fri, Nov 25, 2022 at 4:50 PM Liam Howlett <liam.howlett@oracle.com> wrote:
>
> Rename the fallthrough attribute to better align with the kernel
> version.  Copy the definition from include/linux/compiler_attributes.h
> including the #else clause.  Adding the #else clause allows the tools
> compiler.h header to drop the check for a definition entirely and keeps
> both definitions together.

Thanks for doing this! Consistency with `tools/` is nice to have indeed.

I see that `compiler-gcc.h` is included for Clang too in `tools/`, so
it looks fine to me:

    Reviewed-by: Miguel Ojeda <ojeda@kernel.org>

In the future, it could be nice to enable the warning for Clang too,
though I see it requires a few cleanups.

Another idea is to use something similar to `compiler_attributes.h` in
`tools/` (perhaps just copying it and removing what is not needed?).

Cheers,
Miguel
Liam R. Howlett April 6, 2023, 7:58 p.m. UTC | #2 
* Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> [221125 13:51]:
> On Fri, Nov 25, 2022 at 4:50 PM Liam Howlett <liam.howlett@oracle.com> wrote:
> >
> > Rename the fallthrough attribute to better align with the kernel
> > version.  Copy the definition from include/linux/compiler_attributes.h
> > including the #else clause.  Adding the #else clause allows the tools
> > compiler.h header to drop the check for a definition entirely and keeps
> > both definitions together.
> 
> Thanks for doing this! Consistency with `tools/` is nice to have indeed.
> 
> I see that `compiler-gcc.h` is included for Clang too in `tools/`, so
> it looks fine to me:
> 
>     Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
> 

Any chance of this being accepted?  I'm looking to use this in the radix
test suite and would rather not duplicate code.

> In the future, it could be nice to enable the warning for Clang too,
> though I see it requires a few cleanups.
> 
> Another idea is to use something similar to `compiler_attributes.h` in
> `tools/` (perhaps just copying it and removing what is not needed?).
> 
> Cheers,
> Miguel
Miguel Ojeda April 6, 2023, 8:02 p.m. UTC | #3 
On Thu, Apr 6, 2023 at 9:58 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> Any chance of this being accepted?  I'm looking to use this in the radix
> test suite and would rather not duplicate code.

That would be up to the `tools/perf/` maintainers.

Cheers,
Miguel
Arnaldo Carvalho de Melo April 6, 2023, 9:11 p.m. UTC | #4 
Em Thu, Apr 06, 2023 at 10:02:57PM +0200, Miguel Ojeda escreveu:
> On Thu, Apr 6, 2023 at 9:58 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> >
> > Any chance of this being accepted?  I'm looking to use this in the radix
> > test suite and would rather not duplicate code.
> 
> That would be up to the `tools/perf/` maintainers.

Thanks, applied,

- Arnaldo
Arnaldo Carvalho de Melo April 6, 2023, 9:31 p.m. UTC | #5 
Em Thu, Apr 06, 2023 at 06:11:03PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Apr 06, 2023 at 10:02:57PM +0200, Miguel Ojeda escreveu:
> > On Thu, Apr 6, 2023 at 9:58 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > >
> > > Any chance of this being accepted?  I'm looking to use this in the radix
> > > test suite and would rather not duplicate code.
> > 
> > That would be up to the `tools/perf/` maintainers.
> 
> Thanks, applied,

So, I noticed some missing conversions, fixed that, but then when trying
to test build it with:

	make -C tools/perf build-test

I stumbled on this:

             make_with_gtk2: cd . && make GTK2=1 -j32  DESTDIR=/tmp/tmp.5AP8tPJgiT
cd . && make GTK2=1 -j32 DESTDIR=/tmp/tmp.5AP8tPJgiT
  BUILD:   Doing 'make -j32' parallel build
  HOSTCC  fixdep.o
  HOSTLD  fixdep-in.o
<SNIP>
  CC      trace/beauty/ioctl.o
In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9,
                 from /usr/include/glib-2.0/glib/gtypes.h:34,
                 from /usr/include/glib-2.0/glib/galloca.h:34,
                 from /usr/include/glib-2.0/glib.h:32,
                 from /usr/include/glib-2.0/gobject/gbinding.h:30,
                 from /usr/include/glib-2.0/glib-object.h:24,
                 from /usr/include/glib-2.0/gio/gioenums.h:30,
                 from /usr/include/glib-2.0/gio/giotypes.h:30,
                 from /usr/include/glib-2.0/gio/gio.h:28,
                 from /usr/include/gtk-2.0/gdk/gdkapplaunchcontext.h:30,
                 from /usr/include/gtk-2.0/gdk/gdk.h:32,
                 from /usr/include/gtk-2.0/gtk/gtk.h:32,
                 from ui/gtk/gtk.h:8,
                 from ui/gtk/helpline.c:6:
/usr/include/glib-2.0/glib/gmacros.h:637:28: error: missing ')' after "__has_attribute"
  637 | #if g_macro__has_attribute(fallthrough)
      |                            ^~~~~~~~~~~
In file included from /var/home/acme/git/perf-tools-next/tools/include/linux/compiler_types.h:36,
                 from /var/home/acme/git/perf-tools-next/tools/include/linux/compiler.h:5,
                 from /var/home/acme/git/perf-tools-next/tools/include/linux/build_bug.h:5,
                 from /var/home/acme/git/perf-tools-next/tools/include/linux/kernel.h:8,
                 from ui/gtk/helpline.c:4:
/var/home/acme/git/perf-tools-next/tools/include/linux/compiler-gcc.h:16:55: error: missing binary operator before token "("
   16 | # define fallthrough                    __attribute__((__fallthrough__))
      |                                                       ^
/usr/include/glib-2.0/glib/gmacros.h:637:28: note: in expansion of macro ‘fallthrough’
  637 | #if g_macro__has_attribute(fallthrough)
      |                            ^~~~~~~~~~~
  CC      trace/beauty/kcmp.o
  CC      bench/sched-messaging.o
  CC      trace/beauty/mount_flags.o


I'm trying to fix this now...

- Arnaldo
Arnaldo Carvalho de Melo April 6, 2023, 9:32 p.m. UTC | #6 
Em Thu, Apr 06, 2023 at 06:31:32PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Apr 06, 2023 at 06:11:03PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Apr 06, 2023 at 10:02:57PM +0200, Miguel Ojeda escreveu:
> > > On Thu, Apr 6, 2023 at 9:58 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > > >
> > > > Any chance of this being accepted?  I'm looking to use this in the radix
> > > > test suite and would rather not duplicate code.
> > > 
> > > That would be up to the `tools/perf/` maintainers.
> > 
> > Thanks, applied,
> 
> So, I noticed some missing conversions, fixed that, but then when trying
> to test build it with:
> 
> 	make -C tools/perf build-test
> 
> I stumbled on this:
> 
>              make_with_gtk2: cd . && make GTK2=1 -j32  DESTDIR=/tmp/tmp.5AP8tPJgiT
> cd . && make GTK2=1 -j32 DESTDIR=/tmp/tmp.5AP8tPJgiT
>   BUILD:   Doing 'make -j32' parallel build
>   HOSTCC  fixdep.o
>   HOSTLD  fixdep-in.o
> <SNIP>
>   CC      trace/beauty/ioctl.o
> In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9,
>                  from /usr/include/glib-2.0/glib/gtypes.h:34,
>                  from /usr/include/glib-2.0/glib/galloca.h:34,
>                  from /usr/include/glib-2.0/glib.h:32,
>                  from /usr/include/glib-2.0/gobject/gbinding.h:30,
>                  from /usr/include/glib-2.0/glib-object.h:24,
>                  from /usr/include/glib-2.0/gio/gioenums.h:30,
>                  from /usr/include/glib-2.0/gio/giotypes.h:30,
>                  from /usr/include/glib-2.0/gio/gio.h:28,
>                  from /usr/include/gtk-2.0/gdk/gdkapplaunchcontext.h:30,
>                  from /usr/include/gtk-2.0/gdk/gdk.h:32,
>                  from /usr/include/gtk-2.0/gtk/gtk.h:32,
>                  from ui/gtk/gtk.h:8,
>                  from ui/gtk/helpline.c:6:
> /usr/include/glib-2.0/glib/gmacros.h:637:28: error: missing ')' after "__has_attribute"
>   637 | #if g_macro__has_attribute(fallthrough)
>       |                            ^~~~~~~~~~~

I'm now trying with:

diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 8f3e43d148a8560b..d2dadf3873fbdf41 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
+#include "gtk.h"
 #include "../evsel.h"
 #include "../sort.h"
 #include "../hist.h"
 #include "../helpline.h"
-#include "gtk.h"
 
 #include <signal.h>
Arnaldo Carvalho de Melo April 6, 2023, 9:35 p.m. UTC | #7 
Em Thu, Apr 06, 2023 at 06:32:42PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Apr 06, 2023 at 06:31:32PM -0300, Arnaldo Carvalho de Melo escreveu:
> >                  from /usr/include/glib-2.0/gio/giotypes.h:30,
> >                  from /usr/include/glib-2.0/gio/gio.h:28,
> >                  from /usr/include/gtk-2.0/gdk/gdkapplaunchcontext.h:30,
> >                  from /usr/include/gtk-2.0/gdk/gdk.h:32,
> >                  from /usr/include/gtk-2.0/gtk/gtk.h:32,
> >                  from ui/gtk/gtk.h:8,
> >                  from ui/gtk/helpline.c:6:
> > /usr/include/glib-2.0/glib/gmacros.h:637:28: error: missing ')' after "__has_attribute"
> >   637 | #if g_macro__has_attribute(fallthrough)
> >       |                            ^~~~~~~~~~~
> 
> I'm now trying with:
> 
> diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
> index 8f3e43d148a8560b..d2dadf3873fbdf41 100644
> --- a/tools/perf/ui/gtk/browser.c
> +++ b/tools/perf/ui/gtk/browser.c
> @@ -1,9 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
> +#include "gtk.h"
>  #include "../evsel.h"
>  #include "../sort.h"
>  #include "../hist.h"
>  #include "../helpline.h"
> -#include "gtk.h"

Some more was needed, folding it with your patch.

- Arnaldo

diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 8f3e43d148a8560b..d2dadf3873fbdf41 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
+#include "gtk.h"
 #include "../evsel.h"
 #include "../sort.h"
 #include "../hist.h"
 #include "../helpline.h"
-#include "gtk.h"
 
 #include <signal.h>
 
diff --git a/tools/perf/ui/gtk/helpline.c b/tools/perf/ui/gtk/helpline.c
index e40a006aead87b9b..8ea95898a5bd0295 100644
--- a/tools/perf/ui/gtk/helpline.c
+++ b/tools/perf/ui/gtk/helpline.c
@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
+#include "gtk.h"
 #include <stdio.h>
 #include <string.h>
 #include <linux/kernel.h>
 
-#include "gtk.h"
 #include "../ui.h"
 #include "../helpline.h"
 
diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index c83be2d57f7eb7a7..bae21f336ae6d576 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include "gtk.h"
 #include "../evlist.h"
 #include "../callchain.h"
 #include "../evsel.h"
@@ -6,7 +7,6 @@
 #include "../hist.h"
 #include "../helpline.h"
 #include "../string2.h"
-#include "gtk.h"
 #include <signal.h>
 #include <stdlib.h>
 #include <linux/string.h>
diff mbox series

Patch

diff --git a/tools/include/linux/compiler-gcc.h b/tools/include/linux/compiler-gcc.h
index 8816f06fc6c7..62e7c901ac28 100644
--- a/tools/include/linux/compiler-gcc.h
+++ b/tools/include/linux/compiler-gcc.h
@@ -12,8 +12,10 @@ 
 		     + __GNUC_PATCHLEVEL__)
 #endif
 
-#if GCC_VERSION >= 70000 && !defined(__CHECKER__)
-# define __fallthrough __attribute__ ((fallthrough))
+#if __has_attribute(__fallthrough__)
+# define fallthrough                    __attribute__((__fallthrough__))
+#else
+# define fallthrough                    do {} while (0)  /* fallthrough */
 #endif
 
 #if __has_attribute(__error__)
diff --git a/tools/include/linux/compiler.h b/tools/include/linux/compiler.h
index 90ba44a99199..9d36c8ce1fe7 100644
--- a/tools/include/linux/compiler.h
+++ b/tools/include/linux/compiler.h
@@ -186,10 +186,6 @@  static __always_inline void __write_once_size(volatile void *p, void *res, int s
 })
 
 
-#ifndef __fallthrough
-# define __fallthrough
-#endif
-
 /* Indirect macros required for expanded argument pasting, eg. __LINE__. */
 #define ___PASTE(a, b) a##b
 #define __PASTE(a, b) ___PASTE(a, b)
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index f62298f5db3b..8539843d1909 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -706,7 +706,7 @@  __cmd_probe(int argc, const char **argv)
 			pr_err("  Error: --bootconfig doesn't support uprobes.\n");
 			return -EINVAL;
 		}
-		__fallthrough;
+		fallthrough;
 	case 'a':
 
 		/* Ensure the last given target is used */
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 265b05157972..19b0278d496c 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -616,7 +616,7 @@  static void process_evlist(struct evlist *evlist, unsigned int interval)
 	if (evlist__ctlfd_process(evlist, &cmd) > 0) {
 		switch (cmd) {
 		case EVLIST_CTL_CMD_ENABLE:
-			__fallthrough;
+			fallthrough;
 		case EVLIST_CTL_CMD_DISABLE:
 			if (interval)
 				process_interval();
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 4b3ff7687236..cfe1cde35528 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -707,7 +707,7 @@  static void *display_thread(void *arg)
 		case -1:
 			if (errno == EINTR)
 				continue;
-			__fallthrough;
+			fallthrough;
 		default:
 			c = getc(stdin);
 			tcsetattr(0, TCSAFLUSH, &save);
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index a093a15f048f..588f86a84eaa 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -724,7 +724,7 @@  static enum match_result match_chain(struct callchain_cursor_node *node,
 		if (match != MATCH_ERROR)
 			break;
 		/* otherwise fall-back to symbol-based comparison below */
-		__fallthrough;
+		fallthrough;
 	case CCKEY_FUNCTION:
 		if (node->ms.sym && cnode->ms.sym) {
 			/*
@@ -745,7 +745,7 @@  static enum match_result match_chain(struct callchain_cursor_node *node,
 			}
 		}
 		/* otherwise fall-back to IP-based comparison below */
-		__fallthrough;
+		fallthrough;
 	case CCKEY_ADDRESS:
 	default:
 		match = match_chain_dso_addresses(cnode->ms.map, cnode->ip, node->ms.map, node->ip);
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index 0ac860c8dd2b..57dea8074777 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -3544,7 +3544,7 @@  static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 		switch (decoder->packet.type) {
 		case INTEL_PT_TIP_PGD:
 			decoder->continuous_period = false;
-			__fallthrough;
+			fallthrough;
 		case INTEL_PT_TIP_PGE:
 		case INTEL_PT_TIP:
 		case INTEL_PT_PTWRITE:
@@ -3614,7 +3614,7 @@  static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 			decoder->pge = false;
 			decoder->continuous_period = false;
 			intel_pt_clear_tx_flags(decoder);
-			__fallthrough;
+			fallthrough;
 
 		case INTEL_PT_TNT:
 			decoder->have_tma = false;
@@ -3978,7 +3978,7 @@  const struct intel_pt_state *intel_pt_decode(struct intel_pt_decoder *decoder)
 			decoder->have_last_ip = false;
 			decoder->last_ip = 0;
 			decoder->ip = 0;
-			__fallthrough;
+			fallthrough;
 		case INTEL_PT_STATE_ERR_RESYNC:
 			err = intel_pt_sync_ip(decoder);
 			break;
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
index 18f97f43e01a..af9710622a1f 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
@@ -730,7 +730,7 @@  int intel_pt_pkt_desc(const struct intel_pt_pkt *packet, char *buf,
 	case INTEL_PT_FUP:
 		if (!(packet->count))
 			return snprintf(buf, buf_len, "%s no ip", name);
-		__fallthrough;
+		fallthrough;
 	case INTEL_PT_CYC:
 	case INTEL_PT_VMCS:
 	case INTEL_PT_MTC:
diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index b887dfeea673..8c62611f10aa 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -284,7 +284,7 @@  static int __ordered_events__flush(struct ordered_events *oe, enum oe_flush how,
 	switch (how) {
 	case OE_FLUSH__FINAL:
 		show_progress = true;
-		__fallthrough;
+		fallthrough;
 	case OE_FLUSH__TOP:
 		oe->next_flush = ULLONG_MAX;
 		break;
diff --git a/tools/perf/util/strfilter.c b/tools/perf/util/strfilter.c
index 78aa4c3b990d..02807b9d4122 100644
--- a/tools/perf/util/strfilter.c
+++ b/tools/perf/util/strfilter.c
@@ -274,7 +274,7 @@  static int strfilter_node__sprint(struct strfilter_node *node, char *buf)
 		len = strfilter_node__sprint_pt(node->l, buf);
 		if (len < 0)
 			return len;
-		__fallthrough;
+		fallthrough;
 	case '!':
 		if (buf) {
 			*(buf + len++) = *node->p;
diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 4f12a96f33cc..cf05b0b56c57 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -35,7 +35,7 @@  s64 perf_atoll(const char *str)
 			if (*p)
 				goto out_err;
 
-			__fallthrough;
+			fallthrough;
 		case '\0':
 			return length;
 		default: