mirrors/linux-stable
1
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git synced 2024-09-26 20:38:12 +00:00
Code Issues Releases Wiki Activity

perf metric: Reduce multiplexing with duration_time

Browse source 
It is common to use the same counters with and without duration_time.
The ID sharing code treats duration_time as if it were a hardware event
placed in the same group. This causes unnecessary multiplexing such as
in the following example where l3_cache_access isn't shared:

  $ perf stat -M l3 -a sleep 1

   Performance counter stats for 'system wide':

         3,117,007      l3_cache_miss         #    199.5 MB/s  l3_rd_bw
                                              #     43.6 %  l3_hits
                                              #     56.4 %  l3_miss                 (50.00%)
         5,526,447      l3_cache_access                                             (50.00%)
         5,392,435      l3_cache_access       # 5389191.2 access/s  l3_access_rate  (50.00%)
     1,000,601,901 ns   duration_time

       1.000601901 seconds time elapsed

Fix this by placing duration_time in all groups unless metric
sharing has been disabled on the command line:

  $ perf stat -M l3 -a sleep 1

   Performance counter stats for 'system wide':

         3,597,972      l3_cache_miss         #    230.3 MB/s  l3_rd_bw
                                              #     48.0 %  l3_hits
                                              #     52.0 %  l3_miss
         6,914,459      l3_cache_access       # 6909935.9 access/s  l3_access_rate
     1,000,654,579 ns   duration_time

       1.000654579 seconds time elapsed

  $ perf stat --metric-no-merge -M l3 -a sleep 1

   Performance counter stats for 'system wide':

         3,501,834      l3_cache_miss         #     53.5 %  l3_miss                (24.99%)
         6,548,173      l3_cache_access                                            (24.99%)
         3,417,622      l3_cache_miss         #     45.7 %  l3_hits                (25.04%)
         6,294,062      l3_cache_access                                            (25.04%)
         5,923,238      l3_cache_access       # 5919688.1 access/s  l3_access_rate (24.99%)
     1,000,599,683 ns   duration_time
         3,607,486      l3_cache_miss         #    230.9 MB/s  l3_rd_bw            (49.97%)

       1.000599683 seconds time elapsed

v2. Doesn't count duration_time in the metric_list_cmp function that
    sorts larger metrics first. Without this a metric with duration_time
    and an event is sorted the same as a metric with two events,
    possibly not allowing the first metric to share with the second.

Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: John Garry <john.garry@huawei.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.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: Stephane Eranian <eranian@google.com>
Link: http://lore.kernel.org/lkml/20211124015226.3317994-1-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This commit is contained in:
Ian Rogers 2021-11-23 17:52:26 -08:00 committed by Arnaldo Carvalho de Melo
parent b4515ad6e1
commit 6b6b16b3bb
1 changed files with 33 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

42
tools/perf/util/metricgroup.c
View file

@ -1115,13 +1115,27 @@ static int metricgroup__add_metric_sys_event_iter(const struct pmu_event *pe,
return ret;
}
/**
* metric_list_cmp - list_sort comparator that sorts metrics with more events to
* the front. duration_time is excluded from the count.
*/
static int metric_list_cmp(void *priv __maybe_unused, const struct list_head *l,
const struct list_head *r)
{
const struct metric *left = container_of(l, struct metric, nd);
const struct metric *right = container_of(r, struct metric, nd);
struct expr_id_data *data;
int left_count, right_count;
return hashmap__size(right->pctx->ids) - hashmap__size(left->pctx->ids);
left_count = hashmap__size(left->pctx->ids);
if (!expr__get_id(left->pctx, "duration_time", &data))
left_count--;
right_count = hashmap__size(right->pctx->ids);
if (!expr__get_id(right->pctx, "duration_time", &data))
right_count--;
return right_count - left_count;
}
/**
@ -1299,14 +1313,16 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
/**
* parse_ids - Build the event string for the ids and parse them creating an
* evlist. The encoded metric_ids are decoded.
* @metric_no_merge: is metric sharing explicitly disabled.
* @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,
const char *modifier, bool has_constraint, struct evlist **out_evlist)
static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu,
struct expr_parse_ctx *ids, const char *modifier,
bool has_constraint, struct evlist **out_evlist)
{
struct parse_events_error parse_error;
struct evlist *parsed_evlist;
@ -1314,12 +1330,19 @@ static int parse_ids(struct perf_pmu *fake_pmu, struct expr_parse_ctx *ids,
int ret;
*out_evlist = NULL;
if (hashmap__size(ids->ids) == 0) {
if (!metric_no_merge || hashmap__size(ids->ids) == 0) {
char *tmp;
/*
* No ids/events in the expression parsing context. Events may
* have been removed because of constant evaluation, e.g.:
* event1 if #smt_on else 0
* We may fail to share events between metrics because
* duration_time isn't present in one metric. For example, a
* ratio of cache misses doesn't need duration_time but the same
* events may be used for a misses per second. Events without
* sharing implies multiplexing, that is best avoided, so place
* duration_time in every group.
*
* Also, there may be no ids/events in the expression parsing
* context because of constant evaluation, e.g.:
* event1 if #smt_on else 0
* Add a duration_time event to avoid a parse error on an empty
* string.
*/
@ -1387,7 +1410,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, /*modifier=*/NULL,
ret = parse_ids(metric_no_merge, fake_pmu, combined,
/*modifier=*/NULL,
/*has_constraint=*/true,
&combined_evlist);
}
@ -1435,7 +1459,7 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
}
}
if (!metric_evlist) {
ret = parse_ids(fake_pmu, m->pctx, m->modifier,
ret = parse_ids(metric_no_merge, fake_pmu, m->pctx, m->modifier,
m->has_constraint, &m->evlist);
if (ret)
goto out;