Re: [PATCH v1 49/58] perf rw-by-pid: Port rw-by-pid to use python module

sashiko-bot
2026-04-20T00:44:36Z

Sashiko AI review found 1 potential issue(s):
- [Medium] Loose substring matching for event names incorrectly aggregates unrelated read/write syscalls and corrupts accounting.
--

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

perf rw-by-pid: Port rw-by-pid to use python module

This commit ports the legacy Perl script rw-by-pid.pl to a Python script
using the perf module. It tracks read and write activity by PID for all
processes, aggregating bytes requested, bytes read, total reads, and errors.

> diff --git a/tools/perf/python/rw-by-pid.py b/tools/perf/python/rw-by-pid.py
> new file mode 100755
> index 0000000000000..7bb51d15eb8dc
> --- /dev/null
> +++ b/tools/perf/python/rw-by-pid.py

[ ... ]

> + def process_event(self, sample: perf.sample_event) -> None:
> + """Process events."""
> + event_name = str(sample.evsel)
> + pid = sample.sample_pid
> +
> + assert self.session is not None
> + try:
> + comm = self.session.process(pid).comm()
> + except Exception: # pylint: disable=broad-except
> + comm = "unknown"
> +
> + if "sys_enter_read" in event_name:

Does this loose substring matching incorrectly aggregate unrelated syscalls?

If the event name is syscalls:sys_enter_readv, syscalls:sys_enter_readlink, or
syscalls:sys_enter_readahead, it appears they will match this condition. The
original Perl script bound callbacks to exact function names.

This could cause statistical inaccuracies when analyzing traces with other
syscall events:

1. sys_enter_readv lacks a count field, so sample.count raises an
AttributeError which is silently ignored. However, sys_exit_readv does have a
ret field, so its return value is added to bytes_read without updating
bytes_requested.

2. sys_enter_readahead has a count field, so bytes_requested is inflated. But
sys_exit_readahead returns 0 on success, which the exit handler treats as an
error, falsely registering every successful readahead as a read error with
code 0.

Would exact event name matching, such as checking if the event name ends with
:sys_enter_read, prevent these inaccuracies?

> + self._handle_sys_enter_read(sample, pid, comm)
> + elif "sys_exit_read" in event_name:
> + self._handle_sys_exit_read(sample, pid)
> + elif "sys_enter_write" in event_name:

Are there similar issues here for write and related tracepoints like writev?

> + self._handle_sys_enter_write(sample, pid, comm)
> + elif "sys_exit_write" in event_name:
> + self._handle_sys_exit_write(sample, pid)
> + else:
> + self.unhandled[event_name] += 1

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