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 <yao.jin@linux.intel.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lore.kernel.org/lkml/20200602101736.GE1112120@krava
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index b2b79aa161ddfadd4e9a6bf6fbd46e5103971021..9be020e0098ad0020d39c41c22bd97ffcc4a111f 100644 (file)
--- 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.