[PATCH] rtla: add tests for tracefs instance cleanup

[PATCH] rtla: add tests for tracefs instance cleanup

[Chaosheng Qu]

rtla creates a tracefs instance per run (osnoise_top, osnoise_hist,
timerlat_top, timerlat_hist) and is expected to remove it again on the way
out. Nothing verifies that today, so a cleanup regression would only show up
as a confusing failure of the *next* run: tracefs_instance_create() can fail
with -EEXIST, and an interrupted run can leave stale tracer state behind.

tests/engine.sh already works around this by rmdir-ing leftover instances
before every test ("remove any dangling instances created by improperly
exited rtla runs"), which shows the gap is known but untested.

Add a test for the abnormal-exit paths that are actually hit in practice:

  - SIGINT during a run (interactive Ctrl-C)
  - rtla's own -d/--duration timer
  - a failing --on-threshold shell action
  - several interrupted runs in a row, followed by a normal run

These need rtla running in the background while the test signals it, which
engine.sh's check() (a single foreground command) cannot express, so the new
cleanup.t drives a helper script and emits TAP itself.

SIGTERM is covered too, but reported as a TAP TODO comment: rtla installs
handlers for SIGINT and SIGALRM only, so SIGTERM terminates the process
without running the cleanup path and leaks the instance. Keeping it visible
rather than turning it into a pass means a future regression in the paths
that *do* work is still caught. Once SIGTERM is handled, drop the TODO
comment and the case becomes a plain assertion.

The test needs root, tracefs and CONFIG_OSNOISE_TRACER, and skips cleanly
when tracefs is not mounted writable.

Signed-off-by: Chaosheng Qu <chaosheng.qu.linux@gmail.com>
---
 tools/tracing/rtla/tests/cleanup.t            |  55 ++++++
 .../rtla/tests/scripts/check-cleanup.sh       | 187 ++++++++++++++++++
 2 files changed, 242 insertions(+)
 create mode 100755 tools/tracing/rtla/tests/cleanup.t
 create mode 100755 tools/tracing/rtla/tests/scripts/check-cleanup.sh

diff --git a/tools/tracing/rtla/tests/cleanup.t b/tools/tracing/rtla/tests/cleanup.t
new file mode 100755
index 000000000000..46e69c191792
--- /dev/null
+++ b/tools/tracing/rtla/tests/cleanup.t
@@ -0,0 +1,55 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Verify that rtla removes the tracefs instance it created on the abnormal
+# exit paths users actually hit:
+#
+#   - SIGINT during a run            (interactive Ctrl-C)
+#   - rtla's own -d/--duration timer
+#   - a failing --on-threshold shell action
+#   - several interrupted runs in a row, followed by a normal run
+#
+# A leaked instance pollutes the next invocation, so this is worth pinning
+# down in CI. The SIGTERM case is reported as a TAP TODO comment because
+# rtla does not install a SIGTERM handler yet.
+source tests/engine.sh
+test_begin
+
+set_timeout 2m
+
+#
+# These cases need to run rtla in the background and signal it, which
+# engine.sh's check() (one foreground command) cannot express.
+#
+run_cleanup_cases() {
+	local helper="$testdir/scripts/check-cleanup.sh"
+	local out rc
+
+	ctr=$((ctr + 1))
+	[ -z "$TEST_COUNT" ] && return
+
+	if [ ! -x "$helper" ]; then
+		echo "not ok $ctr - cleanup helper is not executable"
+		return
+	fi
+
+	out=$("$helper" "$RTLA" 2>&1)
+	rc=$?
+	if [ $rc -eq 0 ]; then
+		# Surface known issues as TAP comments so they stay visible
+		# even when the case passes.
+		printf '%s\n' "$out" | grep '^TODO: ' | while read -r line; do
+			echo "# ${line#TODO: }"
+		done
+		echo "ok $ctr - instances cleaned up on SIGINT, timeout and action failure"
+	else
+		echo "not ok $ctr - instances cleaned up on SIGINT, timeout and action failure"
+		printf '%s\n' "$out" | while read -r line; do
+			echo "# $line"
+		done
+	fi
+}
+
+run_cleanup_cases
+
+test_end
diff --git a/tools/tracing/rtla/tests/scripts/check-cleanup.sh b/tools/tracing/rtla/tests/scripts/check-cleanup.sh
new file mode 100755
index 000000000000..1ead5ce3c517
--- /dev/null
+++ b/tools/tracing/rtla/tests/scripts/check-cleanup.sh
@@ -0,0 +1,187 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Verify that rtla removes the tracefs instances it created, even when it is
+# interrupted (SIGINT/SIGTERM), killed by its own timeout, or when a shell
+# action fails.
+#
+# This is a *test* for tools/tracing/rtla; it does not change rtla behaviour.
+#
+# Requires: root, tracefs mounted at /sys/kernel/tracing, CONFIG_OSNOISE_TRACER.
+# Usage: rtla_cleanup_test.sh /path/to/rtla
+
+RTLA=${1:-rtla}
+TRACEFS=/sys/kernel/tracing
+INSTANCES=$TRACEFS/instances
+FAIL=0
+PASS=0
+
+log()  { printf '%s\n' "$*"; }
+pass() { PASS=$((PASS+1)); log "PASS: $*"; }
+fail() { FAIL=$((FAIL+1)); log "FAIL: $*"; }
+todo() { TODO_N=$((TODO_N+1)); log "TODO: $*"; }
+TODO_N=0
+
+cleanup_all() {
+	for i in osnoise_top osnoise_hist timerlat_top timerlat_hist hwnoise_top; do
+		[ -d "$INSTANCES/$i" ] && rmdir "$INSTANCES/$i" 2>/dev/null
+	done
+}
+
+# Wait up to $2 ms for $1 to disappear.
+wait_gone() {
+	_path=$1
+	_ms=${2:-5000}
+	_i=0
+	while [ -d "$_path" ] && [ $_i -lt $_ms ]; do
+		sleep 0.05
+		_i=$((_i+50))
+	done
+	[ ! -d "$_path" ]
+}
+
+# Wait until $1 exists (up to $2 ms).
+wait_present() {
+	_path=$1
+	_ms=${2:-10000}
+	_i=0
+	while [ ! -d "$_path" ] && [ $_i -lt $_ms ]; do
+		sleep 0.05
+		_i=$((_i+50))
+	done
+	[ -d "$_path" ]
+}
+
+check_instances_absent() {
+	_desc=$1
+	_left=""
+	for i in osnoise_top osnoise_hist timerlat_top timerlat_hist; do
+		[ -d "$INSTANCES/$i" ] && _left="$_left $i"
+	done
+	if [ -z "$_left" ]; then
+		pass "$_desc: no leftover instance"
+	else
+		fail "$_desc: leftover instance(s):$_left"
+	fi
+}
+
+if [ ! -d "$INSTANCES" ]; then
+	log "SKIP: tracefs not mounted at $TRACEFS"
+	exit 0
+fi
+
+# ---------------------------------------------------------------------------
+# 1. SIGINT during a long run
+# ---------------------------------------------------------------------------
+cleanup_all
+$RTLA osnoise top -q -d 30s -P F:1 >/dev/null 2>&1 &
+pid=$!
+wait_present "$INSTANCES/osnoise_top" 10000
+sleep 5
+
+if [ ! -d "$INSTANCES/osnoise_top" ]; then
+	fail "SIGINT: instance was never created (rtla did not start?)"
+else
+	pass "SIGINT: instance created while running"
+	kill -INT "$pid" 2>/dev/null
+	wait "$pid" 2>/dev/null
+	if wait_gone "$INSTANCES/osnoise_top"; then
+		pass "SIGINT: instance removed after interrupt"
+	else
+		fail "SIGINT: instance still present after interrupt"
+	fi
+fi
+
+# ---------------------------------------------------------------------------
+# 2. SIGTERM during a long run
+#
+# KNOWN ISSUE (not a test bug): rtla only installs a handler for SIGINT and
+# SIGALRM (src/common.c:set_signals). SIGTERM therefore kills rtla outright,
+# leaving the tracefs instance behind.
+#
+# Record it with TAP "not ok ... # TODO" rather than turning it into a pass:
+# a TODO is still visible in the log, keeps the suite green, and
+# - unlike "pass" - it does not hide a real cleanup regression if the
+#   SIGTERM path ever changes. Remove the TODO directive once rtla handles
+#   SIGTERM.
+# ---------------------------------------------------------------------------
+KNOWN_SIGTERM_LEAK=1
+cleanup_all
+$RTLA osnoise top -q -d 30s -P F:1 >/dev/null 2>&1 &
+pid=$!
+if ! wait_present "$INSTANCES/osnoise_top" 10000; then
+	fail "SIGTERM: instance was never created (rtla did not start)"
+	kill -TERM "$pid" 2>/dev/null
+	wait "$pid" 2>/dev/null
+else
+sleep 1
+
+if ! kill -0 "$pid" 2>/dev/null; then
+	fail "SIGTERM: rtla died before the signal was sent"
+else
+	kill -TERM "$pid" 2>/dev/null
+	wait "$pid" 2>/dev/null
+	if wait_gone "$INSTANCES/osnoise_top"; then
+		pass "SIGTERM: instance removed after terminate"
+	elif [ "$KNOWN_SIGTERM_LEAK" -eq 1 ]; then
+		todo "SIGTERM: instance leaked (rtla has no SIGTERM handler)"
+	else
+		fail "SIGTERM: instance still present after terminate"
+	fi
+fi
+fi
+
+# ---------------------------------------------------------------------------
+# 3. rtla's own -d timeout path
+# ---------------------------------------------------------------------------
+cleanup_all
+$RTLA osnoise top -q -d 2s -P F:1 >/dev/null 2>&1
+rc=$?
+if wait_gone "$INSTANCES/osnoise_top"; then
+	pass "self-timeout: instance removed (exit=$rc)"
+else
+	fail "self-timeout: instance still present (exit=$rc)"
+fi
+
+# ---------------------------------------------------------------------------
+# 4. shell action failure (-a / --on-threshold shell,command=...)
+# ---------------------------------------------------------------------------
+cleanup_all
+$RTLA osnoise top -q -d 5s -P F:1 -r 1000 -S 1 \
+	--on-threshold 'shell,command="/bin/false"' >/dev/null 2>&1
+rc=$?
+if wait_gone "$INSTANCES/osnoise_top"; then
+	pass "action-failure: instance removed (exit=$rc)"
+else
+	fail "action-failure: instance still present (exit=$rc)"
+fi
+
+# ---------------------------------------------------------------------------
+# 5. repeated interrupted runs must not poison the next run
+# ---------------------------------------------------------------------------
+cleanup_all
+for n in 1 2 3; do
+	$RTLA osnoise top -q -d 20s -P F:1 >/dev/null 2>&1 &
+	pid=$!
+	sleep 1
+	kill -INT "$pid" 2>/dev/null
+	wait "$pid" 2>/dev/null
+done
+
+$RTLA osnoise top -q -d 2s -P F:1 >/dev/null 2>&1
+rc=$?
+if [ "$rc" -eq 0 ]; then
+	pass "repeated interrupts: following run still succeeds"
+else
+	fail "repeated interrupts: following run failed (exit=$rc)"
+fi
+if wait_gone "$INSTANCES/osnoise_top"; then
+	pass "repeated interrupts: no leftover instance"
+else
+	fail "repeated interrupts: leftover instance"
+fi
+
+cleanup_all
+log ""
+log "SUMMARY: $PASS passed, $FAIL failed, $TODO_N known issues"
+[ "$FAIL" -eq 0 ]

base-commit: 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e
-- 
2.43.0

Re: [PATCH] rtla: add tests for tracefs instance cleanup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] rtla/tests: missing prerequisite checks and swallowed SKIP in cleanup.t
- [Medium] rtla/tests: SIGINT race with set_signals() and leaked process in check-cleanup.sh
- [Medium] rtla/tests: missing exit code checks and flawed action-failure test in check-cleanup.sh
- [Low] rtla/tests: unused TIMEOUT, skipped reset_osnoise(), and dead code in cleanup test
--

