Re: [PATCH v2 25/58] perf stat-cpi: Port stat-cpi to use python module

sashiko-bot
2026-04-23T04:18:24Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Double subtraction of counter values in live mode leads to mathematically meaningless CPI metrics.
- [Medium] Missing `evsel.read_format` configuration silently disables event multiplexing scaling in live mode.
--

commit 382086625819b9250d682672f3e663444a565126
Author: Ian Rogers <irogers@google.com>

perf stat-cpi: Port stat-cpi to use python module

Port stat-cpi.py from the legacy framework to a standalone script. Support
both file processing mode and live mode. Calculate and display CPI per
interval per CPU/thread.

> diff --git a/tools/perf/python/stat-cpi.py b/tools/perf/python/stat-cpi.py
> new file mode 100755
> index 0000000000000..4b1f1f69c94a6
> --- /dev/null
> +++ b/tools/perf/python/stat-cpi.py
> @@ -0,0 +1,151 @@
[ ... ]
> + def store(self, event: str, cpu: int, thread: int, counts: tuple[int, int, int]) -> None:
> + """Store counter values, computing difference from previous absolute values."""
> + self.store_key(cpu, thread)
> + key = self.get_key(event, cpu, thread)
> +
> + val, ena, run = counts
> + if key in self.prev_data:
> + prev_val, prev_ena, prev_run = self.prev_data[key]
> + cur_val = val - prev_val
> + cur_ena = ena - prev_ena
> + cur_run = run - prev_run

In live mode, evsel.read() relies on the C extension function which already
computes and returns the count delta since the last read.

Does subtracting the previous interval's values again here calculate the
acceleration of the counts instead of the velocity, producing incorrect
metrics?

[ ... ]
> + def run_live(self) -> None:
> + """Read counters live."""
> + evlist = perf.parse_events("cycles,instructions")
> + if not evlist:
> + print("Failed to parse events", file=sys.stderr)
> + return
> + try:
> + evlist.open()

perf.parse_events() generates an evlist with default attributes, which do
not include perf.FORMAT_TOTAL_TIME_ENABLED and perf.FORMAT_TOTAL_TIME_RUNNING
flags.

If these flags are not added to evsel.read_format before calling
evlist.open(), will the kernel omit timing data and leave the ena and run
fields at 0?

Since the script relies on run > 0 to apply scaling:

def get(self, event: str, cpu: int, thread: int) -> float:
...
if run > 0:
return val * (ena / float(run))

Could this bypass scaling logic and produce incorrect CPI calculations if
hardware PMU counters are overcommitted?

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260423035526.1537178-1-irogers@google.com?part=25