A metric like DRAM_BW_Use has on SkylakeX events uncore_imc/cas_count_read/
and uncore_imc/case_count_write/. These events open 6 events per socket
with pmu names of uncore_imc_[0-5]. The current metric setup code in
find_evsel_group assumes one ID will map to 1 event to be recorded in
metric_events. For events with multiple matches, the first event is
recorded in metric_events (avoiding matching >1 event with the same
name) and the evlist_used updated so that duplicate events aren't
removed when the evlist has unused events removed.
Before this change:
$ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
Performance counter stats for 'system wide':
41.14 MiB uncore_imc/cas_count_read/
1,002,614,251 ns duration_time
1.002614251 seconds time elapsed
After this change:
$ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1
Performance counter stats for 'system wide':
157.47 MiB uncore_imc/cas_count_read/ # 0.00 DRAM_BW_Use
126.97 MiB uncore_imc/cas_count_write/
1,003,019,728 ns duration_time
v2. avoids iterating over the whole evlist as suggested by
namhy...@kernel.org. It also fixes the metric_leader computation
that was broken in the same commits.
Erroneous duplication introduced in:
commit 2440689d62e9 ("perf metricgroup: Remove duped metric group events").
Fixes: ded80bda8bc9 ("perf expr: Migrate expr ids table to a hashmap").
Reported-by: Jin Yao <yao....@linux.intel.com>
Signed-off-by: Ian Rogers <irog...@google.com>
---
tools/perf/util/metricgroup.c | 45 ++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 662f4e8777d5..79080de9217d 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -206,6 +206,18 @@ static struct evsel *find_evsel_group(struct evlist
*perf_evlist,
sizeof(struct evsel *) * idnum);
current_leader = ev->leader;
}
+ /*
+ * 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.
+ */
+ for (i = 0; i < matched_events; i++) {
+ if (!strcmp(metric_events[i]->name, ev->name))
+ break;
+ }
+ if (i != matched_events)
+ continue;
if (hashmap__find(&pctx->ids, ev->name, (void **)&val_ptr)) {
if (has_constraint) {
/*
@@ -245,9 +257,36 @@ static struct evsel *find_evsel_group(struct evlist
*perf_evlist,
metric_events[idnum] = NULL;
for (i = 0; i < idnum; i++) {
- ev = metric_events[i];
- ev->metric_leader = ev;
- set_bit(ev->idx, evlist_used);
+ /* Don't free used events. */
+ set_bit(metric_events[i]->idx, evlist_used);
+ /*
+ * The metric leader points to the identically named event in
+ * metric_events.
+ */
+ metric_events[i]->metric_leader = metric_events[i];
+ /*
+ * Mark two events with identical names in the same group (or
+ * globally) as being in use as uncore events may be duplicated
+ * for each pmu. Set the metric leader to be the event that
+ * appears in metric_events.
+ */
+ if (!has_constraint) {
+ for_each_group_evsel(ev, metric_events[i]->leader) {
+ if (ev != metric_events[i] &&
+ !strcmp(metric_events[i]->name, ev->name)) {
+ set_bit(ev->idx, evlist_used);
+ ev->metric_leader = metric_events[i];
+ }
+ }
+ } else {
+ evlist__for_each_entry(perf_evlist, ev) {
+ if (ev != metric_events[i] &&
+ !strcmp(metric_events[i]->name, ev->name)) {
+ set_bit(ev->idx, evlist_used);
+ ev->metric_leader = metric_events[i];
+ }
+ }
+ }
}
return metric_events[0];
--
2.28.0.526.ge36021eeef-goog
