[PATCH v1] perf tests: top/kvm don't depend on coreutils timeout

[PATCH v1] perf tests: top/kvm don't depend on coreutils timeout

[Ian Rogers]

If the coreutils timeout command is missing on a minimal test system
then the kvm and top tests can fail. Provide a fallback bash
implementation for these systems.

Assisted-by: Gemini:gemini-3.1-pro-preview
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/kvm.sh         |  6 ++++-
 tools/perf/tests/shell/lib/timeout.sh | 36 +++++++++++++++++++++++++++
 tools/perf/tests/shell/top.sh         |  7 +++++-
 3 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 tools/perf/tests/shell/lib/timeout.sh

diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
index f88e859025c4..a27b3d6414e9 100755
--- a/tools/perf/tests/shell/kvm.sh
+++ b/tools/perf/tests/shell/kvm.sh
@@ -4,6 +4,9 @@
 
 set -e
 
+# shellcheck source=lib/timeout.sh
+. "$(dirname $0)"/lib/timeout.sh
+
 err=0
 perfdata=$(mktemp /tmp/__perf_kvm_test.perf.data.XXXXX)
 qemu_pid_file=$(mktemp /tmp/__perf_kvm_test.qemu.pid.XXXXX)
@@ -102,7 +105,8 @@ test_kvm_stat_live() {
 
         # Run perf kvm live for 5 seconds, monitoring that PID
 	# Use sleep to keep stdin open but silent, preventing EOF loop or interactive spam
-	if ! sleep 10 | timeout 5s perf kvm stat live -p "${qemu_pid}" > "${log_file}" 2>&1; then
+	if ! sleep 10 | bash_timeout 5s perf kvm stat live -p "${qemu_pid}" > "${log_file}" 2>&1
+	then
 		retval=$?
 		if [ $retval -ne 124 ] && [ $retval -ne 0 ]; then
 			echo "perf kvm stat live [Failed: perf kvm stat live failed to start or run (ret=$retval)]"
diff --git a/tools/perf/tests/shell/lib/timeout.sh b/tools/perf/tests/shell/lib/timeout.sh
new file mode 100644
index 000000000000..b4d2c0acf0e2
--- /dev/null
+++ b/tools/perf/tests/shell/lib/timeout.sh
@@ -0,0 +1,36 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# A version of timeout in pure bash that doesn't require the timeout command.
+bash_timeout() {
+    # Check if the native 'timeout' command is available
+    if command -v timeout >/dev/null 2>&1; then
+        timeout "$@"
+        return $?
+    fi
+
+    local timeout_secs="$1"
+    shift
+    local command=("$@")
+
+    # Execute the target command in the background
+    "${command[@]}" &
+    local cmd_pid=$!
+
+    # Start a watcher process in the background
+    (
+        sleep "$timeout_secs"
+        kill -TERM "$cmd_pid" 2>/dev/null || true
+    ) &
+    local watcher_pid=$!
+
+    # Wait for the main command to complete (or be killed)
+    wait "$cmd_pid" 2>/dev/null
+    local exit_code=$?
+
+    # Clean up: Kill the watcher process if the main command finished early
+    kill "$watcher_pid" 2>/dev/null || true
+
+    # Return the exit status of the target command
+    return $exit_code
+}
diff --git a/tools/perf/tests/shell/top.sh b/tools/perf/tests/shell/top.sh
index ad7fccd09025..7741bca6b41f 100755
--- a/tools/perf/tests/shell/top.sh
+++ b/tools/perf/tests/shell/top.sh
@@ -4,6 +4,9 @@
 
 set -e
 
+# shellcheck source=lib/timeout.sh
+. "$(dirname $0)"/lib/timeout.sh
+
 err=0
 log_file=$(mktemp /tmp/perf.top.log.XXXXX)
 
@@ -35,7 +38,9 @@ test_basic_perf_top() {
 	# Use -d 1 to avoid flooding output
 	# Use -e cpu-clock to ensure we get samples
 	# Use sleep to keep stdin open but silent, preventing EOF loop or interactive spam
-	if ! sleep 10 | timeout 5s perf top --stdio -d 1 -e cpu-clock -p $PID > "${log_file}" 2>&1; then
+	if ! sleep 10 | \
+	     bash_timeout 5s perf top --stdio -d 1 -e cpu-clock -p $PID > "${log_file}" 2>&1
+	then
 		retval=$?
 		if [ $retval -ne 124 ] && [ $retval -ne 0 ]; then
 			echo "Basic perf top test [Failed: perf top failed to start or run (ret=$retval)]"
-- 
2.54.0.rc0.605.g598a273b03-goog

Re: [PATCH v1] perf tests: top/kvm don't depend on coreutils timeout

[sashiko-bot]

Sashiko AI review found 4 potential issue(s):
- [High] Orphaned watcher process can send SIGTERM to unrelated system processes (PID recycling).
- [Medium] Exit code mismatch in timeout emulation falsely fails tests.
- [Medium] Implicit standard input redirection breaks the protective pipeline.
- [Medium] Non-POSIX sleep argument causes immediate timeout on strict minimal systems.
--

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

perf tests: top/kvm don't depend on coreutils timeout

This commit adds a bash-based fallback for the coreutils timeout command
to prevent failures on minimal test systems. It then updates the kvm and
top test scripts to use this new bash_timeout function.

> diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
> index f88e859025c42..a27b3d6414e9d 100755
> --- a/tools/perf/tests/shell/kvm.sh
> +++ b/tools/perf/tests/shell/kvm.sh
[ ... ]
> @@ -102,7 +105,8 @@ test_kvm_stat_live() {
>  
>          # Run perf kvm live for 5 seconds, monitoring that PID
>  	# Use sleep to keep stdin open but silent, preventing EOF loop or interactive spam
> -	if ! sleep 10 | timeout 5s perf kvm stat live -p "${qemu_pid}" > "${log_file}" 2>&1; then
> +	if ! sleep 10 | bash_timeout 5s perf kvm stat live -p "${qemu_pid}" > "${log_file}" 2>&1

Does POSIX sleep support the "s" suffix passed as "5s" here?

On minimal systems lacking coreutils, sleep "5s" might fail instantly, causing
the watcher script to kill the target command in 0 seconds.

> +	then
>  		retval=$?
>  		if [ $retval -ne 124 ] && [ $retval -ne 0 ]; then

When bash_timeout kills the process via SIGTERM, it returns 143 (128 + 15), not
the GNU timeout exit code of 124.

Since this code checks for 124, will an expected timeout incorrectly register
as a test failure?

> diff --git a/tools/perf/tests/shell/lib/timeout.sh b/tools/perf/tests/shell/lib/timeout.sh
> new file mode 100644
> index 0000000000000..b4d2c0acf0e22
> --- /dev/null
> +++ b/tools/perf/tests/shell/lib/timeout.sh
[ ... ]
> +bash_timeout() {
[ ... ]
> +    local timeout_secs="$1"
> +    shift
> +    local command=("$@")
> +
> +    # Execute the target command in the background
> +    "${command[@]}" &

Does placing the target command in the background automatically redirect its
standard input from /dev/null?

This appears to sever the connection to the "sleep 10 |" pipe used in the
calling scripts, causing perf to read an immediate EOF and triggering the loop
and interactive spam the pipe was meant to prevent.

> +    local cmd_pid=$!
> +
> +    # Start a watcher process in the background
> +    (
> +        sleep "$timeout_secs"
> +        kill -TERM "$cmd_pid" 2>/dev/null || true
> +    ) &

If the test script exits early (for example, due to set -e or an interrupt)
before killing the watcher process, does the watcher survive as an orphaned
background task?

Since perf tests often run as root, this orphaned watcher might wake up later
and send SIGTERM to a recycled PID, potentially terminating unrelated system
services.

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