perf metric: Allow metrics with no events
authorIan Rogers <irogers@google.com>
Thu, 23 Sep 2021 07:46:12 +0000 (00:46 -0700)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Wed, 29 Sep 2021 16:50:16 +0000 (13:50 -0300)
A metric may be a constant value, for example, some SMT metrics are
constant 0 if #smt_on is 0. If we eliminate all the events then there is
no printing. Fix this by forcing metrics like this to have a
duration_time tool event, previously the metric would fail when parsing
the events with a parse error.

Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: John Garry <john.garry@huawei.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Clarke <pc@us.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sandeep Dasgupta <sdasgup@google.com>
Cc: Stephane Eranian <eranian@google.com>
Link: https://lore.kernel.org/r/20210923074616.674826-10-irogers@google.com
[ Reflow one __parse_events() call so that a ternary operation gets in a single line ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
tools/perf/util/metricgroup.c

index 046fb3f..8ba5370 100644 (file)
@@ -198,65 +198,69 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
        struct evsel *ev, *current_leader = NULL;
        struct expr_id_data *val_ptr;
        int i = 0, matched_events = 0, events_to_match;
-       const int idnum = (int)hashmap__size(pctx->ids);
+       int idnum = (int)hashmap__size(pctx->ids);
 
-       /*
-        * duration_time is always grouped separately, when events are grouped
-        * (ie has_constraint is false) then ignore it in the matching loop and
-        * add it to metric_events at the end.
-        */
-       if (!has_constraint &&
-           hashmap__find(pctx->ids, "duration_time", (void **)&val_ptr))
-               events_to_match = idnum - 1;
-       else
-               events_to_match = idnum;
-
-       evlist__for_each_entry (perf_evlist, ev) {
+       if (idnum != 0) {
                /*
-                * Events with a constraint aren't grouped and match the first
-                * events available.
+                * duration_time is always grouped separately, when events are
+                * grouped (ie has_constraint is false) then ignore it in the
+                * matching loop and add it to metric_events at the end.
                 */
-               if (has_constraint && ev->weak_group)
-                       continue;
-               /* Ignore event if already used and merging is disabled. */
-               if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
-                       continue;
-               if (!has_constraint && !evsel__has_leader(ev, current_leader)) {
+               events_to_match = idnum;
+               if (!has_constraint && hashmap__find(pctx->ids, "duration_time", (void **)&val_ptr))
+                       events_to_match--;
+
+               evlist__for_each_entry(perf_evlist, ev) {
+                       /*
+                        * Events with a constraint aren't grouped and match the
+                        * first events available.
+                        */
+                       if (has_constraint && ev->weak_group)
+                               continue;
+                       /* Ignore event if already used and merging is disabled. */
+                       if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
+                               continue;
+                       if (!has_constraint && !evsel__has_leader(ev, current_leader)) {
+                               /*
+                                * Start of a new group, discard the whole match
+                                * and start again.
+                                */
+                               matched_events = 0;
+                               memset(metric_events, 0, sizeof(struct evsel *) * idnum);
+                               current_leader = evsel__leader(ev);
+                       }
                        /*
-                        * Start of a new group, discard the whole match and
-                        * start again.
+                        * Check for duplicate events with the same name. For
+                        * example, uncore_imc/cas_count_read/ will turn into 6
+                        * events per socket on skylakex. Only the first such
+                        * event is placed in metric_events. If events aren't
+                        * grouped then this also ensures that the same event in
+                        * different sibling groups aren't both added to
+                        * metric_events.
                         */
-                       matched_events = 0;
-                       memset(metric_events, 0,
-                               sizeof(struct evsel *) * idnum);
-                       current_leader = evsel__leader(ev);
+                       if (contains_event(metric_events, matched_events, ev->name))
+                               continue;
+                       /* Does this event belong to the parse context? */
+                       if (hashmap__find(pctx->ids, ev->name, (void **)&val_ptr))
+                               metric_events[matched_events++] = ev;
+
+                       if (matched_events == events_to_match)
+                               break;
                }
+       } else {
                /*
-                * Check for duplicate events with the same name. For example,
-                * uncore_imc/cas_count_read/ will turn into 6 events per socket
-                * on skylakex. Only the first such event is placed in
-                * metric_events. If events aren't grouped then this also
-                * ensures that the same event in different sibling groups
-                * aren't both added to metric_events.
+                * There are no events to match, but we need to associate the
+                * metric with an event for printing. A duration_time event was
+                * parsed for this.
                 */
-               if (contains_event(metric_events, matched_events, ev->name))
-                       continue;
-               /* Does this event belong to the parse context? */
-               if (hashmap__find(pctx->ids, ev->name, (void **)&val_ptr))
-                       metric_events[matched_events++] = ev;
-
-               if (matched_events == events_to_match)
-                       break;
+               idnum = 1;
+               events_to_match = 0;
        }
-
        if (events_to_match != idnum) {
                /* Add the first duration_time. */
-               evlist__for_each_entry(perf_evlist, ev) {
-                       if (!strcmp(ev->name, "duration_time")) {
-                               metric_events[matched_events++] = ev;
-                               break;
-                       }
-               }
+               ev = evlist__find_evsel_by_str(perf_evlist, "duration_time");
+               if (ev)
+                       metric_events[matched_events++] = ev;
        }
 
        if (matched_events != idnum) {
@@ -320,9 +324,10 @@ static int metricgroup__setup_events(struct list_head *groups,
        list_for_each_entry (m, groups, nd) {
                struct evsel **metric_events;
                struct metric_ref *metric_refs = NULL;
+               const size_t ids_size = hashmap__size(m->pctx->ids);
 
                metric_events = calloc(sizeof(void *),
-                               hashmap__size(m->pctx->ids) + 1);
+                               ids_size == 0 ? 2 : ids_size + 1);
                if (!metric_events) {
                        ret = -ENOMEM;
                        break;
@@ -1240,7 +1245,8 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
                goto out;
        pr_debug("adding %s\n", extra_events.buf);
        bzero(&parse_error, sizeof(parse_error));
-       ret = __parse_events(perf_evlist, extra_events.buf, &parse_error, fake_pmu);
+       ret = __parse_events(perf_evlist, extra_events.len > 0 ? extra_events.buf : "duration_time",
+                            &parse_error, fake_pmu);
        if (ret) {
                parse_events_print_error(&parse_error, extra_events.buf);
                goto out;