Re: [PATCH v7 29/59] perf futex-contention: Port futex-contention to use python module

sashiko-bot
2026-04-25T23:11:55Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Silent omission of futex contention data for unresolved threads.
- [Medium] Silent bogus statistics when tracepoint fields are missing from the sample.
--

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

perf futex-contention: Port futex-contention to use python module

Rewrite tools/perf/scripts/python/futex-contention.py to use the python
module to improve execution performance.

> diff --git a/tools/perf/python/futex-contention.py b/tools/perf/python/futex-contention.py
> new file mode 100755
> index 000000000000..1fc87ec0e6e5
> --- /dev/null
> +++ b/tools/perf/python/futex-contention.py
[ ... ]
> + def handle_start(tid: int, uaddr: int, op: int, start_time: int) -> None:
> + if (op & FUTEX_CMD_MASK) != FUTEX_WAIT:
> + return
> + if tid not in process_names:
> + try:
> + if session:
> + process = session.find_thread(tid)
> + if process:
> + process_names[tid] = process.comm()
> + except (TypeError, AttributeError):
> + return

Does this early return introduce a regression causing silent data loss for
unresolved threads?

If an exception occurs when resolving the thread name, the script returns early
and completely skips the start_times[tid] assignment below. Because the thread
ID is never added to process_names, subsequent events for the same thread will
hit this same branch and fail again, dropping all lock tracking for the thread.

Could the script instead ignore the exception and rely on the
process_names.get(t, 'unknown') fallback during final output?

> + start_times[tid] = (uaddr, start_time)
[ ... ]
> + event_name = str(sample.evsel)
> + if event_name == "evsel(syscalls:sys_enter_futex)":
> + uaddr = getattr(sample, "uaddr", 0)
> + op = getattr(sample, "op", 0)

Will this regression corrupt the contention measurements with bogus data if
tracepoint fields are missing from the sample?

If uaddr or op are not present on the sample object (for example, due to
missing libtraceevent support or unparsed tracepoints), getattr defaults both
variables to 0.

Because 0 exactly matches the value of FUTEX_WAIT, every sys_enter_futex event
(including wakes and requeues) would be incorrectly treated as a wait
contention on lock address 0x0.

> + handle_start(sample.sample_tid, uaddr, op, sample.sample_time)

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