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

sashiko-bot
2026-04-20T00:34:25Z

Sashiko AI review found 5 potential issue(s):
- [High] Unhandled `TypeError` when looking up untracked PIDs crashes the script.
- [High] Python f-string formatting crashes with a `TypeError` for unknown syscall IDs.
- [High] The script ignores input file arguments and hardcodes 'perf.data', breaking `-i`/`--input` support.
- [Medium] Feature regression: Removed the ability to filter syscall counts by Process ID.
- [Low] Unused variable `id_keys` in the summary printing loop.
--

commit 4158f24426deccec5a61785f2dd5ad7903a4ad84
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 script to use the perf Python
module, significantly improving execution performance. It replaces the legacy
event processing and filtering logic.

> 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 0000000000000..a5d91060b18d6
> --- /dev/null
> +++ b/tools/perf/python/syscall-counts-by-pid.py
> @@ -0,0 +1,57 @@

[ ... ]

> +def print_syscall_totals():

[ ... ]

> + comm_keys = syscalls.keys()
> + for comm in comm_keys:
> + pid_keys = syscalls[comm].keys()
> + for pid in pid_keys:
> + print(f"\n{comm} [{pid}]")
> + id_keys = syscalls[comm][pid].keys()

This isn't a bug, but is the id_keys variable needed? It appears to be
unused.

> + for id, val in sorted(syscalls[comm][pid].items(),
> + key=lambda kv: (kv[1], kv[0]), reverse=True):
> + print(f" {perf.syscall_name(id):<38} {val:>10}")

Will this crash with a TypeError if an unknown syscall ID is encountered?

If perf.syscall_name(id) returns None for an unmapped syscall, passing None
to the string format specifier (:<38) will raise a TypeError.

[ ... ]

> +def process_event(sample):
> + event_name = str(sample.evsel)

[ ... ]

> + pid = sample.sample_pid
> + comm = session.process(pid).comm()

Is a try...except block needed here to handle untracked PIDs?

If the underlying C extension fails to find the thread for the PID (which
can happen for short-lived processes or dropped events), it raises a Python
TypeError.

Without handling this exception, encountering a single untracked PID could
abruptly terminate the entire analysis session.

[ ... ]

> +if __name__ == "__main__":
> + ap = argparse.ArgumentParser()
> + ap.add_argument("comm", nargs="?", help="Only report syscalls for comm")

Does this remove the ability to filter syscall counts by Process ID?

The original legacy script allowed users to filter by Process ID by checking
if the argument was parseable as an integer.

With this change, passing a PID will interpret the input as a literal string
command name, silently failing to filter.

> + args = ap.parse_args()
> + for_comm = args.comm
> + session = perf.session(perf.data("perf.data"), sample=process_event)

Does hardcoding perf.data break the ability to use custom input files?

Because the script's argparse configuration does not accept an -i override,
it appears it will completely ignore custom input files passed via perf
script (e.g., perf script -i mytrace.data) and always process perf.data
from the current directory.

> + session.process_events()
> + print_syscall_totals()

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