Commit 37cc8ad7 authored by Ian Rogers's avatar Ian Rogers Committed by Arnaldo Carvalho de Melo

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: default avatarIan Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Caleb Biggers <caleb.biggers@intel.com>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Florian Fischer <florian.fischer@muhq.space>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jing Zhang <renyu.zj@linux.alibaba.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Perry Taylor <perry.taylor@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Sandipan Das <sandipan.das@amd.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: Xing Zhengjun <zhengjun.xing@linux.intel.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Link: https://lore.kernel.org/r/20230219092848.639226-50-irogers@google.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent 8945bef3
...@@ -35,10 +35,10 @@ static void load_runtime_stat(struct evlist *evlist, struct value *vals) ...@@ -35,10 +35,10 @@ static void load_runtime_stat(struct evlist *evlist, struct value *vals)
struct evsel *evsel; struct evsel *evsel;
u64 count; u64 count;
perf_stat__reset_shadow_stats(); evlist__alloc_aggr_stats(evlist, 1);
evlist__for_each_entry(evlist, evsel) { evlist__for_each_entry(evlist, evsel) {
count = find_value(evsel->name, vals); 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")) if (!strcmp(evsel->name, "duration_time"))
update_stats(&walltime_nsecs_stats, count); update_stats(&walltime_nsecs_stats, count);
} }
......
...@@ -863,9 +863,9 @@ static int test__parsing_callback(const struct pmu_metric *pm, ...@@ -863,9 +863,9 @@ static int test__parsing_callback(const struct pmu_metric *pm,
* zero when subtracted and so try to make them unique. * zero when subtracted and so try to make them unique.
*/ */
k = 1; k = 1;
perf_stat__reset_shadow_stats(); evlist__alloc_aggr_stats(evlist, 1);
evlist__for_each_entry(evlist, evsel) { 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")) if (!strcmp(evsel->name, "duration_time"))
update_stats(&walltime_nsecs_stats, k); update_stats(&walltime_nsecs_stats, k);
k++; k++;
......
...@@ -234,7 +234,6 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count, ...@@ -234,7 +234,6 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count,
int aggr_idx) int aggr_idx)
{ {
u64 count_ns = count; u64 count_ns = count;
struct saved_value *v;
struct runtime_stat_data rsd = { struct runtime_stat_data rsd = {
.ctx = evsel_context(counter), .ctx = evsel_context(counter),
.cgrp = counter->cgrp, .cgrp = counter->cgrp,
...@@ -265,19 +264,6 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count, ...@@ -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); update_runtime_stat(STAT_DTLB_CACHE, aggr_idx, count, &rsd);
else if (evsel__match(counter, HW_CACHE, HW_CACHE_ITLB)) else if (evsel__match(counter, HW_CACHE, HW_CACHE_ITLB))
update_runtime_stat(STAT_ITLB_CACHE, aggr_idx, count, &rsd); 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() */ /* used for get_ratio_color() */
...@@ -480,18 +466,17 @@ static int prepare_metric(struct evsel **metric_events, ...@@ -480,18 +466,17 @@ static int prepare_metric(struct evsel **metric_events,
struct expr_parse_ctx *pctx, struct expr_parse_ctx *pctx,
int aggr_idx) int aggr_idx)
{ {
double scale; int i;
char *n;
int i, j, ret;
for (i = 0; metric_events[i]; i++) { for (i = 0; metric_events[i]; i++) {
struct saved_value *v; char *n;
struct stats *stats; double val;
u64 metric_total = 0; int source_count = 0;
int source_count;
if (evsel__is_tool(metric_events[i])) { if (evsel__is_tool(metric_events[i])) {
source_count = 1; struct stats *stats;
double scale;
switch (metric_events[i]->tool_event) { switch (metric_events[i]->tool_event) {
case PERF_TOOL_DURATION_TIME: case PERF_TOOL_DURATION_TIME:
stats = &walltime_nsecs_stats; stats = &walltime_nsecs_stats;
...@@ -515,35 +500,32 @@ static int prepare_metric(struct evsel **metric_events, ...@@ -515,35 +500,32 @@ static int prepare_metric(struct evsel **metric_events,
pr_err("Unknown tool event '%s'", evsel__name(metric_events[i])); pr_err("Unknown tool event '%s'", evsel__name(metric_events[i]));
abort(); abort();
} }
val = avg_stats(stats) * scale;
source_count = 1;
} else { } else {
v = saved_value_lookup(metric_events[i], aggr_idx, false, struct perf_stat_evsel *ps = metric_events[i]->stats;
STAT_NONE, 0, struct perf_stat_aggr *aggr = &ps->aggr[aggr_idx];
metric_events[i]->cgrp);
if (!v) if (!aggr)
break; break;
stats = &v->stats;
/* /*
* If an event was scaled during stat gathering, reverse * If an event was scaled during stat gathering, reverse
* the scale before computing the metric. * 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]); 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])); n = strdup(evsel__metric_id(metric_events[i]));
if (!n) if (!n)
return -ENOMEM; return -ENOMEM;
expr__add_id_val_source_count(pctx, n, expr__add_id_val_source_count(pctx, n, val, source_count);
metric_total ? : avg_stats(stats) * scale,
source_count);
} }
for (j = 0; metric_refs && metric_refs[j].metric_name; j++) { for (int j = 0; metric_refs && metric_refs[j].metric_name; j++) {
ret = expr__add_ref(pctx, &metric_refs[j]); int ret = expr__add_ref(pctx, &metric_refs[j]);
if (ret) if (ret)
return ret; return ret;
} }
......
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