Re: [PATCH v1 26/58] perf mem-phys-addr: Port mem-phys-addr to use python module

sashiko-bot
2026-04-20T00:20:56Z

Sashiko AI review found 2 potential issue(s):
- [High] Usage of the Python 3.10+ `key` argument in `bisect.bisect_right` breaks compatibility with older distributions.
- [Medium] The script hardcodes the input trace file to `"perf.data"`, completely disregarding command-line arguments.
--

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

perf mem-phys-addr: Port mem-phys-addr to use python module

This commit introduces an example of using the perf python session API
to load a trace file and perform the behavior of the mem-phys-addr.py
script.

> diff --git a/tools/perf/python/mem-phys-addr.py b/tools/perf/python/mem-phys-addr.py
> new file mode 100755
> index 0000000000000..17d6fd4f1a2bf
> --- /dev/null
> +++ b/tools/perf/python/mem-phys-addr.py

[ ... ]

> +def find_memory_type(phys_addr) -> Optional[IomemEntry]:
> + """Search iomem for the range containing phys_addr with the maximum indent"""
> + for i in range(max_indent, -1, -1):
> + if i not in iomem:
> + continue
> + position = bisect.bisect_right(iomem[i], phys_addr,
> + key=lambda entry: entry.begin)

Will this crash with a TypeError on older Linux distributions?

The key parameter for the bisect module functions was only introduced in
Python 3.10. Running this script on older, widely-supported environments
like RHEL 9 (Python 3.9) or Ubuntu 20.04 (Python 3.8) might fail to
execute.

[ ... ]

> +if __name__ == "__main__":
> + def process_event(sample):
> + phys_addr = sample.sample_phys_addr
> + entry = find_memory_type(phys_addr)
> + if entry:
> + load_mem_type_cnt[entry] += 1
> +
> + global event_name
> + if event_name is None:
> + event_name = str(sample.evsel)
> +
> + parse_iomem()
> + perf.session(perf.data("perf.data"), sample=process_event).process_events()

Does this ignore command-line arguments by hardcoding the input trace file?

By hardcoding perf.data here, the script ignores sys.argv and forces users
to ensure their trace file is named exactly perf.data and is located in
the current working directory.

Could this be updated to parse arguments and support dynamic trace files,
similar to how the original perf script implementation handled the -i
argument?

> + print_memory_type()

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