[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

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

[Chaosheng Qu]

Thanks, the four points split into test-harness bugs and one test premise
that I over-claimed. The series is fixed locally; details below.

1. SKIP was swallowed

cleanup.t now emits a TAP "# SKIP" line when the helper logs SKIP, instead
of reporting a plain "ok". The helper also treats a missing tracefs, a
non-writable tracefs, or a missing CONFIG_OSNOISE_TRACER as SKIP rather
than a failure.

2. SIGINT race and leaked process

The repeated-interrupt case now waits for the tracefs instance to appear
before sending SIGINT, and verifies that it disappears after each run. The
SIGINT case also reaps the background rtla when the instance never
appeared. The helper is run under the suite timeout, so a stuck wait cannot
hang forever.

I also added SIGTERM to set_signals()/unset_signals() in common.c. Before
this, rtla only handled SIGINT and SIGALRM, so SIGTERM left the instance
behind. The test now checks that path instead of recording it as a known
issue.

3. Missing exit-code checks / action case

The self-timeout case now requires exit 0, and the action case requires exit
2 (the mismatched-action result). The action case uses -S, the stop-total
threshold, and skips if that threshold was not crossed. An idle machine may
not generate a sample above 1 us; that is a property of the workload, not a
cleanup result.

4. Unused TIMEOUT / reset_osnoise / dead code

run_cleanup_cases() now calls reset_osnoise and uses $TIMEOUT. The unused
check_instances_absent() helper is gone.

The series is based on 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e and is
commit 455d304ece2a938d0148a908c83f1774d4dbf298. I ran bash -n and
checkpatch.pl locally. The host has no writable tracefs, so the suite
reports SKIP here rather than a real pass.

The patch is attached.

Thanks,
Chaosheng

[PATCH] rtla: add tests for tracefs instance cleanup

[Chaosheng Qu]

rtla creates one of four tracefs instances per run (osnoise_top,
osnoise_hist, timerlat_top, timerlat_hist) and removes it on exit. A
cleanup failure only shows up as the next run failing with -EEXIST or
stale tracer state. tests/engine.sh already removes leftover instances
before each test, so the failure mode is known but not tested.

Add cleanup.t and a helper script covering the abnormal exits: SIGINT,
SIGTERM, the -d timer, a failing --on-threshold shell action, and three
interrupted runs followed by a normal run.

rtla did not handle SIGTERM, so killing it left the tracefs instance
behind. Install stop_trace as the SIGTERM handler, matching SIGINT and
the -d timer.

The helper needs to start rtla in the background and signal it, which
engine.sh's foreground check() cannot express. cleanup.t drives the
helper and emits the TAP result itself.

The test needs root, a writable tracefs and CONFIG_OSNOISE_TRACER. It
skips when any of those are missing. The action-failure case also skips
if the stop-total threshold is not crossed, since an idle machine may
never generate a sample above the limit.

Signed-off-by: Chaosheng Qu <chaosheng.qu.linux@gmail.com>
---
 tools/tracing/rtla/src/common.c               |   2 +
 tools/tracing/rtla/tests/cleanup.t            |  71 +++++++
 .../rtla/tests/scripts/check-cleanup.sh       | 197 ++++++++++++++++++
 3 files changed, 270 insertions(+)
 create mode 100644 tools/tracing/rtla/tests/cleanup.t
 create mode 100755 tools/tracing/rtla/tests/scripts/check-cleanup.sh

diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c
index 8c7f5e75b2ec..b86a600769ed 100644
--- a/tools/tracing/rtla/src/common.c
+++ b/tools/tracing/rtla/src/common.c
@@ -39,6 +39,7 @@ static void stop_trace(int sig)
 static void set_signals(struct common_params *params)
 {
 	signal(SIGINT, stop_trace);
+	signal(SIGTERM, stop_trace);
 	if (params->duration) {
 		signal(SIGALRM, stop_trace);
 		alarm(params->duration);
@@ -51,6 +52,7 @@ static void set_signals(struct common_params *params)
 static void unset_signals(struct common_params *params)
 {
 	signal(SIGINT, SIG_DFL);
+	signal(SIGTERM, SIG_DFL);
 	if (params->duration) {
 		alarm(0);
 		signal(SIGALRM, SIG_DFL);
diff --git a/tools/tracing/rtla/tests/cleanup.t b/tools/tracing/rtla/tests/cleanup.t
new file mode 100644
index 000000000000..f3b495b0aee4
--- /dev/null
+++ b/tools/tracing/rtla/tests/cleanup.t
@@ -0,0 +1,71 @@
+#!/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)
+#   - SIGTERM during a run
+#   - 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.
+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 timeout_cmd skip
+
+	ctr=$((ctr + 1))
+	[ -z "$TEST_COUNT" ] && return
+
+	if [ ! -x "$helper" ]; then
+		echo "not ok $ctr - cleanup helper is not executable"
+		return
+	fi
+
+	# check() normally does this before every test. Do the same here, both to
+	# leave a clean tracer state and to make a hung helper terminate.
+	reset_osnoise || {
+		echo "ok $ctr - instances cleaned up on SIGINT, timeout and action failure # SKIP tracefs is not writable"
+		return
+	}
+	timeout_cmd=${TIMEOUT:-timeout -v -k 15s 2m}
+
+	out=$($timeout_cmd "$helper" "$RTLA" 2>&1)
+	rc=$?
+	if [ $rc -eq 0 ]; then
+		# The helper may skip individual cases; a group-level SKIP means it
+		# could not run at all.
+		skip=$(printf '%s\n' "$out" | sed -n 's/^SKIP: //p' | head -n 1)
+		if [ -n "$skip" ]; then
+			echo "ok $ctr - instances cleaned up on SIGINT, timeout and action failure # SKIP $skip"
+			return
+		fi
+		printf '%s\n' "$out" | grep '^SKIP: ' | while read -r line; do
+			echo "# ${line}"
+		done
+		echo "ok $ctr - instances cleaned up on SIGINT, timeout and action failure"
+	elif [ $rc -eq 124 ] || [ $rc -eq 137 ]; then
+		echo "not ok $ctr - instances cleaned up on SIGINT, timeout and action failure"
+		echo "# helper timed out after 2m"
+	else
+		echo "not ok $ctr - instances cleaned up on SIGINT, timeout and action failure"
+		echo "# helper exited $rc"
+		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..1b4397123708
--- /dev/null
+++ b/tools/tracing/rtla/tests/scripts/check-cleanup.sh
@@ -0,0 +1,197 @@
+#!/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=${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: $*"; }
+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" ]
+}
+
+if [ ! -d "$INSTANCES" ] || [ ! -w "$INSTANCES" ] ||
+   [ ! -d "$TRACEFS/osnoise" ]; then
+	log "SKIP: tracefs, writable instances or CONFIG_OSNOISE_TRACER unavailable"
+	exit 0
+fi
+
+# Leave nothing behind from the test before it starts, and make sure the
+# command under test is actually usable.
+cleanup_all
+if ! "$RTLA" --version >/dev/null 2>&1; then
+	log "SKIP: $RTLA is not usable"
+	exit 0
+fi
+
+# Kill a background rtla if a case fails before reaping it.
+stop_pid() {
+	[ -n "$1" ] || return
+	kill -KILL "$1" 2>/dev/null
+	wait "$1" 2>/dev/null
+}
+
+# Wait for an rtla instance and its signal handler.  The instance appears
+# before set_signals() runs, so wait for rtla to start its workload (or at
+# least for it to survive a short grace period) before sending SIGINT.
+wait_running() {
+	_path=$1
+	_pid=$2
+	wait_present "$_path" 10000 || return 1
+	sleep 1
+	kill -0 "$_pid" 2>/dev/null
+}
+
+# ---------------------------------------------------------------------------
+# 1. SIGINT during a long run
+# ---------------------------------------------------------------------------
+cleanup_all
+$RTLA osnoise top -q -d 30s -P F:1 >/dev/null 2>&1 &
+pid=$!
+if ! wait_running "$INSTANCES/osnoise_top" "$pid"; then
+	fail "SIGINT: instance was never created (rtla did not start?)"
+	stop_pid "$pid"
+else
+	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
+# ---------------------------------------------------------------------------
+cleanup_all
+$RTLA osnoise top -q -d 30s -P F:1 >/dev/null 2>&1 &
+pid=$!
+if ! wait_running "$INSTANCES/osnoise_top" "$pid"; then
+	fail "SIGTERM: instance was never created (rtla did not start)"
+	stop_pid "$pid"
+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"
+	else
+		fail "SIGTERM: instance still present after terminate"
+	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 [ "$rc" -ne 0 ]; then
+	fail "self-timeout: rtla failed to start or run (exit=$rc)"
+elif 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=...)
+# ---------------------------------------------------------------------------
+# -S is the stop-total threshold and yields exit 2 when the action fails.
+# A shell action only runs if that threshold is crossed; an idle machine may
+# never do so.  Do not call the no-threshold case a pass.
+cleanup_all
+action_rc=0
+for n in 1 2 3; do
+	$RTLA osnoise top -q -d 5s -P F:1 -r 900000 -S 1 \
+		--on-threshold 'shell,command="/bin/false"' >/dev/null 2>&1
+	rc=$?
+	action_rc=$rc
+	[ "$rc" -eq 2 ] && break
+done
+if [ "$action_rc" -eq 0 ]; then
+	log "SKIP: action-failure: stop-total threshold was not crossed"
+elif [ "$action_rc" -ne 2 ]; then
+	fail "action-failure: unexpected exit=$action_rc (wanted 2)"
+elif wait_gone "$INSTANCES/osnoise_top"; then
+	pass "action-failure: instance removed (exit=$action_rc)"
+else
+	fail "action-failure: instance still present (exit=$action_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=$!
+	if ! wait_running "$INSTANCES/osnoise_top" "$pid"; then
+		fail "repeated interrupts: run $n never created an instance"
+		stop_pid "$pid"
+		continue
+	fi
+	kill -INT "$pid" 2>/dev/null
+	wait "$pid" 2>/dev/null
+	if ! wait_gone "$INSTANCES/osnoise_top"; then
+		fail "repeated interrupts: run $n left an instance behind"
+		cleanup_all
+	fi
+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"
+[ "$FAIL" -eq 0 ]
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] rtla: subcase SKIP treated as group skip in run_cleanup_cases()
- [Medium] rtla: tests: invalid --version check in check-cleanup.sh skips all tests
--

commit 39ad3784406cff716e91c58adc60f6311f089ea0
Author: Chaosheng Qu <chaosheng.qu.linux@gmail.com>

rtla: add tests for tracefs instance cleanup

This commit adds a SIGTERM handler to rtla so tracefs instances are cleaned
up on termination, along with a new test suite verifying tracefs instance
cleanup across abnormal exit paths.

> diff --git a/tools/tracing/rtla/tests/cleanup.t b/tools/tracing/rtla/tests/cleanup.t
> new file mode 100644
> index 0000000000000..f3b495b0aee4e
> --- /dev/null
> +++ b/tools/tracing/rtla/tests/cleanup.t
> @@ -0,0 +1,71 @@

[ ... ]

> +run_cleanup_cases() {

[ ... ]

> +	out=$($timeout_cmd "$helper" "$RTLA" 2>&1)
> +	rc=$?
> +	if [ $rc -eq 0 ]; then
> +		# The helper may skip individual cases; a group-level SKIP means it
> +		# could not run at all.
> +		skip=$(printf '%s\n' "$out" | sed -n 's/^SKIP: //p' | head -n 1)
> +		if [ -n "$skip" ]; then
> +			echo "ok $ctr - instances cleaned up on SIGINT, timeout and action failure # SKIP $skip"
> +			return
> +		fi
> +		printf '%s\n' "$out" | grep '^SKIP: ' | while read -r line; do
> +			echo "# ${line}"
> +		done
> +		echo "ok $ctr - instances cleaned up on SIGINT, timeout and action failure"

[Severity: Medium]
In run_cleanup_cases(), sed -n 's/^SKIP: //p' matches any line in $out
starting with "SKIP: ". When the action-failure subtest in
tools/tracing/rtla/tests/scripts/check-cleanup.sh does not cross the
stop-total threshold on an idle system, it logs:

	if [ "$action_rc" -eq 0 ]; then
		log "SKIP: action-failure: stop-total threshold was not crossed"

Won't $skip be populated from that subtest skip even when all other subtests
ran and passed, causing run_cleanup_cases() to report the entire TAP test as
skipped and leaving the grep '^SKIP: ' loop right below it unreachable?

[ ... ]

> 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..1b4397123708e
> --- /dev/null
> +++ b/tools/tracing/rtla/tests/scripts/check-cleanup.sh
> @@ -0,0 +1,197 @@

[ ... ]

> +# Leave nothing behind from the test before it starts, and make sure the
> +# command under test is actually usable.
> +cleanup_all
> +if ! "$RTLA" --version >/dev/null 2>&1; then
> +	log "SKIP: $RTLA is not usable"
> +	exit 0
> +fi

[Severity: Medium]
Does rtla support the --version flag? Looking at main() in
tools/tracing/rtla/src/cli.c:

	if (strcmp(argv[1], "-h") == 0)
		rtla_usage(129);
	else if (strcmp(argv[1], "--help") == 0)
		rtla_usage(129);

	retval = run_tool_command(argc, argv, 1);
	if (retval)
		exit(0);

usage:
	rtla_usage(129);

It looks like passing --version falls through to rtla_usage(129) and exits
with status 129. Won't this check always fail and cause check-cleanup.sh to
skip the entire test suite on every run?

[ ... ]

────
To understand more about how the conclusions weredrawn,Normally:
- Stage 1 (goal): VF1
- Stage 2 (implementation): PVF1
────

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