* [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic @ 2020-05-12 12:23 Jiri Olsa 2020-05-12 13:32 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 5+ messages in thread From: Jiri Olsa @ 2020-05-12 12:23 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Adrian Hunter, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan Adrian reported that is_bpf_image is not working the way it was intended - passing on trampolines and dispatcher names. Instead it returned true for all the bpf names. The reason even this logic worked properly is that all bpf objects, even trampolines and dispatcher, were assigned DSO_BINARY_TYPE__BPF_IMAGE binary_type. The later for bpf_prog objects, the binary_type was fixed in bpf load event processing, which is executed after the ksymbol code. Fixing the is_bpf_image logic, so it properly recognizes trampoline and dispatcher objects. Reported-by: Adrian Hunter <adrian.hunter@intel.com> Signed-off-by: Jiri Olsa <jolsa@redhat.com> --- tools/perf/util/machine.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 8ed2135893bb..d5384807372b 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -738,8 +738,8 @@ int machine__process_switch_event(struct machine *machine __maybe_unused, static int is_bpf_image(const char *name) { - return strncmp(name, "bpf_trampoline_", sizeof("bpf_trampoline_") - 1) || - strncmp(name, "bpf_dispatcher_", sizeof("bpf_dispatcher_") - 1); + return strncmp(name, "bpf_trampoline_", sizeof("bpf_trampoline_") - 1) == 0 || + strncmp(name, "bpf_dispatcher_", sizeof("bpf_dispatcher_") - 1) == 0; } static int machine__process_ksymbol_register(struct machine *machine, -- 2.25.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic 2020-05-12 12:23 [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic Jiri Olsa @ 2020-05-12 13:32 ` Arnaldo Carvalho de Melo 2020-05-12 13:36 ` Jiri Olsa 0 siblings, 1 reply; 5+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-12 13:32 UTC (permalink / raw) To: Jiri Olsa Cc: Adrian Hunter, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan Em Tue, May 12, 2020 at 02:23:10PM +0200, Jiri Olsa escreveu: > Adrian reported that is_bpf_image is not working the way it was > intended - passing on trampolines and dispatcher names. Instead > it returned true for all the bpf names. > > The reason even this logic worked properly is that all bpf objects, > even trampolines and dispatcher, were assigned DSO_BINARY_TYPE__BPF_IMAGE > binary_type. > > The later for bpf_prog objects, the binary_type was fixed in bpf load event > processing, which is executed after the ksymbol code. > > Fixing the is_bpf_image logic, so it properly recognizes trampoline > and dispatcher objects. This is not applying on top of torvalds/master, not tip/perf/urgent, and you forgot to add the Fixes: line, lemme try to find this... - Arnaldo > Reported-by: Adrian Hunter <adrian.hunter@intel.com> > Signed-off-by: Jiri Olsa <jolsa@redhat.com> > --- > tools/perf/util/machine.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > index 8ed2135893bb..d5384807372b 100644 > --- a/tools/perf/util/machine.c > +++ b/tools/perf/util/machine.c > @@ -738,8 +738,8 @@ int machine__process_switch_event(struct machine *machine __maybe_unused, > > static int is_bpf_image(const char *name) > { > - return strncmp(name, "bpf_trampoline_", sizeof("bpf_trampoline_") - 1) || > - strncmp(name, "bpf_dispatcher_", sizeof("bpf_dispatcher_") - 1); > + return strncmp(name, "bpf_trampoline_", sizeof("bpf_trampoline_") - 1) == 0 || > + strncmp(name, "bpf_dispatcher_", sizeof("bpf_dispatcher_") - 1) == 0; > } > > static int machine__process_ksymbol_register(struct machine *machine, > -- > 2.25.4 > -- - Arnaldo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic 2020-05-12 13:32 ` Arnaldo Carvalho de Melo @ 2020-05-12 13:36 ` Jiri Olsa 2020-05-12 14:09 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 5+ messages in thread From: Jiri Olsa @ 2020-05-12 13:36 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, Adrian Hunter, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan On Tue, May 12, 2020 at 10:32:23AM -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, May 12, 2020 at 02:23:10PM +0200, Jiri Olsa escreveu: > > Adrian reported that is_bpf_image is not working the way it was > > intended - passing on trampolines and dispatcher names. Instead > > it returned true for all the bpf names. > > > > The reason even this logic worked properly is that all bpf objects, > > even trampolines and dispatcher, were assigned DSO_BINARY_TYPE__BPF_IMAGE > > binary_type. > > > > The later for bpf_prog objects, the binary_type was fixed in bpf load event > > processing, which is executed after the ksymbol code. > > > > Fixing the is_bpf_image logic, so it properly recognizes trampoline > > and dispatcher objects. > > This is not applying on top of torvalds/master, not tip/perf/urgent, and right.. it's on top of your's perf/core.. I can rebase on perf/urgent > you forgot to add the Fixes: line, lemme try to find this... oops, sorry Fixes: 3c29d4483e85 ("perf annotate: Add basic support for bpf_image") jirka > > - Arnaldo > > > Reported-by: Adrian Hunter <adrian.hunter@intel.com> > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> > > --- > > tools/perf/util/machine.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > > index 8ed2135893bb..d5384807372b 100644 > > --- a/tools/perf/util/machine.c > > +++ b/tools/perf/util/machine.c > > @@ -738,8 +738,8 @@ int machine__process_switch_event(struct machine *machine __maybe_unused, > > > > static int is_bpf_image(const char *name) > > { > > - return strncmp(name, "bpf_trampoline_", sizeof("bpf_trampoline_") - 1) || > > - strncmp(name, "bpf_dispatcher_", sizeof("bpf_dispatcher_") - 1); > > + return strncmp(name, "bpf_trampoline_", sizeof("bpf_trampoline_") - 1) == 0 || > > + strncmp(name, "bpf_dispatcher_", sizeof("bpf_dispatcher_") - 1) == 0; > > } > > > > static int machine__process_ksymbol_register(struct machine *machine, > > -- > > 2.25.4 > > > > -- > > - Arnaldo > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic 2020-05-12 13:36 ` Jiri Olsa @ 2020-05-12 14:09 ` Arnaldo Carvalho de Melo 2020-05-12 14:34 ` Jiri Olsa 0 siblings, 1 reply; 5+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-12 14:09 UTC (permalink / raw) To: Jiri Olsa Cc: Jiri Olsa, Adrian Hunter, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan Em Tue, May 12, 2020 at 03:36:09PM +0200, Jiri Olsa escreveu: > On Tue, May 12, 2020 at 10:32:23AM -0300, Arnaldo Carvalho de Melo wrote: > > Em Tue, May 12, 2020 at 02:23:10PM +0200, Jiri Olsa escreveu: > > > Adrian reported that is_bpf_image is not working the way it was > > > intended - passing on trampolines and dispatcher names. Instead > > > it returned true for all the bpf names. > > > > > > The reason even this logic worked properly is that all bpf objects, > > > even trampolines and dispatcher, were assigned DSO_BINARY_TYPE__BPF_IMAGE > > > binary_type. > > > > > > The later for bpf_prog objects, the binary_type was fixed in bpf load event > > > processing, which is executed after the ksymbol code. > > > > > > Fixing the is_bpf_image logic, so it properly recognizes trampoline > > > and dispatcher objects. > > > > This is not applying on top of torvalds/master, not tip/perf/urgent, and > > right.. it's on top of your's perf/core.. I can rebase on perf/urgent You don't need to, this hasn't hit torvalds/master, it'll be in the next merge window, the one for 5.8. > > you forgot to add the Fixes: line, lemme try to find this... > > oops, sorry > >Fixes: 3c29d4483e85 ("perf annotate: Add basic support for bpf_image") I did it already, and: [acme@five perf]$ git tag --contains 3c29d4483e85 perf-core-for-mingo-5.8-20200420 perf-core-for-mingo-5.8-20200506 perf-for-bpf-2020-05-06 [acme@five perf]$ So can't go to perf/urgent at the moment. - Arnaldo > jirka > > > > > - Arnaldo > > > > > Reported-by: Adrian Hunter <adrian.hunter@intel.com> > > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> > > > --- > > > tools/perf/util/machine.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > > > index 8ed2135893bb..d5384807372b 100644 > > > --- a/tools/perf/util/machine.c > > > +++ b/tools/perf/util/machine.c > > > @@ -738,8 +738,8 @@ int machine__process_switch_event(struct machine *machine __maybe_unused, > > > > > > static int is_bpf_image(const char *name) > > > { > > > - return strncmp(name, "bpf_trampoline_", sizeof("bpf_trampoline_") - 1) || > > > - strncmp(name, "bpf_dispatcher_", sizeof("bpf_dispatcher_") - 1); > > > + return strncmp(name, "bpf_trampoline_", sizeof("bpf_trampoline_") - 1) == 0 || > > > + strncmp(name, "bpf_dispatcher_", sizeof("bpf_dispatcher_") - 1) == 0; > > > } > > > > > > static int machine__process_ksymbol_register(struct machine *machine, > > > -- > > > 2.25.4 > > > > > > > -- > > > > - Arnaldo > > > -- - Arnaldo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic 2020-05-12 14:09 ` Arnaldo Carvalho de Melo @ 2020-05-12 14:34 ` Jiri Olsa 0 siblings, 0 replies; 5+ messages in thread From: Jiri Olsa @ 2020-05-12 14:34 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, Adrian Hunter, lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan On Tue, May 12, 2020 at 11:09:02AM -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, May 12, 2020 at 03:36:09PM +0200, Jiri Olsa escreveu: > > On Tue, May 12, 2020 at 10:32:23AM -0300, Arnaldo Carvalho de Melo wrote: > > > Em Tue, May 12, 2020 at 02:23:10PM +0200, Jiri Olsa escreveu: > > > > Adrian reported that is_bpf_image is not working the way it was > > > > intended - passing on trampolines and dispatcher names. Instead > > > > it returned true for all the bpf names. > > > > > > > > The reason even this logic worked properly is that all bpf objects, > > > > even trampolines and dispatcher, were assigned DSO_BINARY_TYPE__BPF_IMAGE > > > > binary_type. > > > > > > > > The later for bpf_prog objects, the binary_type was fixed in bpf load event > > > > processing, which is executed after the ksymbol code. > > > > > > > > Fixing the is_bpf_image logic, so it properly recognizes trampoline > > > > and dispatcher objects. > > > > > > This is not applying on top of torvalds/master, not tip/perf/urgent, and > > > > right.. it's on top of your's perf/core.. I can rebase on perf/urgent > > You don't need to, this hasn't hit torvalds/master, it'll be in the next > merge window, the one for 5.8. > perfect, thanks jirka ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-12 14:35 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-12 12:23 [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic Jiri Olsa 2020-05-12 13:32 ` Arnaldo Carvalho de Melo 2020-05-12 13:36 ` Jiri Olsa 2020-05-12 14:09 ` Arnaldo Carvalho de Melo 2020-05-12 14:34 ` Jiri Olsa
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.