On Wed, Jul 05, 2017 at 01:02:36PM -0700, Krister Johansen wrote: > Hey Greg, > > > 4.9-stable review patch. If anyone has any objections, please let me know. > > > > ------------------ > > > > From: Masami Hiramatsu <[email protected]> > > > > > > [ Upstream commit 613f050d68a8ed3c0b18b9568698908ef7bbc1f7 ] > > > > 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 > > > > This also fixes same problem on online module as below. > > > > $ perf probe -m i915 -D assert_plane > > p:probe/assert_plane i915:assert_plane.constprop.134+0 > > > > Signed-off-by: Masami Hiramatsu <[email protected]> > > Tested-by: Arnaldo Carvalho de Melo <[email protected]> > > Cc: Jiri Olsa <[email protected]> > > Cc: Namhyung Kim <[email protected]> > > Cc: Peter Zijlstra <[email protected]> > > Link: > > http://lkml.kernel.org/r/148411450673.9978.14905987549651656075.stgit@devbox > > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]> > > Signed-off-by: Sasha Levin <[email protected]> > > Signed-off-by: Greg Kroah-Hartman <[email protected]> > > --- > > tools/perf/util/probe-event.c | 45 > > ++++++++++++++++++++++++++--------------- > > tools/perf/util/probe-finder.c | 7 ++++-- > > tools/perf/util/probe-finder.h | 3 ++ > > 3 files changed, 37 insertions(+), 18 deletions(-) > > > > --- a/tools/perf/util/probe-event.c > > +++ b/tools/perf/util/probe-event.c > > @@ -645,18 +645,31 @@ static int add_exec_to_probe_trace_event > > 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 = 0; > > int i, ret = 0; > > char *mod_name = NULL; > > + struct map *map; > > > > if (!module) > > return 0; > > > > - mod_name = find_module_name(module); > > + map = get_target_map(module, false); > > + if (!map || debuginfo__get_text_offset(dinfo, &text_offs, true) < 0) { > > + pr_warning("Failed to get ELF symbols for %s\n", module); > > + return -EINVAL; > > + } > > > > + mod_name = find_module_name(module); > > for (i = 0; i < ntevs; i++) { > > + ret = post_process_probe_trace_point(&tevs[i].point, > > + map, (unsigned long)text_offs); > > + if (ret < 0) > > + break; > > tevs[i].point.module = > > strdup(mod_name ? mod_name : module); > > if (!tevs[i].point.module) { > > @@ -666,6 +679,8 @@ static int add_module_to_probe_trace_eve > > } > > > > free(mod_name); > > + map__put(map); > > + > > return ret; > > } > > > > @@ -722,7 +737,7 @@ arch__post_process_probe_trace_events(st > > 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; > > > > @@ -730,7 +745,8 @@ static int post_process_probe_trace_even > > 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); > > > > @@ -774,30 +790,27 @@ static int try_to_find_probe_trace_event > > } > > } > > > > - 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"); > > --- a/tools/perf/util/probe-finder.c > > +++ b/tools/perf/util/probe-finder.c > > @@ -1501,7 +1501,8 @@ int debuginfo__find_available_vars_at(st > > } > > > > /* 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, > > + bool adjust_offset) > > { > > int n, i; > > Elf32_Word shndx; > > @@ -1530,6 +1531,8 @@ static int debuginfo__get_text_offset(st > > if (!shdr) > > return -ENOENT; > > *offs = shdr->sh_addr; > > + if (adjust_offset) > > + *offs -= shdr->sh_offset; > > } > > } > > return 0; > > @@ -1545,7 +1548,7 @@ int debuginfo__find_probe_point(struct d > > int baseline = 0, lineno = 0, ret = 0; > > > > /* We always need to relocate the address for aranges */ > > - if (debuginfo__get_text_offset(dbg, &baseaddr) == 0) > > + if (debuginfo__get_text_offset(dbg, &baseaddr, false) == 0) > > addr += baseaddr; > > /* Find cu die */ > > if (!dwarf_addrdie(dbg->dbg, (Dwarf_Addr)addr, &cudie)) { > > --- a/tools/perf/util/probe-finder.h > > +++ b/tools/perf/util/probe-finder.h > > @@ -46,6 +46,9 @@ int debuginfo__find_trace_events(struct > > 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, > > + bool adjust_offset); > > + > > /* Find a line range */ > > int debuginfo__find_line_range(struct debuginfo *dbg, struct line_range > > *lr); > > I'm getting the following error when I try to build perf from 4.9.36: > > util/probe-event.c: In function ‘post_process_module_probe_trace_events’: > util/probe-event.c:685:3: error: implicit declaration of function > ‘post_process_probe_trace_point’ [-Werror=implicit-function-declaration] > ret = post_process_probe_trace_point(&tevs[i].point, > ^ > util/probe-event.c:685:3: error: nested extern declaration of > ‘post_process_probe_trace_point’ [-Werror=nested-externs] > cc1: all warnings being treated as errors > > At first blush, it looks like we're missing another patch upon which > this one depends. However, after cherry-picking the fix for > 3e96dac7c956089d3f23aca98c4dfca57b6aaf8a back to 4.9.36, the build then > fails with: > > util/probe-event.c:549:12: note: declared here > static int get_text_start_address(const char *exec, unsigned long > *address, > ^ > util/probe-event.c: At top level: > util/probe-event.c:672:1: error: ‘post_process_offline_probe_trace_events’ > defined but not used [-Werror=unused-function] > post_process_offline_probe_trace_events(struct probe_trace_event *tevs, > ^ > cc1: all warnings being treated as errors > > It turns out that this has a second dependency, > 8a937a25a7e3c19d5fb3f9d92f605cf5fda219d8. After fiddling with this a > little bit more, I realized I need to invert the order to get a clean > merge of the two fixes. This ordering is good: > > 8a937a25a7e3c19d5fb3f9d92f605cf5fda219d8 > 3e96dac7c956089d3f23aca98c4dfca57b6aaf8a > > With that I can get a clean build of the perf tree. All of that said, > I'm not sure whether there are additional patches needed to fully > leverage the added functionality. Perhaps Arnaldo or Masami can > comment?
Thanks for these, I've now queued them up. greg k-h
