Messages in this thread | | | Date | Thu, 6 Jan 2022 10:56:39 +0900 | From | Masami Hiramatsu <> | Subject | Re: [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>
| |