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

sashiko-bot
2026-04-25T23:06:07Z

Sashiko AI review found 3 potential issue(s):
- [Medium] Python 3.10+ and 3.9+ specific syntax breaks backward compatibility.
- [Medium] Leftover debug `print` causes massive stdout spam and performance degradation.
- [Low] Unintended negative indexing due to flawed bounds checking logic.
--

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

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

Give an example of using the perf python session API to load a
perf.data file and perform the behavior of
tools/perf/scripts/python/mem-phys-addr.py.

> 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
[ ... ]
> +@dataclass(frozen=True)
> +class IomemEntry:
> + """Read from a line in /proc/iomem"""
> + begin: int
> + end: int
> + indent: int
> + label: str
> +
> +# Physical memory layout from /proc/iomem. Key is the indent and then
> +# a list of ranges.
> +iomem: Dict[int, list[IomemEntry]] = collections.defaultdict(list)
> +# Child nodes from the iomem parent.
> +children: Dict[IomemEntry, set[IomemEntry]] = collections.defaultdict(set)

Does using list[IomemEntry] and set[IomemEntry] break compatibility with
Python versions older than 3.9? These generic type hints were introduced in
Python 3.9, which might cause TypeError crashes on older supported
distributions.

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

The key parameter for bisect.bisect_right() was introduced in Python 3.10.
Will this cause TypeError crashes on systems with Python 3.9 or older?

> + if position is None:
> + continue
> + iomem_entry = iomem[i][position-1]

Since bisect_right() always returns an integer, position is None will never be
true.

If phys_addr is smaller than the first element's begin address, position will
evaluate to 0. Doesn't this mean position-1 evaluates to -1, accessing the
last element of the list instead of correctly handling the out-of-bounds case?

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

Will this print statement flood standard output if a large number of physical
addresses cannot be resolved? Since find_memory_type() is called for every
sample in the perf data, it might severely degrade performance and make the
intended summary output unreadable.

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