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

sashiko-bot
2026-04-23T04:27:13Z

Sashiko AI review found 2 potential issue(s):
- [Medium] The `perf_brstack_max.py` script hardcodes the `perf` python module path to the source tree, which breaks out-of-tree tests.
- [Medium] Removing the explicit `perf version` feature check for Python support combined with ignored script exit codes causes false test failures on systems built without the `perf` Python extension.
--

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

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

This commit migrates the Intel PT virtual LBR test to use the newly added
brstack iterator API in the perf Python module, replacing an ad-hoc Python
script that parsed branch stacks.

> 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 0000000000000..c826e14160d64
> --- /dev/null
> +++ b/tools/perf/tests/shell/lib/perf_brstack_max.py
> @@ -0,0 +1,43 @@
[ ... ]
> +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 hardcoding the python path to the source tree break out-of-tree builds?

In out-of-tree builds using O=..., the compiled perf.so extension is
generated in the build directory, not the source tree. This would cause
the import 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 8ee761f03c38b..d711ecdf5be02 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 removing the perf version build-options check for python scripting
cause false test failures on systems built without the perf Python extension?

The setup_python.sh script only verifies if a Python executable exists on the
host, not if perf was built with Python support.

If the perf Python extension is missing, perf_brstack_max.py will crash with
a ModuleNotFoundError. Because test_virtual_lbr is invoked within an OR-list,
POSIX shell rules disable set -e, effectively ignoring the script's crash.
The test evaluates the empty output as times_val=0 and fails instead of
gracefully skipping.

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