Re: [PATCH v7 42/59] perf powerpc-hcalls: Port powerpc-hcalls to use python module

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Accessing `sample.time` raises an `AttributeError`, causing the script to fail on the very first event.
- [High] Tracking in-flight hypervisor calls by process ID (`pid`) instead of `cpu` corrupts statistics for multi-threaded applications.
--

commit 2cb59bf83787181fd674eab1ce36cd9e9c0c966a
Author: Ian Rogers <irogers@google.com>

perf powerpc-hcalls: Port powerpc-hcalls to use python module

This commit ports the powerpc-hcalls script to use the perf.session
python module. It refactors the code to use an HCallAnalyzer class for
state encapsulation and tracks hcall entry and exit to calculate
statistics.

> diff --git a/tools/perf/python/powerpc-hcalls.py b/tools/perf/python/powerpc-hcalls.py
> new file mode 100755
> index 000000000000..c4fa539174c9
> --- /dev/null
> +++ b/tools/perf/python/powerpc-hcalls.py

[ ... ]

> +    def process_event(self, sample: perf.sample_event) -> None:
> +        """Process a single sample event."""
> +        name = str(sample.evsel)
> +        pid = sample.sample_pid
> +        time = sample.time

In the perf Python C extension, the timestamp attribute of a sample event
is exposed as sample_time, not time. Does accessing sample.time raise an
AttributeError here, causing the script to fail on the first event?

> +        opcode = getattr(sample, "opcode", -1)
> +
> +        if opcode == -1:
> +            return
> +
> +        if name == "evsel(powerpc:hcall_entry)":
> +            self.d_enter[pid] = (opcode, time)

Does tracking in-flight hypervisor calls by process ID (pid) corrupt
statistics for multi-threaded applications?

If multiple threads share the same PID and make concurrent hypervisor calls
on different CPUs, the second thread's entry will overwrite the first
thread's entry in self.d_enter[pid].

Since a single CPU can only execute one synchronous hypervisor call at a
time, would it be better to use sample.sample_cpu or sample.sample_tid as
the tracking key instead?

> +        elif name == "evsel(powerpc:hcall_exit)":
> +            if pid in self.d_enter:
> +                opcode_entry, time_entry = self.d_enter[pid]
> +                if opcode_entry == opcode:
> +                    diff = time - time_entry
> +                    del self.d_enter[pid]

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