Re: [PATCH v6 28/59] perf syscall-counts-by-pid: Port syscall-counts-by-pid to use python module

sashiko-bot
2026-04-25T18:03:58Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Output is sorted by syscall ID instead of event count, contradicting the commit message.
- [Medium] Filtering by PID 0 fails because the filter value is evaluated as falsy.
--

commit 4c85b8ae5e1c60397dc5980ed736963359e2b677
Author: Ian Rogers <irogers@google.com>

perf syscall-counts-by-pid: Port syscall-counts-by-pid to use python module

This commit rewrites the syscall-counts-by-pid.py script to use the perf
python module. This avoids the overhead of executing the perf script tool
and improves performance.

> diff --git a/tools/perf/python/syscall-counts-by-pid.py b/tools/perf/python/syscall-counts-by-pid.py
> new file mode 100755
> index 000000000000..ff962334a143
> --- /dev/null
> +++ b/tools/perf/python/syscall-counts-by-pid.py
> @@ -0,0 +1,88 @@
[ ... ]
> +def print_syscall_totals():
> + """Print aggregated statistics."""
[ ... ]
> + print(f"{'comm [pid]/syscalls':<40} {'count':>10}")
> + print("---------------------------------------- -----------")
> +
> + sorted_keys = sorted(syscalls.keys(), key=lambda k: (k[0], k[1], k[2]))

Does this sort the output by syscall ID instead of the event count?

The commit message demonstrates the expected output being sorted in descending
order by event count, but this seems to sort in ascending order by syscall ID.

> + current_comm_pid = None
> + for comm, pid, sc_id in sorted_keys:
[ ... ]
> +def process_event(sample):
> + """Process a single sample event."""
[ ... ]
> + pid = sample.sample_pid
> +
> + if for_pid and pid != for_pid:

Can this fail to filter correctly if the user filters by PID 0?

Since for_pid would be 0, it evaluates as falsy in Python, which would bypass
this early return and process syscalls for all processes. Should this check
for_pid is not None instead?

> + return
> +
> + comm = "unknown"
> + try:
> + if session:
> + proc = session.find_thread(pid)
> + if proc:
> + comm = proc.comm()
> + except (TypeError, AttributeError):
> + pass
> +
> + if for_comm and comm != for_comm:

Similarly, if an empty string is provided for for_comm, does it evaluate as
falsy and bypass the filter?

> + return
> + syscalls[(comm, pid, sc_id)] += 1

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