Re: [PATCH v5 51/58] perf wakeup-latency: Port wakeup-latency to use python module

sashiko-bot
2026-04-24T17:39:38Z

Sashiko AI review found 3 potential issue(s):
- [Medium] The script hardcodes `min_wakeup_latency` to 1,000,000,000 ns (1 second), which will silently discard true minimum latencies greater than 1 second and print a fabricated value if no valid events are processed.
- [Medium] The script fundamentally alters the performance metric being measured from per-CPU scheduling latency to per-task wakeup latency, breaking compatibility with the legacy script it replaces.
- [Medium] The event matching logic uses loose substring matching, which unintentionally catches `sched_wakeup_new` events in addition to `sched_wakeup`.
--

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

perf wakeup-latency: Port wakeup-latency to use python module

This commit ports the legacy Perl script wakeup-latency.pl to a Python script
using the perf module. It leverages the perf.session API for event processing
to measure wakeup latency.

> diff --git a/tools/perf/python/wakeup-latency.py b/tools/perf/python/wakeup-latency.py
> new file mode 100755
> index 00000000000000..1b0db115abcf05
> --- /dev/null
> +++ b/tools/perf/python/wakeup-latency.py
[ ... ]
> +class WakeupLatency:
> + """Tracks and displays wakeup latency statistics."""
> + def __init__(self) -> None:
> + self.last_wakeup: Dict[int, int] = defaultdict(int)
> + self.max_wakeup_latency = 0
> + self.min_wakeup_latency = 1000000000

Is there a regression with how minimum latencies are tracked here? If all
recorded latencies in a trace exceed 1,000,000,000 ns, it appears the minimum
will be capped at exactly 1 second, discarding real data.

Additionally, if no valid events are processed, this prints 1000000000
instead of "N/A".

Could min_wakeup_latency be initialized to infinity, such as float('inf'), to
avoid discarding real data?

> + self.total_wakeup_latency = 0
[ ... ]
> + def process_event(self, sample: perf.sample_event) -> None:
> + """Process events."""
> + event_name = str(sample.evsel)
> + sample_time = sample.sample_time
> +
> + if "sched:sched_wakeup" in event_name:

Can this substring check cause a regression by unintentionally matching
"sched:sched_wakeup_new" events?

The condition evaluates to True for both tracepoints, silently merging normal
wakeups with new task wakeups.

Would using strict equality or explicitly matching the exact event name
prevent this?

> + try:
> + pid = sample.pid
> + self.last_wakeup[pid] = sample_time
> + except AttributeError:
> + self.unhandled[event_name] += 1
> + elif "sched:sched_switch" in event_name:
> + try:
> + next_pid = sample.next_pid
> + wakeup_ts = self.last_wakeup.get(next_pid, 0)
> + if wakeup_ts:
> + latency = sample_time - wakeup_ts

Could this logic introduce a regression in the performance metric being
measured?

By tracking the timestamp using the task pid and measuring latency when that
specific next_pid schedules in, the metric changes to per-task wakeup
latency.

The legacy Perl script measured per-CPU responsiveness by tracking the
timestamp of a wakeup using the target CPU and calculating the latency upon
the next context switch on that same CPU.

Does this change break compatibility with the legacy script's output?

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