From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752661AbcHIPJd (ORCPT ); Tue, 9 Aug 2016 11:09:33 -0400 Received: from mail.kernel.org ([198.145.29.136]:43182 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752297AbcHIPJY (ORCPT ); Tue, 9 Aug 2016 11:09:24 -0400 Date: Tue, 9 Aug 2016 12:09:15 -0300 From: Arnaldo Carvalho de Melo To: Masami Hiramatsu Cc: Ravi Bangoria , linux-kernel@vger.kernel.org, peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com, bsingharora@gmail.com, naveen.n.rao@linux.vnet.ibm.com, ananth@in.ibm.com, wangnan0@huawei.com, namhyung@kernel.org Subject: Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF Message-ID: <20160809150915.GB2293@kernel.org> References: <1470723805-5081-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com> <1470723805-5081-2-git-send-email-ravi.bangoria@linux.vnet.ibm.com> <20160809230240.f8bfbda8b0554a853438b770@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160809230240.f8bfbda8b0554a853438b770@kernel.org> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Aug 09, 2016 at 11:02:40PM +0900, Masami Hiramatsu escreveu: > On Tue, 9 Aug 2016 01:23:25 -0500 > Ravi Bangoria wrote: > > > Powerpc has Global Entry Point and Local Entry Point for functions. > > LEP catches call from both the GEP and the LEP. Symbol table of ELF > > contains GEP and Offset from which we can calculate LEP, but debuginfo > > does not have LEP info. > > > > Currently, perf prioritize symbol table over dwarf to probe on LEP > > for ppc64le. But when user tries to probe with function parameter, > > we fall back to using dwarf(i.e. GEP) and when function called via > > LEP, probe will never hit. > > > > For example: > > $ objdump -d vmlinux > > ... > > do_sys_open(): > > c0000000002eb4a0: e8 00 4c 3c addis r2,r12,232 > > c0000000002eb4a4: 60 00 42 38 addi r2,r2,96 > > c0000000002eb4a8: a6 02 08 7c mflr r0 > > c0000000002eb4ac: d0 ff 41 fb std r26,-48(r1) > > > > $ sudo ./perf probe do_sys_open > > $ sudo cat /sys/kernel/debug/tracing/kprobe_events > > p:probe/do_sys_open _text+3060904 > > > > $ sudo ./perf probe 'do_sys_open filename:string' > > $ sudo cat /sys/kernel/debug/tracing/kprobe_events > > p:probe/do_sys_open _text+3060896 filename_string=+0(%gpr4):string > > > > For second case, perf probed on GEP. So when function will be called > > via LEP, probe won't hit. > > > > $ sudo ./perf record -a -e probe:do_sys_open ls > > [ perf record: Woken up 1 times to write data ] > > [ perf record: Captured and wrote 0.195 MB perf.data ] > > > > To resolve this issue, let's not prioritize symbol table, let perf > > decide what it wants to use. Perf is already converting GEP to LEP > > when it uses symbol table. When perf uses debuginfo, let it find > > LEP offset form symbol table. This way we fall back to probe on LEP > > for all cases. > > > > After patch: > > $ sudo ./perf probe 'do_sys_open filename:string' > > $ sudo cat /sys/kernel/debug/tracing/kprobe_events > > p:probe/do_sys_open _text+3060904 filename_string=+0(%gpr4):string > > > > $ sudo ./perf record -a -e probe:do_sys_open ls > > [ perf record: Woken up 1 times to write data ] > > [ perf record: Captured and wrote 0.197 MB perf.data (11 samples) ] > > OK, I see. > And I'd like to add note below. > > This reverts commit d5c2e2c17ae1 ("perf probe ppc64le: Prefer symbol > table lookup over DWARF"). > > Acked-by: Masami Hiramatsu Ok, I'll do that to expedite this a little bit, trying to get this in my next perf/urgent pull req to Ingo, which I expect to send soon. - Arnaldo > for this series. > > Thanks, > > > > > Signed-off-by: Ravi Bangoria > > --- > > tools/perf/arch/powerpc/util/sym-handling.c | 27 +++++++++++++++++---- > > tools/perf/util/probe-event.c | 37 ++++++++++++++++------------- > > tools/perf/util/probe-event.h | 6 ++++- > > 3 files changed, 49 insertions(+), 21 deletions(-) > > > > diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c > > index c6d0f91..8d4dc97 100644 > > --- a/tools/perf/arch/powerpc/util/sym-handling.c > > +++ b/tools/perf/arch/powerpc/util/sym-handling.c > > @@ -54,10 +54,6 @@ int arch__compare_symbol_names(const char *namea, const char *nameb) > > #endif > > > > #if defined(_CALL_ELF) && _CALL_ELF == 2 > > -bool arch__prefers_symtab(void) > > -{ > > - return true; > > -} > > > > #ifdef HAVE_LIBELF_SUPPORT > > void arch__sym_update(struct symbol *s, GElf_Sym *sym) > > @@ -100,4 +96,27 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev, > > tev->point.offset += lep_offset; > > } > > } > > + > > +void arch__post_process_probe_trace_events(struct perf_probe_event *pev, > > + int ntevs) > > +{ > > + struct probe_trace_event *tev; > > + struct map *map; > > + struct symbol *sym = NULL; > > + struct rb_node *tmp; > > + int i = 0; > > + > > + map = get_target_map(pev->target, pev->uprobes); > > + if (!map || map__load(map, NULL) < 0) > > + return; > > + > > + for (i = 0; i < ntevs; i++) { > > + tev = &pev->tevs[i]; > > + map__for_each_symbol(map, sym, tmp) { > > + if (map->unmap_ip(map, sym->start) == tev->point.address) > > + arch__fix_tev_from_maps(pev, tev, map, sym); > > + } > > + } > > +} > > + > > #endif > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > > index 4e215e7..5efa535 100644 > > --- a/tools/perf/util/probe-event.c > > +++ b/tools/perf/util/probe-event.c > > @@ -178,7 +178,7 @@ static struct map *kernel_get_module_map(const char *module) > > return NULL; > > } > > > > -static struct map *get_target_map(const char *target, bool user) > > +struct map *get_target_map(const char *target, bool user) > > { > > /* Init maps of given executable or kernel */ > > if (user) > > @@ -703,19 +703,32 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs, > > return skipped; > > } > > > > +void __weak > > +arch__post_process_probe_trace_events(struct perf_probe_event *pev __maybe_unused, > > + int ntevs __maybe_unused) > > +{ > > +} > > + > > /* Post processing the probe events */ > > -static int post_process_probe_trace_events(struct probe_trace_event *tevs, > > +static int post_process_probe_trace_events(struct perf_probe_event *pev, > > + struct probe_trace_event *tevs, > > int ntevs, const char *module, > > bool uprobe) > > { > > - if (uprobe) > > - return add_exec_to_probe_trace_events(tevs, ntevs, module); > > + int ret; > > > > - if (module) > > + if (uprobe) > > + ret = add_exec_to_probe_trace_events(tevs, ntevs, module); > > + else if (module) > > /* Currently ref_reloc_sym based probe is not for drivers */ > > - return add_module_to_probe_trace_events(tevs, ntevs, module); > > + ret = add_module_to_probe_trace_events(tevs, ntevs, module); > > + else > > + ret = post_process_kernel_probe_trace_events(tevs, ntevs); > > > > - return post_process_kernel_probe_trace_events(tevs, ntevs); > > + if (ret >= 0) > > + arch__post_process_probe_trace_events(pev, ntevs); > > + > > + return ret; > > } > > > > /* Try to find perf_probe_event with debuginfo */ > > @@ -756,7 +769,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev, > > > > if (ntevs > 0) { /* Succeeded to find trace events */ > > pr_debug("Found %d probe_trace_events.\n", ntevs); > > - ret = post_process_probe_trace_events(*tevs, ntevs, > > + ret = post_process_probe_trace_events(pev, *tevs, ntevs, > > pev->target, pev->uprobes); > > if (ret < 0 || ret == ntevs) { > > clear_probe_trace_events(*tevs, ntevs); > > @@ -2943,8 +2956,6 @@ errout: > > return err; > > } > > > > -bool __weak arch__prefers_symtab(void) { return false; } > > - > > /* Concatinate two arrays */ > > static void *memcat(void *a, size_t sz_a, void *b, size_t sz_b) > > { > > @@ -3165,12 +3176,6 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev, > > if (ret > 0 || pev->sdt) /* SDT can be found only in the cache */ > > return ret == 0 ? -ENOENT : ret; /* Found in probe cache */ > > > > - if (arch__prefers_symtab() && !perf_probe_event_need_dwarf(pev)) { > > - ret = find_probe_trace_events_from_map(pev, tevs); > > - if (ret > 0) > > - return ret; /* Found in symbol table */ > > - } > > - > > /* Convert perf_probe_event with debuginfo */ > > ret = try_to_find_probe_trace_events(pev, tevs); > > if (ret != 0) > > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h > > index e18ea9f..f4f45db 100644 > > --- a/tools/perf/util/probe-event.h > > +++ b/tools/perf/util/probe-event.h > > @@ -158,7 +158,6 @@ int show_line_range(struct line_range *lr, const char *module, bool user); > > int show_available_vars(struct perf_probe_event *pevs, int npevs, > > struct strfilter *filter); > > int show_available_funcs(const char *module, struct strfilter *filter, bool user); > > -bool arch__prefers_symtab(void); > > void arch__fix_tev_from_maps(struct perf_probe_event *pev, > > struct probe_trace_event *tev, struct map *map, > > struct symbol *sym); > > @@ -173,4 +172,9 @@ int e_snprintf(char *str, size_t size, const char *format, ...) > > int copy_to_probe_trace_arg(struct probe_trace_arg *tvar, > > struct perf_probe_arg *pvar); > > > > +struct map *get_target_map(const char *target, bool user); > > + > > +void arch__post_process_probe_trace_events(struct perf_probe_event *pev, > > + int ntevs); > > + > > #endif /*_PROBE_EVENT_H */ > > -- > > 2.7.4 > > > > > -- > Masami Hiramatsu