Re: [PATCH v7 38/59] perf failed-syscalls-by-pid: Port failed-syscalls-by-pid to use python module

sashiko-bot
2026-04-25T23:06:55Z

Sashiko AI review found 3 potential issue(s):
- [High] The script aborts event processing prematurely if `perf.session.find_thread()` fails to find a thread.
- [High] The script crashes with a TypeError during summary sorting if a thread's command string is missing.
- [Medium] The script fails to group errors by system call and loses frequency-based sorting, directly contradicting the commit message.
--

commit 0c01206d74ebfd6bae49fe56d591c99446342a80
Author: Ian Rogers <irogers@google.com>

perf failed-syscalls-by-pid: Port failed-syscalls-by-pid to use python module

This commit ports the failed-syscalls-by-pid script to use the new perf Python
module API. It updates syscall name resolution, error code strings, and adds
filtering by COMM or PID.

> - Used standard collections.defaultdict for nested statistics
> aggregation.

Does the implementation actually use nested statistics aggregation as described
in the commit message?

Looking at the initialization of self.syscalls, it is defined as a flat
dictionary with a 4-tuple key: (comm, pid, syscall_id, ret).

Linear iteration over these flattened keys prints redundant syscall headers for
each distinct error code of a single syscall, instead of grouping them. This
flat structure also breaks the frequency-based sorting of error codes by count.

> diff --git a/tools/perf/python/failed-syscalls-by-pid.py b/tools/perf/python/failed-syscalls-by-pid.py
> new file mode 100755
> index 000000000000..eecd553cbf8f
> --- /dev/null
> +++ b/tools/perf/python/failed-syscalls-by-pid.py
[ ... ]
> + def process_event(self, sample: perf.sample_event) -> None:
> + """Process a single sample event."""
> + event_name = str(sample.evsel)
> + if "sys_exit" not in event_name:
> + return
> +
> + pid = sample.sample_pid
> + if hasattr(self, 'session') and self.session:
> + comm = self.session.find_thread(pid).comm()

Can this cause a regression where the event loop aborts prematurely?

If find_thread() fails to find a thread, the underlying C extension raises a
TypeError.

Without a try-except block here, this exception propagates back to the C caller
and causes process_events() to abort the entire event loop, stopping the trace
processing instead of gracefully defaulting to 'Unknown'.

[ ... ]
> + def print_summary(self) -> None:
> + """Print aggregated statistics."""
[ ... ]
> + print(f"{'comm [pid]':<30} {'count':>10}")
> + print(f"{'-' * 30:<30} {'-' * 10:>10}")
> +
> + sorted_keys = sorted(self.syscalls.keys(), key=lambda k: (k[0], k[1], k[2]))

Could this trigger a TypeError crash regression during summary generation?

The thread command extension can return None if a thread lacks a command string.
This None value is then used as the first element in the tuple key for
self.syscalls.

During this sort operation, Python 3 will raise a TypeError when attempting to
compare None with a string, which happens in normal traces.

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