[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160809150915.GB2293@kernel.org>
Date: Tue, 9 Aug 2016 12:09:15 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>,
linux-kernel@...r.kernel.org, peterz@...radead.org,
mingo@...hat.com, alexander.shishkin@...ux.intel.com,
bsingharora@...il.com, naveen.n.rao@...ux.vnet.ibm.com,
ananth@...ibm.com, wangnan0@...wei.com, namhyung@...nel.org
Subject: Re: [PATCH 2/2] perf ppc64le: Fix probe location when using DWARF
Em Tue, Aug 09, 2016 at 11:02:40PM +0900, Masami Hiramatsu escreveu:
> On Tue, 9 Aug 2016 01:23:25 -0500
> Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com> 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 <mhiramat@...nel.org>
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 <ravi.bangoria@...ux.vnet.ibm.com>
> > ---
> > 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 <mhiramat@...nel.org>
Powered by blists - more mailing lists