Re: [PATCH v7 57/59] perf Documentation: Update for standalone Python scripts and remove obsolete data

sashiko-bot
2026-04-25T23:18:21Z

Sashiko AI review found 3 potential issue(s):
- [Medium] Documentation for `find_thread(tid)` masks a data corruption bug in the Python C extension
- [Low] Documentation for perf.data constructor documents an invalid signature and missing constant
- [Low] Python API documentation suggests unsupported keyword arguments and None defaults
--

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

perf Documentation: Update for standalone Python scripts and remove obsolete data

This commit updates the perf-script-python documentation to describe the
new standalone script usage with the perf module. It also removes obsolete
documentation for perl scripts.

> diff --git a/tools/perf/Documentation/perf-script-python.txt b/tools/perf/Documentation/perf-script-python.txt
> index 27a1cac6fe76..6bcf98e4e6c6 100644
> --- a/tools/perf/Documentation/perf-script-python.txt
> +++ b/tools/perf/Documentation/perf-script-python.txt

[ ... ]

> +### Module Functions
> +
> +- `config_get(name)`: Get a perf config value.
> +- `metrics()`: Returns a list of metrics represented as string values in dictionaries.
> +- `tracepoint(subsystem, name)`: Get tracepoint config.
> +- `parse_events(string)`: Parse a string of events and return an `evlist`.
> +- `parse_metrics(string, pmu=None)`: Parse a string of metrics or metric groups and return an `evlist`.
> +- `pmus()`: Returns a sequence of PMUs.
> +- `syscall_name(num, *, elf_machine=None)`: Turns a syscall number to a string.
> +- `syscall_id(name, *, elf_machine=None)`: Turns a syscall name to a number.

Are these keyword arguments and None defaults actually supported by the
underlying C extensions?

The C extensions, such as pyrf__parse_metrics, appear to use PyArg_ParseTuple
without PyArg_ParseTupleAndKeywords. This means keyword arguments are
unsupported and would cause a TypeError. Furthermore, passing None as a
positional argument to satisfy the default value might also cause a TypeError
because the C parser expects strict types like a string or an int.

[ ... ]

> +### `perf.session`
> +
> +Manages a trace session.
> +
> +- `__init__(data, sample=None)`: Creates a new session. `data` is a `perf.data` object. `sample` is a callback function called for each sample event.
> +- `process_events()`: Reads the trace data and calls the sample callback for each event.
> +- `find_thread(tid)`: Returns the thread associated with a TID.

Is it safe to use this find_thread(tid) method as documented?

The C extension pyrf_session__find_thread parses this single argument into a
pid variable and calls machine__find_thread(machine, pid, pid).

By passing the TID as both the Thread Group ID (PID) and the TID, it triggers
machine__update_thread_pid(machine, th, pid), which forcibly overwrites the
thread's actual PID with its TID if the PID was previously unresolved.

Will this cause the internal thread representation to become disassociated
from its process leader, leading to logic errors in scripts?

> +### `perf.data`
> +
> +Represents a trace file.
> +
> +- `__init__(filename, mode=perf.DATA_MODE_READ)`: Opens a trace file.

Does the C extension pyrf_data__init support keyword arguments?

It seems to only accept positional path and fd arguments, and does not support
kwargs. Also, is the perf.DATA_MODE_READ constant actually exported by the perf
module? If not, scripts relying on this documentation could fail with an
AttributeError or TypeError.

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