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

sashiko-bot
2026-04-25T18:07:13Z

Sashiko AI review found 3 potential issue(s):
- [Medium] The script uses modern Python features that break compatibility with older Python 3 versions commonly used in enterprise distributions.
- [Medium] Unrestricted per-sample printing of unmapped physical addresses causes severe log spam and performance degradation.
- [Low] Incorrect handling of the `bisect_right` return value masks a logical flaw via negative indexing.
--

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

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

This commit adds a Python script that uses the perf python session API to
process a perf.data file. It demonstrates resolving physical address samples
to memory types by parsing /proc/iomem.

> diff --git a/tools/perf/python/mem-phys-addr.py b/tools/perf/python/mem-phys-addr.py
> new file mode 100755
> index 000000000000..ba874d7a2011
> --- /dev/null
> +++ b/tools/perf/python/mem-phys-addr.py
> @@ -0,0 +1,117 @@
[ ... ]
> +# Physical memory layout from /proc/iomem. Key is the indent and then
> +# a list of ranges.
> +iomem: Dict[int, list[IomemEntry]] = collections.defaultdict(list)

Will this cause a TypeError on Python versions older than 3.9? Generic type
hints in standard collections like list require Python 3.9+. Older Python
versions commonly used in enterprise distributions might fail here.

> +# Child nodes from the iomem parent.
> +children: Dict[IomemEntry, set[IomemEntry]] = collections.defaultdict(set)

Similarly, will using set[] instead of Set[] from the typing module cause
exceptions on Python environments older than 3.9?

[ ... ]
> +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)

Does this break compatibility with Python versions older than 3.10? The key
parameter in bisect.bisect_right() was introduced in Python 3.10, so this
might result in a TypeError on older systems.

> + if position is None:

Since bisect.bisect_right() always returns an integer, is this check for
None necessary?

> + continue
> + iomem_entry = iomem[i][position-1]

If phys_addr is smaller than the lowest address in the list, position will
be 0. This would result in accessing index -1, which fetches the last element
of the list via negative indexing wrap-around.

While the bounds check below will handle it safely, relying on this behavior
seems logically incorrect. Would it be better to check for position == 0
instead of checking for None?

> + if iomem_entry.begin <= phys_addr <= iomem_entry.end:
> + return iomem_entry
> + print(f"Didn't find {phys_addr}")

Because process_event() evaluates thousands or millions of samples, won't
this print statement cause massive log spam and severely degrade performance
for unmapped physical addresses?

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