[PATCH perf/urgent] perf tools: Fix is_bpf_image function logic

[PATCH perf/urgent] perf tools: Fix is_bpf_image function logic

[Jiri Olsa]

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

Re: [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic

[Arnaldo Carvalho de Melo]

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

Re: [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic

[Jiri Olsa]

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
> 

Re: [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic

[Arnaldo Carvalho de Melo]

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

Re: [PATCH perf/urgent] perf tools: Fix is_bpf_image function logic

[Jiri Olsa]

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