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

sashiko-bot
2026-04-20T00:46:08Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Hardcoded Python module search path breaks out-of-tree builds.
- [Medium] Removal of libpython support check turns test skips into false failures when perf is built without the Python extension.
--

commit ae322c0b2a25025bdfd1c2b9252929eca12fcd88
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 the ad-hoc Python
script that was previously written on the fly.

> 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 @@
> +#!/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

Will this hardcoded path cause issues for out-of-tree builds?

If perf is built using an output directory, the perf.so Python extension will
be placed in the build directory rather than the source tree.

It looks like the import perf module resolution might fail with a
ModuleNotFoundError during out-of-tree test execution, causing the shell
script to see an invalid output and explicitly fail the virtual LBR test.

[ ... ]

> 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

Could this change cause the test to falsely fail instead of skipping when perf
is built without Python support?

The previous code checked perf version --build-options to verify that
libpython was actually compiled into the tool. Relying only on setup_python.sh
checks if the python executable exists on the host system.

If perf is compiled without python-dev headers, the interpreter will still
try to run the script and crash during import perf. The shell script captures
the empty output, falls back to a 0 value, and registers a test failure
instead of a skip.

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