Message-ID: <20170105153037.GN27864@kernel.org>
Date:   Thu, 5 Jan 2017 12:30:37 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>
Subject: Re: [PATCH perf/core 3/3] perf-probe: Fix to probe on gcc generated
 functions in modules

Em Thu, Jan 05, 2017 at 08:20:19PM +0900, Masami Hiramatsu escreveu:
> On Wed, 4 Jan 2017 11:48:56 -0300
> Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
> 
> > Em Wed, Jan 04, 2017 at 12:31:39PM +0900, Masami Hiramatsu escreveu:
> > > Fix to probe on gcc generated functions on modules. Since
> > > probing on a module is based on its symbol name, it should
> > > be adjusted on actual symbols.
> > > 
> > > E.g. without this fix, perf probe shows probe definition
> > > on non-exist symbol as below.
> > >   -----
> > >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -F in_range*
> > >   in_range.isra.12
> > >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
> > >   p:probe/in_range nf_nat:in_range+0
> > >   -----
> > > With this fix, perf probe correctly shows a probe on
> > > gcc-generated symbol.
> > >   -----
> > >   $ perf probe -m build-x86_64/net/netfilter/nf_nat.ko -D in_range
> > >   p:probe/in_range nf_nat:in_range.isra.12+0
> > >   -----
> > 
> > Tested this one on a x86-64 fedora25 system, applied all three.
> > 
> > As always, please take a look at the patch when sent to Ingo, I usually
> > put some committer notes there, in this case I put the test steps I
> > performed, using e1000e.ko and e1000_flash_cycle_ich8lan.constprop.22
> > 
> > BTW, it would be cool if...
> > 
> > Oops, I retract that, while I was testing what I was went to ask you to
> > implement, I saw that this last patch broke this use case:
> > 
> > [root@...et ~]# perf probe -m e1000e e1000_xmit_frame
> > Failed to get ELF symbols for e1000e
> > Probe point 'e1000_xmit_frame' not found.
> >   Error: Failed to add events.
> > [root@...et ~]#
> 
> Oops, I missed that case.
> However, when I trided to fix that on fedora25,
> 
> $ perf probe -m i915 -vD assert_plane
> probe-definition(0): assert_plane
> symbol:assert_plane file:(null) line:0 offset:0 return:0 lazy:(null)
> 0 arguments
> No kprobe blacklist support, ignored
> Failed to get build-id from i915.
> Cache open error: -1
> Open Debuginfo file: /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz
> Try to find probe point from debuginfo.
> Matched function: assert_plane [6fb806]
> found inline addr: 0x886a0
> Probe point found: assert_plane+0
> Found 1 probe_trace_events.
> text offset: 10030
> Failed to get ELF symbols for /lib/modules/4.8.15-300.fc25.x86_64/kernel/drivers/gpu/drm/i915/i915.ko.xz
> Post processing failed or all events are skipped. (-22)
> Probe point 'assert_plane' not found.
>   Error: Failed to add events. Reason: No such file or directory (Code: -2)
> 
> As far as I can see, elfutils's libdw supports compressed file, but
> libelf API (perf's API) seems not supporting it. This results in
> succeeding opening debuginfo, but failing ELF symbols.
> 
> > 
> > If I remove it, I get it back:
> > 
> > [root@...et ~]# perf probe -m e1000e e1000_xmit_frame
> > Added new event:
> >   probe:e1000_xmit_frame (on e1000_xmit_frame in e1000e)
> > 
> > You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe:e1000_xmit_frame -aR sleep 1
> > 
> > [root@...et ~]# perf probe -l
> >   probe:e1000_xmit_frame (on e1000_get_link_up_info_80003es2lan:7@...el/e1000e/80003es2lan.c in e1000e)
> > [root@...et ~]#
> > 
> > BTW, what I would find cool, besides fixing this new problem, would be that:
> > 
> >   perf probe e1000_xmit_frame
> > 
> > worked, as we can get the module name from kallsyms:
> > 
> > [acme@...et linux]$ grep e1000_xmit_frame /proc/kallsyms 
> > ffffffffc046fc10 t e1000_xmit_frame	[e1000e]
> > [acme@...et linux]$ 
> 
> OK, it sounds reasonable to me too. BTW, how can I get the map for kallsyms?
> May machine__findnew_module_map(host_machine, ,"[kernel.kallsyms]")
> include module symbols too?

Well, the machine abstraction is there for that, i.e. to get all the
abstractions related to a machine (kernel, kernel modules, threads,
maps, symbols).

We have 'struct machines' as well, to deal with VMs, etc. This can come
from the running system or from a perf.data file, when it may be for a
different architecture than the machine where analysis is done.

So, "map for kallsyms" in fact should be "maps for kallsyms", as it will
split the maps for each module, etc.

Probably what you want is:

 symbol = machine__find_kernel_function_by_name(machine, name, &map);

Then, if symbol is not NULL, map->dso->name will have your module name,
map->dso->long_name will have the .ko path, etc.

- Arnaldo

> 
> > 
> > I applied the first two patches, and now I'm preparing a pull req to Ingo,
> 
> Thanks!
> 
> > 
> > Thanks!
> > 
> > - Arnaldo
> >  
> > > Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
> > > ---
> > >  tools/perf/util/probe-event.c  |   38 +++++++++++++++++++++++---------------
> > >  tools/perf/util/probe-finder.c |    2 +-
> > >  tools/perf/util/probe-finder.h |    2 ++
> > >  3 files changed, 26 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > index 4a57c8a..f75c99a 100644
> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -682,15 +682,19 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
> > >  	return ret;
> > >  }
> > >  
> > > -static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
> > > -					    int ntevs, const char *module)
> > > +static int
> > > +post_process_module_probe_trace_events(struct probe_trace_event *tevs,
> > > +				       int ntevs, const char *module,
> > > +				       struct debuginfo *dinfo)
> > >  {
> > > +	Dwarf_Addr text_offs;
> > >  	int i, ret = 0;
> > >  	char *mod_name = NULL;
> > >  
> > >  	if (!module)
> > >  		return 0;
> > >  
> > > +	debuginfo__get_text_offset(dinfo, &text_offs);
> > >  	mod_name = find_module_name(module);
> > >  
> > >  	for (i = 0; i < ntevs; i++) {
> > > @@ -700,9 +704,15 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
> > >  			ret = -ENOMEM;
> > >  			break;
> > >  		}
> > > +		/* kernel module needs a special care to adjust addresses */
> > > +		tevs[i].point.address -= (unsigned long)text_offs;
> > >  	}
> > >  
> > >  	free(mod_name);
> > > +
> > > +	if (!ret)
> > > +		ret = post_process_offline_probe_trace_events(tevs, ntevs,
> > > +								module);
> > >  	return ret;
> > >  }
> > >  
> > > @@ -760,7 +770,7 @@ arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unuse
> > >  static int post_process_probe_trace_events(struct perf_probe_event *pev,
> > >  					   struct probe_trace_event *tevs,
> > >  					   int ntevs, const char *module,
> > > -					   bool uprobe)
> > > +					   bool uprobe, struct debuginfo *dinfo)
> > >  {
> > >  	int ret;
> > >  
> > > @@ -768,7 +778,8 @@ static int post_process_probe_trace_events(struct perf_probe_event *pev,
> > >  		ret = add_exec_to_probe_trace_events(tevs, ntevs, module);
> > >  	else if (module)
> > >  		/* Currently ref_reloc_sym based probe is not for drivers */
> > > -		ret = add_module_to_probe_trace_events(tevs, ntevs, module);
> > > +		ret = post_process_module_probe_trace_events(tevs, ntevs,
> > > +							     module, dinfo);
> > >  	else
> > >  		ret = post_process_kernel_probe_trace_events(tevs, ntevs);
> > >  
> > > @@ -812,30 +823,27 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> > >  		}
> > >  	}
> > >  
> > > -	debuginfo__delete(dinfo);
> > > -
> > >  	if (ntevs > 0) {	/* Succeeded to find trace events */
> > >  		pr_debug("Found %d probe_trace_events.\n", ntevs);
> > >  		ret = post_process_probe_trace_events(pev, *tevs, ntevs,
> > > -						pev->target, pev->uprobes);
> > > +					pev->target, pev->uprobes, dinfo);
> > >  		if (ret < 0 || ret == ntevs) {
> > > +			pr_debug("Post processing failed or all events are skipped. (%d)\n", ret);
> > >  			clear_probe_trace_events(*tevs, ntevs);
> > >  			zfree(tevs);
> > > +			ntevs = 0;
> > >  		}
> > > -		if (ret != ntevs)
> > > -			return ret < 0 ? ret : ntevs;
> > > -		ntevs = 0;
> > > -		/* Fall through */
> > >  	}
> > >  
> > > +	debuginfo__delete(dinfo);
> > > +
> > >  	if (ntevs == 0)	{	/* No error but failed to find probe point. */
> > >  		pr_warning("Probe point '%s' not found.\n",
> > >  			   synthesize_perf_probe_point(&pev->point));
> > >  		return -ENOENT;
> > > -	}
> > > -	/* Error path : ntevs < 0 */
> > > -	pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
> > > -	if (ntevs < 0) {
> > > +	} else if (ntevs < 0) {
> > > +		/* Error path : ntevs < 0 */
> > > +		pr_debug("An error occurred in debuginfo analysis (%d).\n", ntevs);
> > >  		if (ntevs == -EBADF)
> > >  			pr_warning("Warning: No dwarf info found in the vmlinux - "
> > >  				"please rebuild kernel with CONFIG_DEBUG_INFO=y.\n");
> > > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > > index df4debe..6dede24 100644
> > > --- a/tools/perf/util/probe-finder.c
> > > +++ b/tools/perf/util/probe-finder.c
> > > @@ -1501,7 +1501,7 @@ int debuginfo__find_available_vars_at(struct debuginfo *dbg,
> > >  }
> > >  
> > >  /* For the kernel module, we need a special code to get a DIE */
> > > -static int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
> > > +int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs)
> > >  {
> > >  	int n, i;
> > >  	Elf32_Word shndx;
> > > diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> > > index f1d8558..1e03bbb 100644
> > > --- a/tools/perf/util/probe-finder.h
> > > +++ b/tools/perf/util/probe-finder.h
> > > @@ -46,6 +46,8 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
> > >  int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
> > >  				struct perf_probe_point *ppt);
> > >  
> > > +int debuginfo__get_text_offset(struct debuginfo *dbg, Dwarf_Addr *offs);
> > > +
> > >  /* Find a line range */
> > >  int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range *lr);
> > >  
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@...nel.org>

Powered by blists - more mailing lists