* [PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output
@ 2026-07-29 17:04 Chun-Tse Shao
2026-07-29 17:17 ` sashiko-bot
0 siblings, 1 reply; 3+ messages in thread
From: Chun-Tse Shao @ 2026-07-29 17:04 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, James Clark, linux-perf-users, linux-kernel,
Chun-Tse Shao
When running `perf stat` with `-A` (--no-aggr) and `--metric-only` in
JSON output mode (`-j`), `perf stat` evaluates metric expressions
across all matching PMUs (including uncore PMUs like `uncore_iio_0`,
`uncore_iio_1`, etc.).
However, `perf stat` previously formatted JSON output by printing only
"cpu" : "<id>" and grouping all metric values on a single line per CPU
without identifying which PMU instance evaluated each metric. As a
result, when an uncore event spans multiple PMU boxes, `perf stat`
printed repeated, ambiguous metric keys without PMU names.
Fix this by:
1. Including "pmu" : "<pmu_name>" in print_aggr_id_json when evsel->pmu
is a non-core or hybrid PMU in AGGR_NONE mode (-A).
2. Starting a new JSON metric line in AGGR_NONE mode (-A) whenever the
underlying PMU instance changes across PMU events.
3. Updating perf_json_output_lint.py to recognize the new "pmu" key in
the JSON test suite.
Before the fix:
$ perf stat -M iio_bandwidth_read -a -A --metric-only -j -I 1000
{"interval" : 1.001017947, "cpu" : "0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "22.5", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.2", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0", "MB/s iio_bandwidth_read" : "0.0"}
There is no way to determine which uncore device generated the metrics.
After:
$ perf stat -M iio_bandwidth_read -a -A --metric-only -j -I 1000
{"interval" : 1.000314908, "cpu" : "0", "pmu" : "uncore_iio_0", "MB/s iio_bandwidth_read" : "0.0"}
{"interval" : 1.000314908, "cpu" : "0", "pmu" : "uncore_iio_1", "MB/s iio_bandwidth_read" : "0.1"}
{"interval" : 1.000314908, "cpu" : "0", "pmu" : "uncore_iio_11", "MB/s iio_bandwidth_read" : "0.0"}
...
{"interval" : 1.000314908, "cpu" : "56", "pmu" : "uncore_iio_0", "MB/s iio_bandwidth_read" : "0.0"}
{"interval" : 1.000314908, "cpu" : "56", "pmu" : "uncore_iio_1", "MB/s iio_bandwidth_read" : "0.0"}
{"interval" : 1.000314908, "cpu" : "56", "pmu" : "uncore_iio_11", "MB/s iio_bandwidth_read" : "0.0"}
Signed-off-by: Chun-Tse Shao <ctshao@google.com>
Assisted-by: Gemini:gemini-3.1-pro-preview
Reviewed-by: Ian Rogers <irogers@google.com>
---
v3:
- Add before the fix output.
v2: lore.kernel.org/20260715203055.2981363-1-ctshao@google.com
- Fix print_metric_begin() initialization in print_no_aggr_metric()
to ensure tool events always open lines cleanly.
- Simplify and fix PMU line-split transition tracking when switching
between different PMU instances or transitioning from tool events.
- Add bounds check on aggr_map index in print_no_aggr_metric() to
prevent out-of-bounds array reads if sysfs CPU topology is incomplete.
v1: lore.kernel.org/20260714174052.1826692-1-ctshao@google.com
.../tests/shell/lib/perf_json_output_lint.py | 1 +
tools/perf/util/stat-display.c | 48 ++++++++++++++-----
2 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py
index dafbde56cc76..36767905fcdb 100644
--- a/tools/perf/tests/shell/lib/perf_json_output_lint.py
+++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py
@@ -64,6 +64,7 @@ def check_json_output(expected_items):
'metric-threshold': lambda x: x in ['unknown', 'good', 'less good', 'nearly bad', 'bad'],
'metricgroup': lambda x: True,
'node': lambda x: True,
+ 'pmu': lambda x: True,
'pcnt-running': lambda x: isfloat(x),
'socket': lambda x: True,
'thread': lambda x: True,
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index b337cc23f413..6785e2c442bd 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -397,6 +397,8 @@ static void print_aggr_id_json(struct perf_stat_config *config, struct outstate
json_out(os, "\"cpu\" : \"%d\"",
id.cpu.cpu);
}
+ if (evsel && evsel->pmu && (!evsel->pmu->is_core || evsel__is_hybrid(evsel)))
+ json_out(os, "\"pmu\" : \"%s\"", evsel->pmu->name);
break;
case AGGR_THREAD:
json_out(os, "\"thread\" : \"%s-%d\"",
@@ -1012,11 +1014,12 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
static void print_metric_begin(struct perf_stat_config *config,
struct evlist *evlist,
- struct outstate *os, int aggr_idx)
+ struct outstate *os, int aggr_idx,
+ struct evsel *counter)
{
struct perf_stat_aggr *aggr;
struct aggr_cpu_id id;
- struct evsel *evsel;
+ struct evsel *evsel = counter ?: evlist__first(evlist);
os->first = true;
if (!config->metric_only)
@@ -1031,7 +1034,6 @@ static void print_metric_begin(struct perf_stat_config *config,
else
fprintf(config->output, "%s", os->timestamp);
}
- evsel = evlist__first(evlist);
id = config->aggr_map->map[aggr_idx];
aggr = &evsel->stats->aggr[aggr_idx];
aggr_printout(config, os, evsel, id, aggr->nr);
@@ -1069,7 +1071,7 @@ static void print_aggr(struct perf_stat_config *config,
* Without each counter has its own line.
*/
cpu_aggr_map__for_each_idx(aggr_idx, config->aggr_map) {
- print_metric_begin(config, evlist, os, aggr_idx);
+ print_metric_begin(config, evlist, os, aggr_idx, NULL);
evlist__for_each_entry(evlist, counter) {
print_counter_aggrdata(config, counter, aggr_idx, os);
@@ -1095,7 +1097,7 @@ static void print_aggr_cgroup(struct perf_stat_config *config,
os->cgrp = evsel->cgrp;
cpu_aggr_map__for_each_idx(aggr_idx, config->aggr_map) {
- print_metric_begin(config, evlist, os, aggr_idx);
+ print_metric_begin(config, evlist, os, aggr_idx, NULL);
evlist__for_each_entry(evlist, counter) {
if (counter->cgrp != os->cgrp)
@@ -1131,7 +1133,8 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
perf_cpu_map__for_each_cpu(cpu, all_idx, evlist__core(evlist)->user_requested_cpus) {
struct evsel *counter;
- bool first = true;
+ struct perf_pmu *last_pmu = NULL;
+ bool line_open = false;
evlist__for_each_entry(evlist, counter) {
u64 ena, run, val;
@@ -1146,13 +1149,34 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
if (config->aggr_map->map[aggr_idx].cpu.cpu == cpu.cpu)
break;
}
+ if (aggr_idx >= config->aggr_map->nr)
+ continue;
os->evsel = counter;
os->id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
- if (first) {
- print_metric_begin(config, evlist, os, aggr_idx);
- first = false;
+
+ if (config->metric_only) {
+ struct perf_pmu *pmu = counter->pmu;
+
+ if (!evsel__is_tool(counter)) {
+ if (config->json_output && line_open &&
+ (!last_pmu || pmu != last_pmu)) {
+ print_metric_end(config, os);
+ line_open = false;
+ }
+ if (!line_open) {
+ print_metric_begin(config, evlist, os,
+ aggr_idx, counter);
+ line_open = true;
+ }
+ last_pmu = pmu;
+ } else if (!line_open) {
+ print_metric_begin(config, evlist, os,
+ aggr_idx, counter);
+ line_open = true;
+ }
}
+
val = ps->aggr[aggr_idx].counts.val;
ena = ps->aggr[aggr_idx].counts.ena;
run = ps->aggr[aggr_idx].counts.run;
@@ -1160,7 +1184,7 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
uval = val * counter->scale;
printout(config, os, uval, run, ena, 1.0, aggr_idx);
}
- if (!first)
+ if (line_open)
print_metric_end(config, os);
}
}
@@ -1523,7 +1547,7 @@ static void print_cgroup_counter(struct perf_stat_config *config, struct evlist
print_metric_end(config, os);
os->cgrp = counter->cgrp;
- print_metric_begin(config, evlist, os, /*aggr_idx=*/0);
+ print_metric_begin(config, evlist, os, /*aggr_idx=*/0, NULL);
}
print_counter(config, counter, os);
@@ -1573,7 +1597,7 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
} else if (config->cgroup_list) {
print_cgroup_counter(config, evlist, &os);
} else {
- print_metric_begin(config, evlist, &os, /*aggr_idx=*/0);
+ print_metric_begin(config, evlist, &os, /*aggr_idx=*/0, NULL);
evlist__for_each_entry(evlist, counter) {
print_counter(config, counter, &os);
}
--
2.55.0.508.g3f0d502094-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output
2026-07-29 17:04 [PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output Chun-Tse Shao
@ 2026-07-29 17:17 ` sashiko-bot
2026-07-31 23:14 ` Namhyung Kim
0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-07-29 17:17 UTC (permalink / raw)
To: Chun-Tse Shao; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Flawed line-split transition logic causes spurious empty JSON objects and incorrect PMU attribution for tool events.
--
--- Patch [1]: [PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output ---
commit 20e3cd08fd5e415b458343262ee76bc1f5f6ce85
Author: Chun-Tse Shao <ctshao@google.com>
perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output
This commit modifies perf stat JSON output in --metric-only and -A mode to
group metrics by PMU. It achieves this by adding a "pmu" key to the JSON
output and starting a new JSON line when the underlying PMU instance changes.
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index b337cc23f4131..6785e2c442bd7 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
[ ... ]
> @@ -1146,13 +1149,34 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
> if (config->aggr_map->map[aggr_idx].cpu.cpu == cpu.cpu)
> break;
> }
> + if (aggr_idx >= config->aggr_map->nr)
> + continue;
>
> os->evsel = counter;
> os->id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
> - if (first) {
> - print_metric_begin(config, evlist, os, aggr_idx);
> - first = false;
> +
> + if (config->metric_only) {
> + struct perf_pmu *pmu = counter->pmu;
> +
> + if (!evsel__is_tool(counter)) {
> + if (config->json_output && line_open &&
> + (!last_pmu || pmu != last_pmu)) {
[Severity: Medium]
Does this line-split transition logic handle tool events correctly depending
on their order in the event list?
If a tool event is processed first, line_open becomes true, but last_pmu
remains NULL. When the next event (a PMU event) is processed, the condition
(!last_pmu || pmu != last_pmu) evaluates to true. This prematurely closes
the line and creates a spurious empty JSON object for the tool event.
Conversely, if a tool event comes after a PMU event, the else-if block below
is taken because !evsel__is_tool(counter) evaluates to false. This bypasses
the line-splitting check entirely, causing the tool event to silently merge
into the preceding PMU's JSON object and incorrectly inherit its PMU
attribution.
> + print_metric_end(config, os);
> + line_open = false;
> + }
> + if (!line_open) {
> + print_metric_begin(config, evlist, os,
> + aggr_idx, counter);
> + line_open = true;
> + }
> + last_pmu = pmu;
> + } else if (!line_open) {
> + print_metric_begin(config, evlist, os,
> + aggr_idx, counter);
> + line_open = true;
> + }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260729170416.41904-1-ctshao@google.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output
2026-07-29 17:17 ` sashiko-bot
@ 2026-07-31 23:14 ` Namhyung Kim
0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2026-07-31 23:14 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Chun-Tse Shao, linux-perf-users
On Wed, Jul 29, 2026 at 05:17:41PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Flawed line-split transition logic causes spurious empty JSON objects and incorrect PMU attribution for tool events.
> --
>
> --- Patch [1]: [PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output ---
> commit 20e3cd08fd5e415b458343262ee76bc1f5f6ce85
> Author: Chun-Tse Shao <ctshao@google.com>
>
> perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output
>
> This commit modifies perf stat JSON output in --metric-only and -A mode to
> group metrics by PMU. It achieves this by adding a "pmu" key to the JSON
> output and starting a new JSON line when the underlying PMU instance changes.
>
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index b337cc23f4131..6785e2c442bd7 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
>
> [ ... ]
>
> > @@ -1146,13 +1149,34 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
> > if (config->aggr_map->map[aggr_idx].cpu.cpu == cpu.cpu)
> > break;
> > }
> > + if (aggr_idx >= config->aggr_map->nr)
> > + continue;
> >
> > os->evsel = counter;
> > os->id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
> > - if (first) {
> > - print_metric_begin(config, evlist, os, aggr_idx);
> > - first = false;
> > +
> > + if (config->metric_only) {
> > + struct perf_pmu *pmu = counter->pmu;
> > +
> > + if (!evsel__is_tool(counter)) {
> > + if (config->json_output && line_open &&
> > + (!last_pmu || pmu != last_pmu)) {
>
> [Severity: Medium]
> Does this line-split transition logic handle tool events correctly depending
> on their order in the event list?
>
> If a tool event is processed first, line_open becomes true, but last_pmu
> remains NULL. When the next event (a PMU event) is processed, the condition
> (!last_pmu || pmu != last_pmu) evaluates to true. This prematurely closes
> the line and creates a spurious empty JSON object for the tool event.
Something like this?
$ sudo ./perf stat -aA -e user_time -M insn_per_cycle -j --metric-only true
{"cpu" : "0", "pmu" : "tool"}
{"cpu" : "0", "pmu" : "cpu_core", "instructions insn_per_cycle" : "0.8"}
{"cpu" : "1", "pmu" : "tool"}
{"cpu" : "1", "pmu" : "cpu_core", "instructions insn_per_cycle" : "0.1"}
{"cpu" : "2", "pmu" : "tool"}
{"cpu" : "2", "pmu" : "cpu_core", "instructions insn_per_cycle" : "0.5"}
{"cpu" : "3", "pmu" : "tool"}
{"cpu" : "3", "pmu" : "cpu_core", "instructions insn_per_cycle" : "0.3"}
...
Thanks,
Namhyung
>
> Conversely, if a tool event comes after a PMU event, the else-if block below
> is taken because !evsel__is_tool(counter) evaluates to false. This bypasses
> the line-splitting check entirely, causing the tool event to silently merge
> into the preceding PMU's JSON object and incorrectly inherit its PMU
> attribution.
>
> > + print_metric_end(config, os);
> > + line_open = false;
> > + }
> > + if (!line_open) {
> > + print_metric_begin(config, evlist, os,
> > + aggr_idx, counter);
> > + line_open = true;
> > + }
> > + last_pmu = pmu;
> > + } else if (!line_open) {
> > + print_metric_begin(config, evlist, os,
> > + aggr_idx, counter);
> > + line_open = true;
> > + }
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260729170416.41904-1-ctshao@google.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-31 23:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 17:04 [PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output Chun-Tse Shao
2026-07-29 17:17 ` sashiko-bot
2026-07-31 23:14 ` Namhyung Kim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.