Skip to content

Commit

Permalink
perf metric: Allow modifiers on metrics
Browse files Browse the repository at this point in the history 
By allowing modifiers on metrics we can, for example, gather the
same metric for kernel and user mode. On a SkylakeX with
TopDownL1 this gives:

  $ perf stat -M TopDownL1:u,TopDownL1:k -a sleep 2

   Performance counter stats for 'system wide':

         849,855,577    uops_issued.any:k         #     0.06 Bad_Speculation:k
                                                  #     0.51 Backend_Bound:k          (16.71%)
       1,995,257,996    cycles:k
                                                  # 7981031984.00 SLOTS:k
                                                  #     0.35 Frontend_Bound:k
                                                  #     0.08 Retiring:k               (16.71%)
       2,791,940,753    idq_uops_not_delivered.core:k                                 (16.71%)
         641,961,928    uops_retired.retire_slots:k                                   (16.71%)
          72,239,337    int_misc.recovery_cycles:k                                    (16.71%)
       2,294,413,647    uops_issued.any:u         #     0.04 Bad_Speculation:u
                                                  #     0.39 Backend_Bound:u          (16.78%)
       1,333,248,940    cycles:u
                                                  # 5332995760.00 SLOTS:u
                                                  #     0.16 Frontend_Bound:u
                                                  #     0.40 Retiring:u               (16.78%)
         858,517,081    idq_uops_not_delivered.core:u                                 (16.78%)
       2,153,789,582    uops_retired.retire_slots:u                                   (16.78%)
          19,373,627    int_misc.recovery_cycles:u                                    (16.78%)
          31,503,661    cpu_clk_unhalted.one_thread_active:k #     0.18 CoreIPC_SMT:k (16.73%)
         315,454,104    inst_retired.any:k        # 315454104.00 Instructions:k       (16.73%)
          42,533,729    cpu_clk_unhalted.ref_xclk:k                                   (16.73%)
       2,043,119,037    cpu_clk_unhalted.thread:k                                     (16.73%)
          28,843,803    cpu_clk_unhalted.one_thread_active:u #     1.55 CoreIPC_SMT:u (16.60%)
       2,153,353,869    inst_retired.any:u        # 2153353869.00 Instructions:u      (16.60%)
          28,844,743    cpu_clk_unhalted.ref_xclk:u                                   (16.60%)
       1,387,544,378    cpu_clk_unhalted.thread:u                                     (16.60%)
         308,031,603    inst_retired.any:k        #     0.15 CoreIPC:k                (33.19%)
       2,036,774,753    cycles:k                                                      (33.19%)
       1,994,344,281    inst_retired.any:u        #     1.59 CoreIPC:u                (33.18%)
       1,251,538,227    cycles:u                                                      (33.18%)

         2.000342948 seconds time elapsed

Modifiers are naively copy and pasted on to events, this can yield errors like:

  $ perf stat -M Kernel_Utilization:k -a sleep 2
  event syntax error: '..d.thread:k/kk,cpu_clk_unhalted.thread/metric-id=cpu_clk_unhalted.thread/k..'
                                    \___ Bad modifier

   Usage: perf stat [<options>] [<command>]

      -M, --metrics <metric/metric group list>
                            monitor specified metrics or metric groups (separated by ,)

