Commit f01642e4 authored by Jin Yao's avatar Jin Yao Committed by Arnaldo Carvalho de Melo

perf metricgroup: Support multiple events for metricgroup

Some uncore metrics don't work as expected. For example, on
cascadelakex:

  root@lkp-csl-2sp2:~# perf stat -M UNC_M_PMM_BANDWIDTH.TOTAL -a -- sleep 1

   Performance counter stats for 'system wide':

           1841092      unc_m_pmm_rpq_inserts
           3680816      unc_m_pmm_wpq_inserts

       1.001775055 seconds time elapsed

  root@lkp-csl-2sp2:~# perf stat -M UNC_M_PMM_READ_LATENCY -a -- sleep 1

   Performance counter stats for 'system wide':

         860649746      unc_m_pmm_rpq_occupancy.all
           1840557      unc_m_pmm_rpq_inserts
       12790627455      unc_m_clockticks

       1.001773348 seconds time elapsed

No metrics 'UNC_M_PMM_BANDWIDTH.TOTAL' or 'UNC_M_PMM_READ_LATENCY' are
reported.

The issue is, the case of an alias expanding to mulitple events is not
supported, typically the uncore events.  (see comments in
find_evsel_group()).

For UNC_M_PMM_BANDWIDTH.TOTAL in above example, the expanded event group
is '{unc_m_pmm_rpq_inserts,unc_m_pmm_wpq_inserts}:W', but the actual
events passed to find_evsel_group are:

  unc_m_pmm_rpq_inserts
  unc_m_pmm_rpq_inserts
  unc_m_pmm_rpq_inserts
  unc_m_pmm_rpq_inserts
  unc_m_pmm_rpq_inserts
  unc_m_pmm_rpq_inserts
  unc_m_pmm_wpq_inserts
  unc_m_pmm_wpq_inserts
  unc_m_pmm_wpq_inserts
  unc_m_pmm_wpq_inserts
  unc_m_pmm_wpq_inserts
  unc_m_pmm_wpq_inserts

For this multiple events case, it's not supported well.

This patch introduces a new field 'metric_leader' in struct evsel. The
first event is considered as a metric leader. For the rest of same
events, they point to the first event via it's metric_leader field in
struct evsel.

This design is for adding the counting results of all same events to the
first event in group (the metric_leader).

With this patch,

  root@lkp-csl-2sp2:~# perf stat -M UNC_M_PMM_BANDWIDTH.TOTAL -a -- sleep 1

   Performance counter stats for 'system wide':

           1842108      unc_m_pmm_rpq_inserts     #    337.2 MB/sec  UNC_M_PMM_BANDWIDTH.TOTAL
           3682209      unc_m_pmm_wpq_inserts

       1.001819706 seconds time elapsed

  root@lkp-csl-2sp2:~# perf stat -M UNC_M_PMM_READ_LATENCY -a -- sleep 1

   Performance counter stats for 'system wide':

         861970685      unc_m_pmm_rpq_occupancy.all #    219.4 ns  UNC_M_PMM_READ_LATENCY
           1842772      unc_m_pmm_rpq_inserts
       12790196356      unc_m_clockticks

       1.001749103 seconds time elapsed

