From a9a1790247bdcf3b1c8e2e5f8fbfc08d77cfd054 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Tue, 2 Jun 2020 12:17:36 +0200 Subject: perf stat: Ensure group is defined on top of the same cpu mask Jin Yao reported the issue (and posted first versions of this change) with groups being defined over events with different cpu mask. This causes assert aborts in get_group_fd, like: # perf stat -M "C2_Pkg_Residency" -a -- sleep 1 perf: util/evsel.c:1464: get_group_fd: Assertion `!(fd == -1)' failed. Aborted All the events in the group have to be defined over the same cpus so the group_fd can be found for every leader/member pair. Adding check to ensure this condition is met and removing the group (with warning) if we detect mixed cpus, like: $ sudo perf stat -e '{power/energy-cores/,cycles},{instructions,power/energy-cores/}' WARNING: event cpu maps do not match, disabling group: anon group { power/energy-cores/, cycles } anon group { instructions, power/energy-cores/ } Ian asked also for cpu maps details, it's displayed in verbose mode: $ sudo perf stat -e '{cycles,power/energy-cores/}' -v WARNING: group events cpu maps do not match, disabling group: anon group { power/energy-cores/, cycles } power/energy-cores/: 0 cycles: 0-7 anon group { instructions, power/energy-cores/ } instructions: 0-7 power/energy-cores/: 0 Committer testing: [root@seventh ~]# perf stat -e '{power/energy-cores/,cycles},{instructions,power/energy-cores/}' WARNING: grouped events cpus do not match, disabling group: anon group { power/energy-cores/, cycles } anon group { instructions, power/energy-cores/ } ^C Performance counter stats for 'system wide': 12.62 Joules power/energy-cores/ 106,920,637 cycles 80,228,899 instructions # 0.75 insn per cycle 12.62 Joules power/energy-cores/ 14.514476987 seconds time elapsed [root@seventh ~]# But if we put compatible events in each group it works: [root@seventh ~]# perf stat -e '{power/energy-cores/,power/energy-ram/},{instructions,cycles}' -a sleep 2 Performance counter stats for 'system wide': 1.95 Joules power/energy-cores/ 0.92 Joules power/energy-ram/ 29,305,715 instructions # 1.03 insn per cycle 28,423,338 cycles 2.001438142 seconds time elapsed [root@seventh ~]# This needs improvement tho: [root@seventh ~]# perf stat -e '{power/energy-cores/,power/energy-ram/},{instructions,cycles}' sleep 2 Error: The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (power/energy-cores/). /bin/dmesg | grep -i perf may provide additional information. [root@seventh ~]# We need to emit a better message, one stating that the power/ events can't be used for a specific workload, instead it is per-cpu or system wide. Fixes: 6a4bb04caacc8 ("perf tools: Enable grouping logic for parsed events") Co-developed-by: Jin Yao Signed-off-by: Jiri Olsa Acked-by: Ian Rogers Tested-by: Arnaldo Carvalho de Melo Cc: Alexander Shishkin Cc: Andi Kleen Cc: Jiri Olsa Cc: Michael Petlan Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lore.kernel.org/lkml/20200602101736.GE1112120@krava Signed-off-by: Jin Yao Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-stat.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) (limited to 'tools/perf') diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index b2b79aa161dd..9be020e0098a 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -190,6 +190,59 @@ static struct perf_stat_config stat_config = { .big_num = true, }; +static bool cpus_map_matched(struct evsel *a, struct evsel *b) +{ + if (!a->core.cpus && !b->core.cpus) + return true; + + if (!a->core.cpus || !b->core.cpus) + return false; + + if (a->core.cpus->nr != b->core.cpus->nr) + return false; + + for (int i = 0; i < a->core.cpus->nr; i++) { + if (a->core.cpus->map[i] != b->core.cpus->map[i]) + return false; + } + + return true; +} + +static void evlist__check_cpu_maps(struct evlist *evlist) +{ + struct evsel *evsel, *pos, *leader; + char buf[1024]; + + evlist__for_each_entry(evlist, evsel) { + leader = evsel->leader; + + /* Check that leader matches cpus with each member. */ + if (leader == evsel) + continue; + if (cpus_map_matched(leader, evsel)) + continue; + + /* If there's mismatch disable the group and warn user. */ + WARN_ONCE(1, "WARNING: grouped events cpus do not match, disabling group:\n"); + evsel__group_desc(leader, buf, sizeof(buf)); + pr_warning(" %s\n", buf); + + if (verbose) { + cpu_map__snprint(leader->core.cpus, buf, sizeof(buf)); + pr_warning(" %s: %s\n", leader->name, buf); + cpu_map__snprint(evsel->core.cpus, buf, sizeof(buf)); + pr_warning(" %s: %s\n", evsel->name, buf); + } + + for_each_group_evsel(pos, leader) { + pos->leader = pos; + pos->core.nr_members = 0; + } + evsel->leader->core.nr_members = 0; + } +} + static inline void diff_timespec(struct timespec *r, struct timespec *a, struct timespec *b) { @@ -2113,6 +2166,8 @@ int cmd_stat(int argc, const char **argv) goto out; } + evlist__check_cpu_maps(evsel_list); + /* * Initialize thread_map with comm names, * so we could print it out on output. -- cgit v1.2.3-59-g8ed1b