| Message ID | 20221125154947.2163498-1-Liam.Howlett@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools: Rename __fallthrough to fallthrough | expand |
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
* 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
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
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
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
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>
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 --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:
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(-)