lkml.org 
[lkml]   [2021]   [Oct]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    Subject[PATCH v2 21/21] perf metric: Allow modifiers on metrics.
    From
    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.

    Acked-by: Andi Kleen <ak@linux.intel.com>
    Signed-off-by: Ian Rogers <irogers@google.com>
    ---
    tools/perf/util/metricgroup.c | 125 ++++++++++++++++++++++++++--------
    1 file changed, 98 insertions(+), 27 deletions(-)

    diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
    index b6ba1317973e..c34a3683e719 100644
    --- a/tools/perf/util/metricgroup.c
    +++ b/tools/perf/util/metricgroup.c
    @@ -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);
    @@ -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;
    /**
    @@ -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)
    {
    @@ -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;
    @@ -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);
    }

    @@ -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;
    @@ -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;
    @@ -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;
    @@ -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) {
    @@ -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;
    @@ -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,
    @@ -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.
    @@ -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,
    @@ -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;
    @@ -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.
    @@ -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,
    @@ -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;

    @@ -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);
    }

    @@ -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,
    @@ -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;
    @@ -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);
    }

    @@ -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;
    @@ -1086,6 +1123,7 @@ 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.
    @@ -1093,7 +1131,8 @@ static int metric_list_cmp(void *priv __maybe_unused, const struct list_head *l,
    * @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)
    {
    @@ -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)
    @@ -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,
    @@ -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) {
    /*
    @@ -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) {
    @@ -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;
    @@ -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;

    @@ -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;

    @@ -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)
    @@ -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);
    @@ -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;

    @@ -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;
    @@ -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;

    --
    2.33.0.1079.g6e70778dc9-goog
    \
     
     \ /
      Last update: 2021-10-15 19:37    [W:2.502 / U:0.760 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site