From 3cf6a32f3f2a45944dd5be5c6ac4deb46bcd3bee Mon Sep 17 00:00:00 2001 From: Michael Petlan Date: Thu, 17 Mar 2022 14:55:36 +0100 Subject: [PATCH 1/3] perf symbols: Fix symbol size calculation condition Before this patch, the symbol end address fixup to be called, needed two conditions being met: if (prev->end == prev->start && prev->end != curr->start) Where "prev->end == prev->start" means that prev is zero-long (and thus needs a fixup) and "prev->end != curr->start" means that fixup hasn't been applied yet However, this logic is incorrect in the following situation: *curr = {rb_node = {__rb_parent_color = 278218928, rb_right = 0x0, rb_left = 0x0}, start = 0xc000000000062354, end = 0xc000000000062354, namelen = 40, type = 2 '\002', binding = 0 '\000', idle = 0 '\000', ignore = 0 '\000', inlined = 0 '\000', arch_sym = 0 '\000', annotate2 = false, name = 0x1159739e "kprobe_optinsn_page\t[__builtin__kprobes]"} *prev = {rb_node = {__rb_parent_color = 278219041, rb_right = 0x109548b0, rb_left = 0x109547c0}, start = 0xc000000000062354, end = 0xc000000000062354, namelen = 12, type = 2 '\002', binding = 1 '\001', idle = 0 '\000', ignore = 0 '\000', inlined = 0 '\000', arch_sym = 0 '\000', annotate2 = false, name = 0x1095486e "optinsn_slot"} In this case, prev->start == prev->end == curr->start == curr->end, thus the condition above thinks that "we need a fixup due to zero length of prev symbol, but it has been probably done, since the prev->end == curr->start", which is wrong. After the patch, the execution path proceeds to arch__symbols__fixup_end function which fixes up the size of prev symbol by adding page_size to its end offset. Fixes: 3b01a413c196c910 ("perf symbols: Improve kallsyms symbol end addr calculation") Signed-off-by: Michael Petlan Cc: Athira Jajeev Cc: Jiri Olsa Cc: Kajol Jain Cc: Madhavan Srinivasan Link: http://lore.kernel.org/lkml/20220317135536.805-1-mpetlan@redhat.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/symbol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index b2ed3140a1fa..dfde9eada224 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -231,7 +231,7 @@ void symbols__fixup_end(struct rb_root_cached *symbols) prev = curr; curr = rb_entry(nd, struct symbol, rb_node); - if (prev->end == prev->start && prev->end != curr->start) + if (prev->end == prev->start || prev->end != curr->start) arch__symbols__fixup_end(prev, curr); } From 8b464eac9765dfc84d0327fa3f3668faa439d1ce Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 17 Mar 2022 16:16:43 -0700 Subject: [PATCH 2/3] perf evlist: Avoid iteration for empty evlist. As seen with 'perf stat --null ..' and reported in: https://lore.kernel.org/lkml/YjCLcpcX2peeQVCH@kernel.org/ v2. Avoids setting evsel in the empty list case as suggested by Jiri Olsa. Committer testing: Before: $ perf stat --null sleep 1 Segmentation fault (core dumped) $ After: $ perf stat --null sleep 1 Performance counter stats for 'sleep 1': 1.010340646 seconds time elapsed 0.001420000 seconds user 0.000000000 seconds sys $ Fixes: 472832d2c000b961 ("perf evlist: Refactor evlist__for_each_cpu()") Reported-by: Arnaldo Carvalho de Melo Signed-off-by: Ian Rogers Tested-by: Arnaldo Carvalho de Melo Acked-by: Jiri Olsa Cc: Adrian Hunter Cc: Ian Rogers Cc: Namhyung Kim Link: https://lore.kernel.org/r/20220317231643.550902-1-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index eaad04e1672a..41a66a48cbdf 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -346,7 +346,7 @@ struct evlist_cpu_iterator evlist__cpu_begin(struct evlist *evlist, struct affin { struct evlist_cpu_iterator itr = { .container = evlist, - .evsel = evlist__first(evlist), + .evsel = NULL, .cpu_map_idx = 0, .evlist_cpu_map_idx = 0, .evlist_cpu_map_nr = perf_cpu_map__nr(evlist->core.all_cpus), @@ -354,16 +354,22 @@ struct evlist_cpu_iterator evlist__cpu_begin(struct evlist *evlist, struct affin .affinity = affinity, }; - if (itr.affinity) { - itr.cpu = perf_cpu_map__cpu(evlist->core.all_cpus, 0); - affinity__set(itr.affinity, itr.cpu.cpu); - itr.cpu_map_idx = perf_cpu_map__idx(itr.evsel->core.cpus, itr.cpu); - /* - * If this CPU isn't in the evsel's cpu map then advance through - * the list. - */ - if (itr.cpu_map_idx == -1) - evlist_cpu_iterator__next(&itr); + if (evlist__empty(evlist)) { + /* Ensure the empty list doesn't iterate. */ + itr.evlist_cpu_map_idx = itr.evlist_cpu_map_nr; + } else { + itr.evsel = evlist__first(evlist); + if (itr.affinity) { + itr.cpu = perf_cpu_map__cpu(evlist->core.all_cpus, 0); + affinity__set(itr.affinity, itr.cpu.cpu); + itr.cpu_map_idx = perf_cpu_map__idx(itr.evsel->core.cpus, itr.cpu); + /* + * If this CPU isn't in the evsel's cpu map then advance + * through the list. + */ + if (itr.cpu_map_idx == -1) + evlist_cpu_iterator__next(&itr); + } } return itr; } From 7bd1da15d211d439d96eb7cc8a35ce694b71d120 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 17 Mar 2022 15:43:09 -0700 Subject: [PATCH 3/3] perf parse-events: Ignore case in topdown.slots check An issue with icelakex metrics: https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json?h=perf/core&id=65eab2bc7dab326ee892ec5a4c749470b368b51a#n48 That causes the slots not to be first. Fixes: 94dbfd6781a0e87b ("perf parse-events: Architecture specific leader override") Reported-by: Caleb Biggers Signed-off-by: Ian Rogers Cc: Alexander Shishkin Cc: Alexandre Torgue Cc: Andi Kleen Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kan Liang Cc: Mark Rutland Cc: Maxime Coquelin Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Zhengjun Xing Link: https://lore.kernel.org/r/20220317224309.543736-1-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/arch/x86/util/evlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/arch/x86/util/evlist.c b/tools/perf/arch/x86/util/evlist.c index f924246eff78..8d9b55959256 100644 --- a/tools/perf/arch/x86/util/evlist.c +++ b/tools/perf/arch/x86/util/evlist.c @@ -29,7 +29,7 @@ struct evsel *arch_evlist__leader(struct list_head *list) __evlist__for_each_entry(list, evsel) { if (evsel->pmu_name && !strcmp(evsel->pmu_name, "cpu") && - evsel->name && strstr(evsel->name, "slots")) + evsel->name && strcasestr(evsel->name, "slots")) return evsel; } return first;