Re: [PATCH v5 52/58] perf test: Migrate Intel PT virtual LBR test to use Python API

sashiko-bot
2026-04-24T17:28:06Z

Sashiko AI review found 3 potential issue(s):
- [Medium] The virtual LBR test incorrectly fails instead of skipping when perf is compiled without Python scripting support.
- [Medium] The Python script `perf_brstack_max.py` hardcodes the module import path to the source tree, breaking out-of-tree builds.
- [Medium] The target workload being traced in the virtual LBR test was altered without documentation.
--

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

perf test: Migrate Intel PT virtual LBR test to use Python API

The Intel PT virtual LBR test used an ad-hoc Python script written on
the fly to parse branch stacks. This change migrates it to use the
newly added brstack iterator API in the perf Python module.

> The Intel PT virtual LBR test used an ad-hoc Python script written on
> the fly to parse branch stacks. This change migrates it to use the
> newly added brstack iterator API in the perf Python module.

Is it intentional that the workload was changed from uname to perf test -w
brstack, and a new perf inject step was added?

The commit message only mentions migrating the python script to the new API,
but does not document the changes to the test payload and processing
pipeline.

> diff --git a/tools/perf/tests/shell/lib/perf_brstack_max.py b/tools/perf/tests/shell/lib/perf_brstack_max.py
> new file mode 100644
> index 00000000000000..c826e14160d640
> --- /dev/null
> +++ b/tools/perf/tests/shell/lib/perf_brstack_max.py
[ ... ]
> @@ -0,0 +1,43 @@
> +#!/usr/bin/python
> +# SPDX-License-Identifier: GPL-2.0
> +# Determine the maximum size of branch stacks in a perf.data file.
> +
> +import argparse
> +import sys
> +
> +import os
> +
> +script_dir = os.path.dirname(os.path.abspath(__file__))
> +python_dir = os.path.abspath(os.path.join(script_dir, "../../../python"))
> +sys.path.insert(0, python_dir)
> +
> +import perf

Does this hardcoded relative path break out-of-tree builds like
make O=/path/to/build?

The compiled perf.so module would be generated in the build directory
rather than the source directory, which might cause import perf to fail
with a ModuleNotFoundError.

[ ... ]
> diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
> index 8ee761f03c38b5..d711ecdf5be020 100755
> --- a/tools/perf/tests/shell/test_intel_pt.sh
> +++ b/tools/perf/tests/shell/test_intel_pt.sh
[ ... ]
> @@ -539,34 +538,20 @@ test_kernel_trace()
> test_virtual_lbr()
> {
> echo "--- Test virtual LBR ---"
> - # Check if python script is supported
> - libpython=$(perf version --build-options | grep python | grep -cv OFF)
> - if [ "${libpython}" != "1" ] ; then
> - echo "SKIP: python scripting is not supported"
> + # shellcheck source=lib/setup_python.sh
> + . "$(dirname "$0")"/lib/setup_python.sh
> +
> + if [ -z "$PYTHON" ] ; then
> + echo "SKIP: Python not found"
> return 2
> fi

Will this cause the test to fail instead of skipping if perf was built
without Python support?

The setup_python.sh script only checks if the python executable is available
on the host system. If Python is installed but the perf python extension
was not compiled, the script's import perf statement will fail. This will
result in an empty stdout, evaluating to times_val=0 and causing a hard test
failure rather than a graceful skip.

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