commit 5c3de0149491a2031a1d6a6944918bec1fa8f6ee
Author: Chaosheng Qu <chaosheng.qu.linux@gmail.com>

rtla: add tests for tracefs instance cleanup

This commit adds cleanup.t and a helper script check-cleanup.sh to verify
that rtla removes its tracefs instances on SIGINT, timeout, action failure,
and repeated interrupted runs.

> diff --git a/tools/tracing/rtla/tests/cleanup.t b/tools/tracing/rtla/tests/cleanup.t
> new file mode 100755
> index 0000000000000..46e69c1917924
> --- /dev/null
> +++ b/tools/tracing/rtla/tests/cleanup.t
> @@ -0,0 +1,55 @@
[ ... ]
> +source tests/engine.sh
> +test_begin
> +
> +set_timeout 2m
> +
> +#
> +# These cases need to run rtla in the background and signal it, which
> +# engine.sh's check() (one foreground command) cannot express.
> +#
> +run_cleanup_cases() {
> +	local helper="$testdir/scripts/check-cleanup.sh"
> +	local out rc
> +
> +	ctr=$((ctr + 1))
> +	[ -z "$TEST_COUNT" ] && return
> +
> +	if [ ! -x "$helper" ]; then
> +		echo "not ok $ctr - cleanup helper is not executable"
> +		return
> +	fi
> +
> +	out=$("$helper" "$RTLA" 2>&1)

[Severity: Low]
Calling set_timeout 2m sets the TIMEOUT variable in tests/engine.sh, and
engine.sh's check() normally runs reset_osnoise before executing each test
under $TIMEOUT.

Because run_cleanup_cases() bypasses check() and invokes $helper directly,
$TIMEOUT is never applied (allowing wait "$pid" in check-cleanup.sh to hang
indefinitely if rtla does not exit) and global osnoise settings and
tracing_thresh are not reset before the test runs. Should run_cleanup_cases()
call reset_osnoise and prefix the helper invocation with $TIMEOUT?

> +	rc=$?
> +	if [ $rc -eq 0 ]; then
> +		# Surface known issues as TAP comments so they stay visible
> +		# even when the case passes.
> +		printf '%s\n' "$out" | grep '^TODO: ' | while read -r line; do
> +			echo "# ${line#TODO: }"
> +		done
> +		echo "ok $ctr - instances cleaned up on SIGINT, timeout and action failure"

[Severity: Medium]
When check-cleanup.sh prints "SKIP: ..." and exits with 0, run_cleanup_cases()
only greps for '^TODO: ' and emits a plain "ok $ctr" line instead of a TAP
"# SKIP" directive. Should run_cleanup_cases() detect '^SKIP: ' in $out and
emit "ok $ctr - ... # SKIP ..."?

> +	else
> +		echo "not ok $ctr - instances cleaned up on SIGINT, timeout and action failure"
[ ... ]
> diff --git a/tools/tracing/rtla/tests/scripts/check-cleanup.sh b/tools/tracing/rtla/tests/scripts/check-cleanup.sh
> new file mode 100755
> index 0000000000000..1ead5ce3c5177
> --- /dev/null
> +++ b/tools/tracing/rtla/tests/scripts/check-cleanup.sh
> @@ -0,0 +1,187 @@
[ ... ]
> +check_instances_absent() {
> +	_desc=$1
> +	_left=""
> +	for i in osnoise_top osnoise_hist timerlat_top timerlat_hist; do
> +		[ -d "$INSTANCES/$i" ] && _left="$_left $i"
> +	done
> +	if [ -z "$_left" ]; then
> +		pass "$_desc: no leftover instance"
> +	else
> +		fail "$_desc: leftover instance(s):$_left"
> +	fi
> +}

[Severity: Low]
check_instances_absent() is defined here, but it is never called anywhere in
check-cleanup.sh. Should this unused function be removed or used in the test
cases below?

> +
> +if [ ! -d "$INSTANCES" ]; then
> +	log "SKIP: tracefs not mounted at $TRACEFS"
> +	exit 0
> +fi

[Severity: Medium]
The commit message states:

  The test needs root, tracefs and CONFIG_OSNOISE_TRACER, and skips cleanly
  when tracefs is not mounted writable.

However, this check only tests whether $INSTANCES is a directory. If the test
runs without root privileges, with a read-only tracefs mount, or on a kernel
without CONFIG_OSNOISE_TRACER ($TRACEFS/osnoise), [ ! -d "$INSTANCES" ] can
still be false, causing all test cases to run and fail instead of skipping.
Should this also verify [ -w "$INSTANCES" ] and [ -d "$TRACEFS/osnoise" ]?

> +
> +# ---------------------------------------------------------------------------
> +# 1. SIGINT during a long run
> +# ---------------------------------------------------------------------------
> +cleanup_all
> +$RTLA osnoise top -q -d 30s -P F:1 >/dev/null 2>&1 &
> +pid=$!
> +wait_present "$INSTANCES/osnoise_top" 10000
> +sleep 5
> +
> +if [ ! -d "$INSTANCES/osnoise_top" ]; then
> +	fail "SIGINT: instance was never created (rtla did not start?)"

[Severity: Medium]
If $INSTANCES/osnoise_top is not present here, the failure branch logs a
failure without killing or waiting on $pid (unlike Case 2 below). Can this
leave the background rtla process running while the remaining test cases
execute?

> +else
> +	pass "SIGINT: instance created while running"
[ ... ]
> +# ---------------------------------------------------------------------------
> +# 3. rtla's own -d timeout path
> +# ---------------------------------------------------------------------------
> +cleanup_all
> +$RTLA osnoise top -q -d 2s -P F:1 >/dev/null 2>&1
> +rc=$?
> +if wait_gone "$INSTANCES/osnoise_top"; then
> +	pass "self-timeout: instance removed (exit=$rc)"
> +else
> +	fail "self-timeout: instance still present (exit=$rc)"
> +fi
> +
> +# ---------------------------------------------------------------------------
> +# 4. shell action failure (-a / --on-threshold shell,command=...)
> +# ---------------------------------------------------------------------------
> +cleanup_all
> +$RTLA osnoise top -q -d 5s -P F:1 -r 1000 -S 1 \
> +	--on-threshold 'shell,command="/bin/false"' >/dev/null 2>&1
> +rc=$?
> +if wait_gone "$INSTANCES/osnoise_top"; then
> +	pass "action-failure: instance removed (exit=$rc)"
> +else
> +	fail "action-failure: instance still present (exit=$rc)"
> +fi

[Severity: Medium]
Can Cases 3 and 4 pass without exercising their intended cleanup paths?

First, neither Case 3 nor Case 4 verifies $rc or checks that osnoise_top was
actually created. Because cleanup_all removes $INSTANCES/osnoise_top before
running $RTLA, if $RTLA fails during startup before creating the instance,
wait_gone "$INSTANCES/osnoise_top" immediately succeeds and reports a pass.

Second, in Case 4, -r 1000 sets a 1ms runtime per 1s period (0.1% duty cycle)
with SCHED_FIFO:1. On an idle system, 5 periods can finish without
accumulating 1us of noise, so rtla exits via the 5s duration timeout (rc=0)
rather than hitting the stop-tracing threshold (rc=2) and never runs the
shell action.

Third, in tools/tracing/rtla/src/common.c:common_threshold_handler():

	actions_perform(&tool->params->threshold_actions);

	if (!should_continue_tracing(tool->params))
		/* continue flag not set, break */
		return 0;

common_threshold_handler() ignores the return value of actions_perform() and
stops tracing whenever continue_flag is not set, which is only set if a
subsequent "--on-threshold continue" action is reached after the shell action
succeeds. Without "--on-threshold continue" and checking [ "$rc" -eq 2 ],
Case 4 does not distinguish a failing shell action from a succeeding one or
from a normal timeout.

> +
> +# ---------------------------------------------------------------------------
> +# 5. repeated interrupted runs must not poison the next run
> +# ---------------------------------------------------------------------------
> +cleanup_all
> +for n in 1 2 3; do
> +	$RTLA osnoise top -q -d 20s -P F:1 >/dev/null 2>&1 &
> +	pid=$!
> +	sleep 1
> +	kill -INT "$pid" 2>/dev/null
> +	wait "$pid" 2>/dev/null
> +done

[Severity: Medium]
In tools/tracing/rtla/src/common.c:run_tool(), the tracefs instance is
created at the beginning of ops->init_tool(params) -> trace_instance_init(),
before tracefs_local_events(NULL), ops->apply_config(tool),
enable_tracer_by_name(), set_comm_sched_attr(), and ops->enable(tool), while
set_signals(params) only installs the SIGINT handler afterward:

	tool = ops->init_tool(params);
	...
	retval = ops->enable(tool);
	if (retval)
		goto out_trace;

	tool->start_time = time(NULL);
	set_signals(params);

In Case 5, each background invocation of $RTLA is sent SIGINT after a fixed
"sleep 1" without wait_present or waiting for set_signals() to complete:

1. Under non-interactive POSIX /bin/sh, background jobs started with '&'
   inherit SIG_IGN for SIGINT until set_signals() overrides it. If rtla is
   still initializing after 1 second (for example while parsing tracefs event
   formats or setting per-CPU osnoise kthreads to SCHED_FIFO:1), kill -INT is
   ignored and wait "$pid" blocks for the full 20s duration timer without
   testing SIGINT interruption.
2. If SIGINT is not ignored in the caller's environment and arrives after
   create_instance() but before set_signals(), rtla is terminated by the
   default SIGINT disposition before stop_trace() is installed, leaking the
   instance.

Should this loop wait for $INSTANCES/osnoise_top to be present and for
set_signals() to be active before sending SIGINT, and verify wait_gone on
each iteration?

> +
> +$RTLA osnoise top -q -d 2s -P F:1 >/dev/null 2>&1
[ ... ]

 base-commit: 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e
-- 
2.51.0

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017122359.402696-1-chaosheng.qu.linux@gmail.com?part=1