[PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output

[PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output

[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

Re: [PATCH v3] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output

[sashiko-bot]

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