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

sashiko-bot
2026-04-25T18:14:06Z

Sashiko AI review found 2 potential issue(s):
- [Medium] The test incorrectly fails instead of skipping when `perf` is compiled without Python scripting support.
- [Medium] Hardcoded relative path in the Python script breaks out-of-tree builds (O=...).
--

commit 0468e87feb947646888210ed3df346abde440b42
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 instead of generating
an ad-hoc Python script.

> 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 000000000000..c826e14160d6
> --- /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)

Does this hardcoded relative path cause regressions for out-of-tree builds?

If built with make O=/tmp/build, the compiled perf C extension is placed in
the build output directory rather than the source tree. It looks like running
this script in such an environment will fail to find the module and crash
with a ModuleNotFoundError.

[ ... ]

> diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
> index 8ee761f03c38..d711ecdf5be0 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 removing the explicit feature check cause false test failures when perf
is built without Python support?

The original code checked perf version --build-options to ensure perf itself
had Python scripting support. The new setup_python.sh only verifies the
presence of a Python interpreter on the host system.

If a user tests a build lacking Python support, setup_python.sh passes, but
the subsequent execution of perf_brstack_max.py crashes when attempting to
import perf. This leaves stdout empty, making times_val evaluate to 0 and
failing the test instead of appropriately skipping it.

[ ... ]

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