[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20200531162206.911168-1-jolsa@kernel.org>
Date: Sun, 31 May 2020 18:22:06 +0200
From: Jiri Olsa <jolsa@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Jin Yao <yao.jin@...ux.intel.com>,
lkml <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Michael Petlan <mpetlan@...hat.com>,
Ian Rogers <irogers@...gle.com>,
Stephane Eranian <eranian@...gle.com>,
Andi Kleen <ak@...ux.intel.com>
Subject: [PATCH] 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/ }
Fixes: 6a4bb04caacc8 ("perf tools: Enable grouping logic for parsed events")
Co-developed-by: Jin Yao <yao.jin@...ux.intel.com>
Signed-off-by: Jin Yao <yao.jin@...ux.intel.com>
Signed-off-by: Jiri Olsa <jolsa@...nel.org>
---
tools/perf/builtin-stat.c | 51 +++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index b2b79aa161dd..512a41363d07 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -190,6 +190,55 @@ 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 display dismantle the
+ * group and warn user.
+ */
+ WARN_ONCE(1, "WARNING: group events cpu maps do not match, disabling group:\n");
+ evsel__group_desc(leader, buf, sizeof(buf));
+ pr_warning(" %s\n", 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)
{
@@ -1962,6 +2011,8 @@ int cmd_stat(int argc, const char **argv)
} else if (argc && !strncmp(argv[0], "rep", 3))
return __cmd_report(argc, argv);
+ evlist__check_cpu_maps(evsel_list);
+
interval = stat_config.interval;
timeout = stat_config.timeout;
--
2.25.4
Powered by blists - more mailing lists