Re: [PATCH v5 2/2] perf test: Test perf timechart --live

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf test: unprivileged failure in test_no_events() due to system-wide -a
--

commit a678a998e849226efd51ba949028c49bad18c75e
Author: Alice Rogers <alice.mei.rogers@gmail.com>

perf test: Test perf timechart --live

This commit extends test_ttimechart_python.sh to test --live option
validation, LiveRecorder's perf record subprocess, synthetic live UI
behavior, and live workload recording.

> diff --git a/tools/perf/tests/shell/test_ttimechart_python.sh b/tools/perf/tests/shell/test_ttimechart_python.sh
> index 87b91ed8fc167..c03e71b68991c 100755
> --- a/tools/perf/tests/shell/test_ttimechart_python.sh
> +++ b/tools/perf/tests/shell/test_ttimechart_python.sh

[ ... ]

> @@ -115,6 +145,43 @@ test_no_events() {
>  		err=1
>  		return
>  	fi
> +	# Exercise LiveRecorder's perf record subprocess, control pipe and
> +	# -o output file using task-clock when tracepoints may be unavailable.
> +	rm -f "${clockdata}"
> +	if ! "$PYTHON" - "${PERF_BIN:-perf}" "${clockdata}" \
> +		"$(declare -f workload); workload" > "${output}" 2>&1 <<'EOF'
> +import asyncio
> +import os
> +import sys
> +import ttimechart as tc
> +
> +async def run() -> None:
> +    rec = tc.LiveRecorder(sys.argv[1], False, False, ["bash", "-c", sys.argv[3]], sys.argv[2])
> +    rec.events = lambda: ["task-clock"]
> +    rec.start()
> +    app = tc.TimechartApp("live", False, False, [], live=rec, window=5.0)
> +    try:
> +        async with app.run_test(size=(120, 40)) as pilot:
> +            await pilot.press("space")
> +            await pilot.pause(0.1)
> +            await pilot.press("space")
> +            for _ in range(300):
> +                if app.return_code is not None:
> +                    break
> +                await pilot.pause(0.05)
> +    finally:
> +        rec.stop()
> +    if app.return_code != 1 or not app.data.nr_samples or not os.path.getsize(sys.argv[2]):
> +        raise RuntimeError(f"Unexpected result: rc={app.return_code} samples={app.data.nr_samples}")
> +
> +asyncio.run(run())
> +EOF
> +	then
> +		echo "ttimechart no events test [Failed: LiveRecorder task-clock test]"
> +		cat "${output}"
> +		err=1
> +		return
> +	fi

[Severity: Medium]
Can this cause test_no_events() to fail instead of skip when run by an
unprivileged user on a machine with the default kernel.perf_event_paranoid
setting of 1 or 2?

At the start of test_no_events(), the permission check runs a per-task
recording without -a:

tools/perf/tests/shell/test_ttimechart_python.sh:test_no_events() {
	if ! perf record -e task-clock -o "${clockdata}" -- true > /dev/null 2>&1; then
		echo "ttimechart no events test [Skipped: perf record failed]"
		return
	fi
    ...
}

Because that check opens a per-task software event, it succeeds for
unprivileged users under kernel.perf_event_paranoid = 1 or 2.

However, LiveRecorder.start() unconditionally passes -a for system-wide
recording:

tools/perf/python/ttimechart.py:LiveRecorder.start() {
    ...
            cmd.append("-a")
            if self.workload:
                cmd += ["--", *self.workload]
    ...
}

When kernel.perf_event_paranoid > 0 and the user lacks CAP_PERFMON,
system-wide perf record -a -e task-clock fails with EACCES via
find_get_context() -> perf_allow_cpu(). That leaves app.data.nr_samples
and the output file empty, raising RuntimeError and setting err=1, which
causes the script to exit with 1 (failure) rather than 2 (skip) when
record() subsequently skips.

Should the LiveRecorder task-clock subtest check whether perf record -a -e
task-clock is permitted before running, or only run when system-wide
recording is available?

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