When modifiers are present with constraints, from --metric-no-group or
the NMI watchdog, they are no longer placed in the same set - which may
miss deduplicating events.

Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Andi Kleen <ak@linux.intel.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Antonov <alexander.antonov@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andrew Kilroy <andrew.kilroy@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Changbin Du <changbin.du@intel.com>
Cc: Denys Zagorui <dzagorui@cisco.com>
Cc: Fabian Hemmer <copy@copy.sh>
Cc: Felix Fietkau <nbd@nbd.name>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Joakim Zhang <qiangqing.zhang@nxp.com>
Cc: John Garry <john.garry@huawei.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Kees Kook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Nicholas Fraser <nfraser@codeweavers.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Paul Clarke <pc@us.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Riccardo Mancini <rickyman7@gmail.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: ShihCheng Tu <mrtoastcheng@gmail.com>
Cc: Song Liu <songliubraving@fb.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sumanth Korikkar <sumanthk@linux.ibm.com>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Cc: Wan Jiabing <wanjiabing@vivo.com>
Cc: Zhen Lei <thunder.leizhen@huawei.com>
Link: https://lore.kernel.org/r/20211015172132.1162559-22-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  • Loading branch information
Ian Rogers authored and Arnaldo Carvalho de Melo committed Oct 20, 2021
1 parent eabd452 commit b85a4d6
Showing 1 changed file with 98 additions and 27 deletions.
125 changes: 98 additions & 27 deletions tools/perf/util/metricgroup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ static void metric_event_delete(struct rblist *rblist __maybe_unused,
struct metric_expr *expr, *tmp;

list_for_each_entry_safe(expr, tmp, &me->head, nd) {
free((char *)expr->metric_name);
free(expr->metric_refs);
free(expr->metric_events);
free(expr);
Expand Down Expand Up @@ -130,6 +131,8 @@ struct metric {
struct expr_parse_ctx *pctx;
/** The name of the metric such as "IPC". */
const char *metric_name;
/** Modifier on the metric such as "u" or NULL for none. */
const char *modifier;
/** The expression to parse, for example, "instructions/cycles". */
const char *metric_expr;
/**
Expand Down Expand Up @@ -186,6 +189,7 @@ static bool metricgroup__has_constraint(const struct pmu_event *pe)
}

static struct metric *metric__new(const struct pmu_event *pe,
const char *modifier,
bool metric_no_group,
int runtime)
{
Expand All @@ -202,6 +206,12 @@ static struct metric *metric__new(const struct pmu_event *pe,
}

m->metric_name = pe->metric_name;
m->modifier = modifier ? strdup(modifier) : NULL;
if (modifier && !m->modifier) {
free(m);
expr__ctx_free(m->pctx);
return NULL;
}
m->metric_expr = pe->metric_expr;
m->metric_unit = pe->unit;
m->pctx->runtime = runtime;
Expand All @@ -216,6 +226,7 @@ static void metric__free(struct metric *m)
{
free(m->metric_refs);
expr__ctx_free(m->pctx);
free((char *)m->modifier);
free(m);
}

Expand Down Expand Up @@ -645,7 +656,7 @@ static int decode_metric_id(struct strbuf *sb, const char *x)
return 0;
}

static int decode_all_metric_ids(struct evlist *perf_evlist)
static int decode_all_metric_ids(struct evlist *perf_evlist, const char *modifier)
{
struct evsel *ev;
struct strbuf sb = STRBUF_INIT;
Expand Down Expand Up @@ -675,10 +686,24 @@ static int decode_all_metric_ids(struct evlist *perf_evlist)
* give a more friendly display version.
*/
if (strstr(ev->name, "metric-id=")) {
bool has_slash = false;

free(ev->name);
for (cur = strchr(sb.buf, '@') ; cur; cur = strchr(++cur, '@'))
for (cur = strchr(sb.buf, '@') ; cur; cur = strchr(++cur, '@')) {
*cur = '/';
has_slash = true;
}

if (modifier) {
if (!has_slash && !strchr(sb.buf, ':')) {
ret = strbuf_addch(&sb, ':');
if (ret)
break;
}
ret = strbuf_addstr(&sb, modifier);
if (ret)
break;
}
ev->name = strdup(sb.buf);
if (!ev->name) {
ret = -ENOMEM;
Expand All @@ -692,6 +717,7 @@ static int decode_all_metric_ids(struct evlist *perf_evlist)

static int metricgroup__build_event_string(struct strbuf *events,
const struct expr_parse_ctx *ctx,
const char *modifier,
bool has_constraint)
{
struct hashmap_entry *cur;
Expand Down Expand Up @@ -765,6 +791,10 @@ static int metricgroup__build_event_string(struct strbuf *events,
ret = strbuf_addstr(events, sep + 1);
RETURN_IF_NON_ZERO(ret);
}
if (modifier) {
ret = strbuf_addstr(events, modifier);
RETURN_IF_NON_ZERO(ret);
}
}
if (has_duration) {
if (no_group) {
Expand Down Expand Up @@ -798,6 +828,7 @@ struct visited_metric {
struct metricgroup_add_iter_data {
struct list_head *metric_list;
const char *metric_name;
const char *modifier;
int *ret;
bool *has_match;
bool metric_no_group;
Expand All @@ -808,6 +839,7 @@ struct metricgroup_add_iter_data {

static int add_metric(struct list_head *metric_list,
const struct pmu_event *pe,
const char *modifier,
bool metric_no_group,
struct metric *root_metric,
const struct visited_metric *visited,
Expand All @@ -817,6 +849,7 @@ static int add_metric(struct list_head *metric_list,
* resolve_metric - Locate metrics within the root metric and recursively add
* references to them.
* @metric_list: The list the metric is added to.
* @modifier: if non-null event modifiers like "u".
* @metric_no_group: Should events written to events be grouped "{}" or
* global. Grouping is the default but due to multiplexing the
* user may override.
Expand All @@ -829,6 +862,7 @@ static int add_metric(struct list_head *metric_list,
* architecture perf is running upon.
*/
static int resolve_metric(struct list_head *metric_list,
const char *modifier,
bool metric_no_group,
struct metric *root_metric,
const struct visited_metric *visited,
Expand Down Expand Up @@ -876,7 +910,7 @@ static int resolve_metric(struct list_head *metric_list,
* context.
*/
for (i = 0; i < pending_cnt; i++) {
ret = add_metric(metric_list, pending[i].pe, metric_no_group,
ret = add_metric(metric_list, pending[i].pe, modifier, metric_no_group,
root_metric, visited, map);
if (ret)
break;
Expand All @@ -890,6 +924,7 @@ static int resolve_metric(struct list_head *metric_list,
* __add_metric - Add a metric to metric_list.
* @metric_list: The list the metric is added to.
* @pe: The pmu_event containing the metric to be added.
* @modifier: if non-null event modifiers like "u".
* @metric_no_group: Should events written to events be grouped "{}" or
* global. Grouping is the default but due to multiplexing the
* user may override.
Expand All @@ -904,6 +939,7 @@ static int resolve_metric(struct list_head *metric_list,
*/
static int __add_metric(struct list_head *metric_list,
const struct pmu_event *pe,
const char *modifier,
bool metric_no_group,
int runtime,
struct metric *root_metric,
Expand All @@ -930,7 +966,7 @@ static int __add_metric(struct list_head *metric_list,
* This metric is the root of a tree and may reference other
* metrics that are added recursively.
*/
root_metric = metric__new(pe, metric_no_group, runtime);
root_metric = metric__new(pe, modifier, metric_no_group, runtime);
if (!root_metric)
return -ENOMEM;

Expand Down Expand Up @@ -979,7 +1015,7 @@ static int __add_metric(struct list_head *metric_list,
ret = -EINVAL;
} else {
/* Resolve referenced metrics. */
ret = resolve_metric(metric_list, metric_no_group, root_metric,
ret = resolve_metric(metric_list, modifier, metric_no_group, root_metric,
&visited_node, map);
}

Expand Down Expand Up @@ -1021,6 +1057,7 @@ const struct pmu_event *metricgroup__find_metric(const char *metric,

static int add_metric(struct list_head *metric_list,
const struct pmu_event *pe,
const char *modifier,
bool metric_no_group,
struct metric *root_metric,
const struct visited_metric *visited,
Expand All @@ -1031,7 +1068,7 @@ static int add_metric(struct list_head *metric_list,
pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);

if (!strstr(pe->metric_expr, "?")) {
ret = __add_metric(metric_list, pe, metric_no_group, 0,
ret = __add_metric(metric_list, pe, modifier, metric_no_group, 0,
root_metric, visited, map);
} else {
int j, count;
Expand All @@ -1044,7 +1081,7 @@ static int add_metric(struct list_head *metric_list,
*/

for (j = 0; j < count && !ret; j++)
ret = __add_metric(metric_list, pe, metric_no_group, j,
ret = __add_metric(metric_list, pe, modifier, metric_no_group, j,
root_metric, visited, map);
}

Expand All @@ -1060,7 +1097,7 @@ static int metricgroup__add_metric_sys_event_iter(const struct pmu_event *pe,
if (!match_pe_metric(pe, d->metric_name))
return 0;

ret = add_metric(d->metric_list, pe, d->metric_no_group,
ret = add_metric(d->metric_list, pe, d->modifier, d->metric_no_group,
d->root_metric, d->visited, d->map);
if (ret)
goto out;
Expand All @@ -1086,14 +1123,16 @@ static int metric_list_cmp(void *priv __maybe_unused, const struct list_head *l,
* @metric_name: The name of the metric or metric group. For example, "IPC"
* could be the name of a metric and "TopDownL1" the name of a
* metric group.
* @modifier: if non-null event modifiers like "u".
* @metric_no_group: Should events written to events be grouped "{}" or
* global. Grouping is the default but due to multiplexing the
* user may override.
* @metric_list: The list that the metric or metric group are added to.
* @map: The map that is searched for metrics, most commonly the table for the
* architecture perf is running upon.
*/
static int metricgroup__add_metric(const char *metric_name, bool metric_no_group,
static int metricgroup__add_metric(const char *metric_name, const char *modifier,
bool metric_no_group,
struct list_head *metric_list,
const struct pmu_events_map *map)
{
Expand All @@ -1108,7 +1147,7 @@ static int metricgroup__add_metric(const char *metric_name, bool metric_no_group
*/
map_for_each_metric(pe, i, map, metric_name) {
has_match = true;
ret = add_metric(&list, pe, metric_no_group,
ret = add_metric(&list, pe, modifier, metric_no_group,
/*root_metric=*/NULL,
/*visited_metrics=*/NULL, map);
if (ret)
Expand All @@ -1121,6 +1160,7 @@ static int metricgroup__add_metric(const char *metric_name, bool metric_no_group
.data = (void *) &(struct metricgroup_add_iter_data) {
.metric_list = &list,
.metric_name = metric_name,
.modifier = modifier,
.metric_no_group = metric_no_group,
.has_match = &has_match,
.ret = &ret,
Expand Down Expand Up @@ -1160,26 +1200,31 @@ static int metricgroup__add_metric_list(const char *list, bool metric_no_group,
struct list_head *metric_list,
const struct pmu_events_map *map)
{
char *llist, *nlist, *p;
char *list_itr, *list_copy, *metric_name, *modifier;
int ret, count = 0;

nlist = strdup(list);
if (!nlist)
list_copy = strdup(list);
if (!list_copy)
return -ENOMEM;
llist = nlist;
list_itr = list_copy;

while ((p = strsep(&llist, ",")) != NULL) {
ret = metricgroup__add_metric(p, metric_no_group, metric_list,
while ((metric_name = strsep(&list_itr, ",")) != NULL) {
modifier = strchr(metric_name, ':');
if (modifier)
*modifier++ = '\0';

ret = metricgroup__add_metric(metric_name, modifier,
metric_no_group, metric_list,
map);
if (ret == -EINVAL)
pr_err("Cannot find metric or group `%s'\n", p);
pr_err("Cannot find metric or group `%s'\n", metric_name);

if (ret)
break;

count++;
}
free(nlist);
free(list_copy);

if (!ret) {
/*
Expand Down Expand Up @@ -1225,7 +1270,7 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
return -ENOMEM;

list_for_each_entry(m, metric_list, nd) {
if (m->has_constraint) {
if (m->has_constraint && !m->modifier) {
hashmap__for_each_entry(m->pctx->ids, cur, bkt) {
dup = strdup(cur->key);
if (!dup) {
Expand All @@ -1250,11 +1295,12 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
* evlist. The encoded metric_ids are decoded.
* @fake_pmu: used when testing metrics not supported by the current CPU.
* @ids: the event identifiers parsed from a metric.
* @modifier: any modifiers added to the events.
* @has_constraint: false if events should be placed in a weak group.
* @out_evlist: the created list of events.
*/
static int parse_ids(struct perf_pmu *fake_pmu, struct expr_parse_ctx *ids,
bool has_constraint, struct evlist **out_evlist)
const char *modifier, bool has_constraint, struct evlist **out_evlist)
{
struct parse_events_error parse_error;
struct evlist *parsed_evlist;
Expand All @@ -1277,7 +1323,8 @@ static int parse_ids(struct perf_pmu *fake_pmu, struct expr_parse_ctx *ids,

ids__insert(ids->ids, tmp);
}
ret = metricgroup__build_event_string(&events, ids, has_constraint);
ret = metricgroup__build_event_string(&events, ids, modifier,
has_constraint);
if (ret)
return ret;

Expand All @@ -1293,7 +1340,7 @@ static int parse_ids(struct perf_pmu *fake_pmu, struct expr_parse_ctx *ids,
parse_events_print_error(&parse_error, events.buf);
goto err_out;
}
ret = decode_all_metric_ids(parsed_evlist);
ret = decode_all_metric_ids(parsed_evlist, modifier);
if (ret)
goto err_out;

Expand Down Expand Up @@ -1333,7 +1380,8 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
ret = build_combined_expr_ctx(&metric_list, &combined);

if (!ret && combined && hashmap__size(combined->ids)) {
ret = parse_ids(fake_pmu, combined, /*has_constraint=*/true,
ret = parse_ids(fake_pmu, combined, /*modifier=*/NULL,
/*has_constraint=*/true,
&combined_evlist);
}
if (combined)
Expand Down Expand Up @@ -1364,6 +1412,12 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
if (n->evlist == NULL)
continue;

if ((!m->modifier && n->modifier) ||
(m->modifier && !n->modifier) ||
(m->modifier && n->modifier &&
strcmp(m->modifier, n->modifier)))
continue;

if (expr__subset_of_ids(n->pctx, m->pctx)) {
pr_debug("Events in '%s' fully contained within '%s'\n",
m->metric_name, n->metric_name);
Expand All @@ -1374,8 +1428,8 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
}
}
if (!metric_evlist) {
ret = parse_ids(fake_pmu, m->pctx, m->has_constraint,
&m->evlist);
ret = parse_ids(fake_pmu, m->pctx, m->modifier,
m->has_constraint, &m->evlist);
if (ret)
goto out;

Expand All @@ -1400,7 +1454,21 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
expr->metric_refs = m->metric_refs;
m->metric_refs = NULL;
expr->metric_expr = m->metric_expr;
expr->metric_name = m->metric_name;
if (m->modifier) {
char *tmp;

if (asprintf(&tmp, "%s:%s", m->metric_name, m->modifier) < 0)
expr->metric_name = NULL;
else
expr->metric_name = tmp;
} else
expr->metric_name = strdup(m->metric_name);

if (!expr->metric_name) {
ret = -ENOMEM;
free(metric_events);
goto out;
}
expr->metric_unit = m->metric_unit;
expr->metric_events = metric_events;
expr->runtime = m->pctx->runtime;
Expand Down Expand Up @@ -1500,7 +1568,10 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
return -ENOMEM;

new_expr->metric_expr = old_expr->metric_expr;
new_expr->metric_name = old_expr->metric_name;
new_expr->metric_name = strdup(old_expr->metric_name);
if (!new_expr->metric_name)
return -ENOMEM;

new_expr->metric_unit = old_expr->metric_unit;
new_expr->runtime = old_expr->runtime;

Expand Down

0 comments on commit b85a4d6

Please sign in to comment.