Re: [PATCH v5] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output
flat view
From: sashiko-bot@kernel.org
Date: 2026-08-11 23:18:18
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Bypassing line initialization and termination for tool events in '--metric-only' mode corrupts JSON, CSV, and standard formatting when those events evaluate to metrics. --
--- Patch [1]: [PATCH v5] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output ---commit f2dfadb52a27c2891ce164d2b06f57924949dd1d Author: Chun-Tse Shao [off-list ref] perf stat: Include PMU name and split uncore events per PMU in metric-only JSON output This commit updates perf stat JSON output to include the PMU name for uncore events in metric-only mode and split output lines when transitioning between PMUs. It bypasses line initialization for tool events to avoid creating spurious empty JSON objects.
quoted hunk ↗ jump to hunk
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c --- a/tools/perf/util/stat-display.c +++ b/tools/perf/util/stat-display.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1146,12 +1151,42 @@ 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; + bool is_tool = evsel__is_tool(counter); + + if (!is_tool && pmu) {
[Severity: Medium]
Does bypassing line initialization for tool events corrupt the output
formatting if the tool event evaluates to a metric?
If a tool event has an associated metric (e.g., from a custom metric using
duration_time), it will fail the !is_tool check and skip calling
print_metric_begin().
The event still proceeds to printout() down below:
uval = val * counter->scale;
printout(config, os, uval, run, ena, 1.0, aggr_idx);
If printout() outputs the metric directly, won't this lack the necessary JSON
or CSV preamble (such as { "interval"... ) because print_metric_begin() was
skipped?
Furthermore, since line_open remains false, print_metric_end() is also
bypassed at the end of the loop, leaving the JSON object unclosed.
quoted hunk ↗ jump to hunk
+ bool pmu_changed = false; + + if (line_open && last_pmu) { + bool non_core = !pmu->is_core || + evsel__is_hybrid(counter) || + !last_pmu->is_core || + evsel__is_hybrid(last_evsel);
[ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260811230518.3360-2-ctshao@google.com?part=1