lkml.org 
[lkml]   [2022]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] powerpc/perf: fix ppc64 perf probe add events failed
On Wed, 5 Jan 2022 10:56:46 +0800
chenzechuan <chenzechuan1@huawei.com> wrote:

>
> 在 2022/1/2 22:56, Arnaldo Carvalho de Melo 写道:
> > Em Tue, Dec 28, 2021 at 07:13:38PM +0800, Zechuan Chen escreveu:
> >> Because of commit bf794bf52a80 ("powerpc/kprobes: Fix kallsyms lookup
> >> across powerpc ABIv1 and ABIv2"), in ppc64 ABIv1, our perf command
> >> eliminates the need to use the prefix "." at the symbol name. But when
> >> the command "perf probe -a schedule" is executed on ppc64 ABIv1, it
> >> obtains two symbol address information through /proc/kallsyms, for example:
> >>
> >> cat /proc/kallsyms | grep -w schedule
> >> c000000000657020 T .schedule
> >> c000000000d4fdb8 D schedule
> >>
> >> The symbol "D schedule" is not a function symbol, and perf will print:
> >> "p:probe/schedule _text+13958584"Failed to write event: Invalid argument
> >>
> >> Therefore, when searching symbols from map and adding probe point for
> >> them, a symbol type check is added. If the type of symbol is not a
> >> function, skip it.
> >>
> >> Fixes: bf794bf52a80 ("powerpc/kprobes: Fix kallsyms lookup across powerpc ABIv1 and ABIv2")
> >>
> >> Signed-off-by: Zechuan Chen <chenzechuan1@huawei.com>
> >> ---
> >> tools/perf/util/probe-event.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >> index b2a02c9ab8ea..a834918a0a0d 100644
> >> --- a/tools/perf/util/probe-event.c
> >> +++ b/tools/perf/util/probe-event.c
> >> @@ -3083,6 +3083,9 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
> >> for (j = 0; j < num_matched_functions; j++) {
> >> sym = syms[j];
> >>
> >> + if (sym->type != STT_FUNC)
> >> + continue;
> >> +
> > Humm, shouldn't this be handled by find_probe_functions(), i.e. it
> > shoudn't return data symbols, right? Otherwise other places using this
> > function may malfunction as well?
> >
> > Naveen? Masami?
> >
> > - Arnaldo
>
> Re:Thanks for your review. You are right.
> 1. Only function symbols should be returned here, not function descriptors.
> I've moved the modification into function "find_probe_functions()",
> and the new patch is added at the end of this email. I've re-tested it on my environment.
> Please review it again, thanks.
>
> 2. Function find_probe_functions only use in "perf probe".
> Malfunction exist for all export functions, another example:
>
> cat /proc/kallsyms | grep -w sys_fork
> c000000000058100 T .sys_fork
> c000000000d485e0 D sys_fork
>
> perf probe -a sys_fork -v
> No kprobe blacklist support, ignored
> Looking at the vmlinux_path (8 entries long)
> symsrc__init: cannot get elf header.
> Could not open debuginfo. Try to use symbols.
> Looking at the vmlinux_path (8 entries long)
> symsrc__init: cannot get elf header.
> Using /proc/kcore for kernel data
> Using /proc/kallsyms for symbols
> Opening /sys/kernel/tracing//kprobe_events write=1
> Opening /sys/kernel/tracing//README write=0
> Writing event: p:probe/sys_fork _text+360704
> Writing event: p:probe/sys_fork _text+13927904
> Failed to write event: Invalid argument
> Error: Failed to add events. Reason: Invalid argument (Code: -22)
>
> >> /* There can be duplicated symbols in the map */
> >> for (i = 0; i < j; i++)
> >> if (sym->start == syms[i]->start) {
> >> --
> >> 2.12.3
> > .
>
> From 8d8f5de99d16e6aa8179ecbc33c0692fc887e2af Mon Sep 17 00:00:00 2001
> From: Zechuan Chen <chenzechuan1@huawei.com>
> Date: Mon, 27 Dec 2021 09:55:11 +0800
> Subject: [PATCH] powerpc/perf: fix ppc64 perf probe add events failed
>
> Because of commit bf794bf52a80 ("powerpc/kprobes: Fix kallsyms lookup
> across powerpc ABIv1 and ABIv2"), in ppc64 ABIv1, our perf command
> eliminates the need to use the prefix "." at the symbol name. But when
> the command "perf probe -a schedule" is executed on ppc64 ABIv1, it
> obtains two symbol address information through /proc/kallsyms, for example:
>
> cat /proc/kallsyms | grep -w schedule
> c000000000657020 T .schedule
> c000000000d4fdb8 D schedule
>
> The symbol "D schedule" is not a function symbol, and perf will print:
> "p:probe/schedule _text+13958584"Failed to write event: Invalid argument
>
> Therefore, when searching symbols from map and adding probe point for
> them, a symbol type check is added. If the type of symbol is not a
> function, skip it.

OK, this looks good to me. I forgot to check the sym->type :(

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you!

>
> Fixes: bf794bf52a80 ("powerpc/kprobes: Fix kallsyms lookup across
> powerpc ABIv1 and ABIv2")
>
> Signed-off-by: Zechuan Chen <chenzechuan1@huawei.com>
> ---
>  tools/perf/util/probe-event.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index b2a02c9ab8ea..4bd6f438d73c 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2959,6 +2959,9 @@ static int find_probe_functions(struct map *map,
> char *name,
>                 cut_version = false;
>
>         map__for_each_symbol(map, sym, tmp) {
> +               if (sym->type != STT_FUNC)
> +                       continue;
> +
>                 norm = arch__normalize_symbol_name(sym->name);
>                 if (!norm)
>                         continue;
> --
> 2.12.3
>


--
Masami Hiramatsu <mhiramat@kernel.org>

\
 
 \ /
  Last update: 2022-01-06 02:57    [W:0.074 / U:0.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site