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

sashiko-bot
2026-04-20T00:33:01Z

Sashiko AI review found 1 potential issue(s):
- [High] Silent data loss and missing output in offline trace mode due to incorrect background thread synchronization.
--

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

perf sctop: Port sctop to use python module

This commit ports sctop.py to tools/perf/python/, refactoring it to use a
class-based structure and the perf.session API. It also adds support for
live mode using the LiveSession helper when no input file is specified.

> diff --git a/tools/perf/python/sctop.py b/tools/perf/python/sctop.py
> new file mode 100755
> index 0000000000000..6daa5f0a3b218
> --- /dev/null
> +++ b/tools/perf/python/sctop.py
[ ... ]
> + def print_syscall_totals(self):
> + """Periodically print syscall totals."""
> + while not self.stop_event.is_set():
[ ... ]
> + for syscall_id, val in current_syscalls:
> + print(f"{self.syscall_name(syscall_id):<40s} {val:10d}")
> +
> + self.stop_event.wait(self.interval)

Does this code drop the final batch of accumulated syscall events when
running in offline mode?

When process_events() completes, the script reaches the finally block and
calls stop():

> + def stop(self):
> + """Stop the background thread."""
> + self.stop_event.set()
> + self.thread.join()

Setting stop_event causes stop_event.wait(self.interval) to return
immediately. The loop condition evaluates to false, and the thread exits
without printing any events that were gathered since the last loop iteration.

If the trace file takes less than the default 3 seconds to parse, would the
script exit silently without producing any output at all?

[ ... ]
> + try:
> + if args.input:
> + session = perf.session(perf.data(args.input), sample=analyzer.process_event)
> + session.process_events()
[ ... ]
> + finally:
> + analyzer.stop()

Also, since process_events() parses the trace file as fast as the CPU allows
during offline mode, does relying on a wall-clock wait() mean the interval
sizes are dependent on the host's parsing speed rather than the trace
duration?

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