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 <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Miguel Ojeda <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Nathan Chancellor <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Tom Rix <[email protected]>
Signed-off-by: Liam Howlett <[email protected]>
---
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(-)
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:
--
2.35.1
On Fri, Nov 25, 2022 at 4:50 PM Liam Howlett <[email protected]> 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 <[email protected]>
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
On Thu, Apr 6, 2023 at 9:58 PM Liam R. Howlett <[email protected]> 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
* Miguel Ojeda <[email protected]> [221125 13:51]:
> On Fri, Nov 25, 2022 at 4:50 PM Liam Howlett <[email protected]> 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 <[email protected]>
>
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
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 <[email protected]> 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
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 <[email protected]> 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
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 <[email protected]> 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>
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>