Now we can see the correct metrics 'UNC_M_PMM_BANDWIDTH.TOTAL' and
'UNC_M_PMM_READ_LATENCY'.
Signed-off-by: default avatarJin Yao <yao.jin@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20190828055932.8269-5-yao.jin@linux.intel.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent 287f2649
...@@ -168,6 +168,7 @@ struct evsel { ...@@ -168,6 +168,7 @@ struct evsel {
const char * metric_expr; const char * metric_expr;
const char * metric_name; const char * metric_name;
struct evsel **metric_events; struct evsel **metric_events;
struct evsel *metric_leader;
bool collect_stat; bool collect_stat;
bool weak_group; bool weak_group;
bool percore; bool percore;
......
...@@ -90,57 +90,61 @@ struct egroup { ...@@ -90,57 +90,61 @@ struct egroup {
const char *metric_unit; const char *metric_unit;
}; };
static bool record_evsel(int *ind, struct evsel **start,
int idnum,
struct evsel **metric_events,
struct evsel *ev)
{
metric_events[*ind] = ev;
if (*ind == 0)
*start = ev;
if (++*ind == idnum) {
metric_events[*ind] = NULL;
return true;
}
return false;
}
static struct evsel *find_evsel_group(struct evlist *perf_evlist, static struct evsel *find_evsel_group(struct evlist *perf_evlist,
const char **ids, const char **ids,
int idnum, int idnum,
struct evsel **metric_events) struct evsel **metric_events)
{ {
struct evsel *ev, *start = NULL; struct evsel *ev;
int ind = 0; int i = 0;
bool leader_found;
evlist__for_each_entry (perf_evlist, ev) { evlist__for_each_entry (perf_evlist, ev) {
if (ev->collect_stat) if (!strcmp(ev->name, ids[i])) {
continue; if (!metric_events[i])
if (!strcmp(ev->name, ids[ind])) { metric_events[i] = ev;
if (record_evsel(&ind, &start, idnum,
metric_events, ev))
return start;
} else { } else {
/* if (++i == idnum) {
* We saw some other event that is not /* Discard the whole match and start again */
* in our list of events. Discard i = 0;
* the whole match and start again. memset(metric_events, 0,
*/ sizeof(struct evsel *) * idnum);
ind = 0; continue;
start = NULL; }
if (!strcmp(ev->name, ids[ind])) {
if (record_evsel(&ind, &start, idnum, if (!strcmp(ev->name, ids[i]))
metric_events, ev)) metric_events[i] = ev;
return start; else {
/* Discard the whole match and start again */
i = 0;
memset(metric_events, 0,
sizeof(struct evsel *) * idnum);
continue;
} }
} }
} }
/*
* This can happen when an alias expands to multiple if (i != idnum - 1) {
* events, like for uncore events. /* Not whole match */
* We don't support this case for now. return NULL;
*/ }
return NULL;
metric_events[idnum] = NULL;
for (i = 0; i < idnum; i++) {
leader_found = false;
evlist__for_each_entry(perf_evlist, ev) {
if (!leader_found && (ev == metric_events[i]))
leader_found = true;
if (leader_found &&
!strcmp(ev->name, metric_events[i]->name)) {
ev->metric_leader = metric_events[i];
}
}
}
return metric_events[0];
} }
static int metricgroup__setup_events(struct list_head *groups, static int metricgroup__setup_events(struct list_head *groups,
......
...@@ -31,6 +31,8 @@ struct saved_value { ...@@ -31,6 +31,8 @@ struct saved_value {
int cpu; int cpu;
struct runtime_stat *stat; struct runtime_stat *stat;
struct stats stats; struct stats stats;
u64 metric_total;
int metric_other;
}; };
static int saved_value_cmp(struct rb_node *rb_node, const void *entry) static int saved_value_cmp(struct rb_node *rb_node, const void *entry)
...@@ -212,6 +214,7 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count, ...@@ -212,6 +214,7 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count,
{ {
int ctx = evsel_context(counter); int ctx = evsel_context(counter);
u64 count_ns = count; u64 count_ns = count;
struct saved_value *v;
count *= counter->scale; count *= counter->scale;
...@@ -266,9 +269,15 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count, ...@@ -266,9 +269,15 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count,
update_runtime_stat(st, STAT_APERF, ctx, cpu, count); update_runtime_stat(st, STAT_APERF, ctx, cpu, count);
if (counter->collect_stat) { if (counter->collect_stat) {
struct saved_value *v = saved_value_lookup(counter, cpu, true, v = saved_value_lookup(counter, cpu, true, STAT_NONE, 0, st);
STAT_NONE, 0, st);
update_stats(&v->stats, count); update_stats(&v->stats, count);
if (counter->metric_leader)
v->metric_total += count;
} else if (counter->metric_leader) {
v = saved_value_lookup(counter->metric_leader,
cpu, true, STAT_NONE, 0, st);
v->metric_total += count;
v->metric_other++;
} }
} }
...@@ -729,10 +738,10 @@ static void generic_metric(struct perf_stat_config *config, ...@@ -729,10 +738,10 @@ static void generic_metric(struct perf_stat_config *config,
char *n, *pn; char *n, *pn;
expr__ctx_init(&pctx); expr__ctx_init(&pctx);
expr__add_id(&pctx, name, avg);
for (i = 0; metric_events[i]; i++) { for (i = 0; metric_events[i]; i++) {
struct saved_value *v; struct saved_value *v;
struct stats *stats; struct stats *stats;
u64 metric_total = 0;
if (!strcmp(metric_events[i]->name, "duration_time")) { if (!strcmp(metric_events[i]->name, "duration_time")) {
stats = &walltime_nsecs_stats; stats = &walltime_nsecs_stats;
...@@ -744,6 +753,9 @@ static void generic_metric(struct perf_stat_config *config, ...@@ -744,6 +753,9 @@ static void generic_metric(struct perf_stat_config *config,
break; break;
stats = &v->stats; stats = &v->stats;
scale = 1.0; scale = 1.0;
if (v->metric_other)
metric_total = v->metric_total;
} }
n = strdup(metric_events[i]->name); n = strdup(metric_events[i]->name);
...@@ -757,8 +769,15 @@ static void generic_metric(struct perf_stat_config *config, ...@@ -757,8 +769,15 @@ static void generic_metric(struct perf_stat_config *config,
pn = strchr(n, ' '); pn = strchr(n, ' ');
if (pn) if (pn)
*pn = 0; *pn = 0;
expr__add_id(&pctx, n, avg_stats(stats)*scale);
if (metric_total)
expr__add_id(&pctx, n, metric_total);
else
expr__add_id(&pctx, n, avg_stats(stats)*scale);
} }
expr__add_id(&pctx, name, avg);
if (!metric_events[i]) { if (!metric_events[i]) {
const char *p = metric_expr; const char *p = metric_expr;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment