[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230219092848.639226-50-irogers@google.com>
Date: Sun, 19 Feb 2023 01:28:46 -0800
From: Ian Rogers <irogers@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Zhengjun Xing <zhengjun.xing@...ux.intel.com>,
Sandipan Das <sandipan.das@....com>,
James Clark <james.clark@....com>,
Kajol Jain <kjain@...ux.ibm.com>,
John Garry <john.g.garry@...cle.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Andrii Nakryiko <andrii@...nel.org>,
Eduard Zingerman <eddyz87@...il.com>,
Suzuki Poulouse <suzuki.poulose@....com>,
Leo Yan <leo.yan@...aro.org>,
Florian Fischer <florian.fischer@...q.space>,
Ravi Bangoria <ravi.bangoria@....com>,
Jing Zhang <renyu.zj@...ux.alibaba.com>,
Sean Christopherson <seanjc@...gle.com>,
Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org,
Perry Taylor <perry.taylor@...el.com>,
Caleb Biggers <caleb.biggers@...el.com>
Cc: Stephane Eranian <eranian@...gle.com>,
Ian Rogers <irogers@...gle.com>
Subject: [PATCH v1 49/51] perf metric: Directly use counts rather than saved_value
Bugs with double aggregation have been introduced because of
aggregation of counters and again with saved_value. Remove the generic
metric use-case. Update parse-metric and pmu-events tests to update
aggregate rather than saved_value counts.
Signed-off-by: Ian Rogers <irogers@...gle.com>
---
tools/perf/tests/parse-metric.c | 4 +--
tools/perf/tests/pmu-events.c | 4 +--
tools/perf/util/stat-shadow.c | 56 +++++++++++----------------------
3 files changed, 23 insertions(+), 41 deletions(-)
diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
index 37e3371d978e..b9b8a48289c4 100644
--- a/tools/perf/tests/parse-metric.c
+++ b/tools/perf/tests/parse-metric.c
@@ -35,10 +35,10 @@ static void load_runtime_stat(struct evlist *evlist, struct value *vals)
struct evsel *evsel;
u64 count;
- perf_stat__reset_shadow_stats();
+ evlist__alloc_aggr_stats(evlist, 1);
evlist__for_each_entry(evlist, evsel) {
count = find_value(evsel->name, vals);
- perf_stat__update_shadow_stats(evsel, count, 0);
+ evsel->stats->aggr->counts.val = count;
if (!strcmp(evsel->name, "duration_time"))
update_stats(&walltime_nsecs_stats, count);
}
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 122e74c282a7..4ec2a4ca1a82 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -863,9 +863,9 @@ static int test__parsing_callback(const struct pmu_metric *pm,
* zero when subtracted and so try to make them unique.
*/
k = 1;
- perf_stat__reset_shadow_stats();
+ evlist__alloc_aggr_stats(evlist, 1);
evlist__for_each_entry(evlist, evsel) {
- perf_stat__update_shadow_stats(evsel, k, 0);
+ evsel->stats->aggr->counts.val = k;
if (!strcmp(evsel->name, "duration_time"))
update_stats(&walltime_nsecs_stats, k);
k++;
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 7b48e2bd3ba1..eba98520cea2 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -234,7 +234,6 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count,
int aggr_idx)
{
u64 count_ns = count;
- struct saved_value *v;
struct runtime_stat_data rsd = {
.ctx = evsel_context(counter),
.cgrp = counter->cgrp,
@@ -265,19 +264,6 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count,
update_runtime_stat(STAT_DTLB_CACHE, aggr_idx, count, &rsd);
else if (evsel__match(counter, HW_CACHE, HW_CACHE_ITLB))
update_runtime_stat(STAT_ITLB_CACHE, aggr_idx, count, &rsd);
-
- if (counter->collect_stat) {
- v = saved_value_lookup(counter, aggr_idx, true, STAT_NONE, 0,
- rsd.cgrp);
- update_stats(&v->stats, count);
- if (counter->metric_leader)
- v->metric_total += count;
- } else if (counter->metric_leader && !counter->merged_stat) {
- v = saved_value_lookup(counter->metric_leader,
- aggr_idx, true, STAT_NONE, 0, rsd.cgrp);
- v->metric_total += count;
- v->metric_other++;
- }
}
/* used for get_ratio_color() */
@@ -480,18 +466,17 @@ static int prepare_metric(struct evsel **metric_events,
struct expr_parse_ctx *pctx,
int aggr_idx)
{
- double scale;
- char *n;
- int i, j, ret;
+ int i;
for (i = 0; metric_events[i]; i++) {
- struct saved_value *v;
- struct stats *stats;
- u64 metric_total = 0;
- int source_count;
+ char *n;
+ double val;
+ int source_count = 0;
if (evsel__is_tool(metric_events[i])) {
- source_count = 1;
+ struct stats *stats;
+ double scale;
+
switch (metric_events[i]->tool_event) {
case PERF_TOOL_DURATION_TIME:
stats = &walltime_nsecs_stats;
@@ -515,35 +500,32 @@ static int prepare_metric(struct evsel **metric_events,
pr_err("Unknown tool event '%s'", evsel__name(metric_events[i]));
abort();
}
+ val = avg_stats(stats) * scale;
+ source_count = 1;
} else {
- v = saved_value_lookup(metric_events[i], aggr_idx, false,
- STAT_NONE, 0,
- metric_events[i]->cgrp);
- if (!v)
+ struct perf_stat_evsel *ps = metric_events[i]->stats;
+ struct perf_stat_aggr *aggr = &ps->aggr[aggr_idx];
+
+ if (!aggr)
break;
- stats = &v->stats;
+
/*
* If an event was scaled during stat gathering, reverse
* the scale before computing the metric.
*/
- scale = 1.0 / metric_events[i]->scale;
-
+ val = aggr->counts.val * (1.0 / metric_events[i]->scale);
source_count = evsel__source_count(metric_events[i]);
-
- if (v->metric_other)
- metric_total = v->metric_total * scale;
}
n = strdup(evsel__metric_id(metric_events[i]));
if (!n)
return -ENOMEM;
- expr__add_id_val_source_count(pctx, n,
- metric_total ? : avg_stats(stats) * scale,
- source_count);
+ expr__add_id_val_source_count(pctx, n, val, source_count);
}
- for (j = 0; metric_refs && metric_refs[j].metric_name; j++) {
- ret = expr__add_ref(pctx, &metric_refs[j]);
+ for (int j = 0; metric_refs && metric_refs[j].metric_name; j++) {
+ int ret = expr__add_ref(pctx, &metric_refs[j]);
+
if (ret)
return ret;
}
--
2.39.2.637.g21b0678d19-goog
Powered by blists - more mailing lists