Re: [PATCH v5 44/58] perf sctop: Port sctop to use python module

sashiko-bot
2026-04-24T17:26:44Z

Sashiko AI review found 4 potential issue(s):
- [High] The fallback logic for `syscalls:sys_enter` checks the wrong trace field, causing all fallback events to be discarded.
- [High] Offline interval tracking is non-functional because it accesses a non-existent `time` attribute instead of `sample_time`.
- [High] Command name (`comm`) filtering in live mode silently discards all events.
- [High] Offline thread resolution crashes the script with an unhandled `TypeError` when a PID is missing from the thread map.
--

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

perf sctop: Port sctop to use python module

This commit ports sctop.py from tools/perf/scripts/python/ to
tools/perf/python/ using a class-based structure. It also adds live mode
support using the LiveSession helper with a fallback strategy for
tracepoint names.

> diff --git a/tools/perf/python/sctop.py b/tools/perf/python/sctop.py
> new file mode 100755
> index 00000000000000..d7ac922da510cd
> --- /dev/null
> +++ b/tools/perf/python/sctop.py

[ ... ]

> + def process_event(self, sample: perf.sample_event) -> None:
> + """Collect syscall events."""
> + name = str(sample.evsel)
> + syscall_id = getattr(sample, "id", -1)

Is the id field available for syscalls:sys_enter tracepoints? The fallback
tracepoint defines its field as __syscall_nr (or nr on older kernels),
whereas raw_syscalls:sys_enter provides id. Will this cause getattr to
return -1 and silently drop all fallback events?

> +
> + if syscall_id == -1:
> + return
> +
> + if hasattr(self, 'session') and self.session:
> + comm = self.session.process(sample.sample_pid).comm()

Can this raise an unhandled type error exception? If the PID is not present
in the thread map, the C extension function pyrf_session__process raises an
exception. Could this exception propagate back to the C callback loop and
prematurely abort processing?

> + else:
> + comm = getattr(sample, "comm", "Unknown")

Does the sample have a comm attribute in live mode? Native sample_event
objects do not have a comm attribute, and sys_enter tracepoints do not
expose it. Will this always default to Unknown and cause the live mode
command filter to discard all events?

> +
> + if name in ("evsel(raw_syscalls:sys_enter)", "evsel(syscalls:sys_enter)"):
> + if self.for_comm is not None and comm != self.for_comm:
> + return
> + with self.lock:
> + self.syscalls[syscall_id] += 1
> +
> + if self.offline and hasattr(sample, "time"):

Does the Python sample_event API expose the timestamp as time or
sample_time? Since sys_enter tracepoints do not contain a time trace field,
it looks like this will evaluate to False and bypass the periodic printing
logic entirely.

> + interval_ns = self.interval * (10 ** 9)
> + if self.last_print_time is None:
> + self.last_print_time = sample.time
> + elif sample.time - self.last_print_time >= interval_ns:
> + self.print_current_totals()
> + self.last_print_time = sample.time

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