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

sashiko-bot
2026-04-20T00:50:35Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Events missing expected fields (like `fd` or `count`) are silently dropped instead of being logged as unhandled.
- [Medium] Inconsistent syscall substring matching leads to some variants being silently dropped while others are correctly marked as unhandled.
--

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

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

This commit ports the legacy Perl script rw-by-file.pl to a Python script
using the perf module in tools/perf/python. It leverages the perf.session
API to track read and write activity by file descriptor for a given program.

> diff --git a/tools/perf/python/rw-by-file.py b/tools/perf/python/rw-by-file.py
> new file mode 100755
> index 0000000000000..4dd164a091e2f
> --- /dev/null
> +++ b/tools/perf/python/rw-by-file.py
[ ... ]
> + def process_event(self, sample: perf.sample_event) -> None:
> + """Process events."""
> + event_name = str(sample.evsel)
[ ... ]
> + if "sys_enter_read" in event_name:

Does this substring check inadvertently match variants like sys_enter_readv
or sys_enter_readlink?

If these variants are matched but lack a count field, they will trigger an
AttributeError below, whereas other variants like sys_enter_pread64 would
correctly fall through to the unhandled block.

> + try:
> + fd = sample.fd
> + count = sample.count
> + self.reads[fd]["bytes_requested"] += count
> + self.reads[fd]["total_reads"] += 1
> + except AttributeError:
> + return

If an event is missing expected fields (such as if the user did not record
trace fields, or if a syscall variant like sys_enter_readv is matched),
should it be added to the self.unhandled tracker instead of silently
returning here?

Silently dropping these events could result in misleading statistics without
alerting the user.

> + elif "sys_enter_write" in event_name:
> + try:
> + fd = sample.fd
> + count = sample.count
> + self.writes[fd]["bytes_written"] += count
> + self.writes[fd]["total_writes"] += 1
> + except AttributeError:
> + return

Similarly, is it intended for write events missing the fd or count fields to
be silently dropped rather than tracked as unhandled?

> + else:
> + self.unhandled[event_name] += 1

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