[PATCH v3 15/17] selftests/verification: Fix wrong errexit assumption

[PATCH v3 15/17] selftests/verification: Fix wrong errexit assumption

[Gabriele Monaco]

RV selftest rely on bash errexit (set -e) to terminate with error, when
a step is expected to return false, the following syntax is used:

  ! cmd

This however prevents the test from exiting when cmd is false (desired)
but doesn't exit if cmd is true, since commands prefixed with ! are
explicitly excluded from errexit.

Use the syntax

  ! cmd || false

Which ends up checking the exit value of ! cmd and supplies a false
command for errexit to evaluate.

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 .../verification/test.d/rv_monitor_enable_disable.tc   | 10 +++++-----
 .../verification/test.d/rv_monitor_reactor.tc          |  4 ++--
 .../selftests/verification/test.d/rv_wwnr_printk.tc    |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/verification/test.d/rv_monitor_enable_disable.tc b/tools/testing/selftests/verification/test.d/rv_monitor_enable_disable.tc
index f29236defb..61e2c8b54d 100644
--- a/tools/testing/selftests/verification/test.d/rv_monitor_enable_disable.tc
+++ b/tools/testing/selftests/verification/test.d/rv_monitor_enable_disable.tc
@@ -10,7 +10,7 @@ test_simple_monitor() {
     grep -q "$monitor$" enabled_monitors
 
     echo 0 > "monitors/$prefix$monitor/enable"
-    ! grep -q "$monitor$" enabled_monitors
+    ! grep -q "$monitor$" enabled_monitors || false
 
     echo "$monitor" >> enabled_monitors
     grep -q 1 "monitors/$prefix$monitor/enable"
@@ -34,12 +34,12 @@ test_container_monitor() {
 	test -n "$nested"
 
     echo 0 > "monitors/$monitor/enable"
-    ! grep -q "^$monitor$" enabled_monitors
+    ! grep -q "^$monitor$" enabled_monitors || false
 
     for nested_dir in "monitors/$monitor"/*; do
         [ -d "$nested_dir" ] || continue
         nested=$(basename "$nested_dir")
-        ! grep -q "^$monitor:$nested$" enabled_monitors
+        ! grep -q "^$monitor:$nested$" enabled_monitors || false
     done
 
     echo "$monitor" >> enabled_monitors
@@ -71,5 +71,5 @@ for monitor_dir in monitors/*; do
     fi
 done
 
-! echo non_existent_monitor > enabled_monitors
-! grep -q "^non_existent_monitor$" enabled_monitors
+! echo non_existent_monitor > enabled_monitors || false
+! grep -q "^non_existent_monitor$" enabled_monitors || false
diff --git a/tools/testing/selftests/verification/test.d/rv_monitor_reactor.tc b/tools/testing/selftests/verification/test.d/rv_monitor_reactor.tc
index 2958bf8493..516a209713 100644
--- a/tools/testing/selftests/verification/test.d/rv_monitor_reactor.tc
+++ b/tools/testing/selftests/verification/test.d/rv_monitor_reactor.tc
@@ -64,5 +64,5 @@ done
 
 monitor=$(ls /sys/kernel/tracing/rv/monitors -1 | head -n 1)
 test -f "monitors/$monitor/reactors"
-! echo non_existent_reactor > "monitors/$monitor/reactors"
-! grep -q "\\[non_existent_reactor\\]" "monitors/$monitor/reactors"
+! echo non_existent_reactor > "monitors/$monitor/reactors" || false
+! grep -q "\\[non_existent_reactor\\]" "monitors/$monitor/reactors" || false
diff --git a/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc b/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
index 5a59432b1d..96de95edb5 100644
--- a/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
+++ b/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
@@ -17,13 +17,13 @@ echo printk > monitors/wwnr/reactors
 load
 
 echo 0 > monitoring_on
-! load
+! load || false
 echo 1 > monitoring_on
 
 load
 
 echo 0 > reacting_on
-! load
+! load || false
 echo 1 > reacting_on
 
 echo nop > monitors/wwnr/reactors
-- 
2.54.0

Re: [PATCH v3 15/17] selftests/verification: Fix wrong errexit assumption

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:
> RV selftest rely on bash errexit (set -e) to terminate with error, when
> a step is expected to return false, the following syntax is used:
>
>   ! cmd
>
> This however prevents the test from exiting when cmd is false (desired)
> but doesn't exit if cmd is true, since commands prefixed with ! are
> explicitly excluded from errexit.
>
> Use the syntax
>
>   ! cmd || false
>
> Which ends up checking the exit value of ! cmd and supplies a false
> command for errexit to evaluate.
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>

Sounds reasonable.

Acked-by: Nam Cao <namcao@linutronix.de>

Re: [PATCH v3 15/17] selftests/verification: Fix wrong errexit assumption

[Wen Yang]

On 6/25/26 20:14, Gabriele Monaco wrote:
> RV selftest rely on bash errexit (set -e) to terminate with error, when
> a step is expected to return false, the following syntax is used:
> 
>    ! cmd
> 
> This however prevents the test from exiting when cmd is false (desired)
> but doesn't exit if cmd is true, since commands prefixed with ! are
> explicitly excluded from errexit.
> 
> Use the syntax
> 
>    ! cmd || false
> 
> Which ends up checking the exit value of ! cmd and supplies a false
> command for errexit to evaluate.
> 
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>


Reviewed-by: Wen Yang <wen.yang@linux.dev>


--
Best wishes,
Wen

[PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test

[Gabriele Monaco]

The wwnr_printk test expects no reactions in some situations, after
fixing the bash assertion, the test is failing because expecting no
reaction after a previous step had reactions is flaky without making
sure all buffers are flushed.

Simplify the test and run the steps expecting no reaction before the one
expecting reactions. Also simplify the load function to stop loads as
soon as a reaction occurs, this limits the number of lines to flush and
makes tests overall more stable.

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 .../verification/test.d/rv_wwnr_printk.tc       | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc b/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
index 96de95edb5..a23d22f6ec 100644
--- a/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
+++ b/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
@@ -4,27 +4,30 @@
 # requires: available_reactors wwnr:monitor printk:reactor stress-ng:program
 
 load() { # returns true if there was a reaction
-	local lines_before num
+	local lines_before num load_pid ret
 	num=$((($(nproc) + 1) / 2))
 	lines_before=$(dmesg | wc -l)
-	stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q
-	dmesg | tail -n $((lines_before + 1)) | grep -q "rv: monitor wwnr does not allow event"
+	stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q &
+	load_pid=$!
+	timeout 5 dmesg -w | tail -n +$((lines_before + 1)) | grep -m 1 -q "rv: monitor wwnr does not allow event"
+	ret=$?
+	kill "$load_pid"
+	wait "$load_pid"
+	return $ret
 }
 
 echo 1 > monitors/wwnr/enable
 echo printk > monitors/wwnr/reactors
 
-load
-
 echo 0 > monitoring_on
 ! load || false
 echo 1 > monitoring_on
 
-load
-
 echo 0 > reacting_on
 ! load || false
 echo 1 > reacting_on
 
+load
+
 echo nop > monitors/wwnr/reactors
 echo 0 > monitors/wwnr/enable
-- 
2.54.0

Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test

[Wen Yang]

On 6/25/26 20:14, Gabriele Monaco wrote:
> The wwnr_printk test expects no reactions in some situations, after
> fixing the bash assertion, the test is failing because expecting no
> reaction after a previous step had reactions is flaky without making
> sure all buffers are flushed.
> 
> Simplify the test and run the steps expecting no reaction before the one
> expecting reactions. Also simplify the load function to stop loads as
> soon as a reaction occurs, this limits the number of lines to flush and
> makes tests overall more stable.
> 
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
>   .../verification/test.d/rv_wwnr_printk.tc       | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc b/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
> index 96de95edb5..a23d22f6ec 100644
> --- a/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
> +++ b/tools/testing/selftests/verification/test.d/rv_wwnr_printk.tc
> @@ -4,27 +4,30 @@
>   # requires: available_reactors wwnr:monitor printk:reactor stress-ng:program
>   
>   load() { # returns true if there was a reaction
> -	local lines_before num
> +	local lines_before num load_pid ret
>   	num=$((($(nproc) + 1) / 2))
>   	lines_before=$(dmesg | wc -l)
> -	stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q
> -	dmesg | tail -n $((lines_before + 1)) | grep -q "rv: monitor wwnr does not allow event"
> +	stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q &
> +	load_pid=$!
> +	timeout 5 dmesg -w | tail -n +$((lines_before + 1)) | grep -m 1 -q "rv: monitor wwnr does not allow event"


Minor nit: could we add a small delay (e.g., sleep 0.1) before dmesg?

Reviewed-by: Wen Yang <wen.yang@linux.dev>

> +	ret=$?
> +	kill "$load_pid"
> +	wait "$load_pid"
> +	return $ret
>   }
>   
>   echo 1 > monitors/wwnr/enable
>   echo printk > monitors/wwnr/reactors
>   
> -load
> -
>   echo 0 > monitoring_on
>   ! load || false
>   echo 1 > monitoring_on
>   
> -load
> -
>   echo 0 > reacting_on
>   ! load || false
>   echo 1 > reacting_on
>   
> +load
> +
>   echo nop > monitors/wwnr/reactors
>   echo 0 > monitors/wwnr/enable

Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test

[Gabriele Monaco]

On Fri, 2026-07-10 at 01:08 +0800, Wen Yang wrote:
> >   load() { # returns true if there was a reaction
> > -	local lines_before num
> > +	local lines_before num load_pid ret
> >   	num=$((($(nproc) + 1) / 2))
> >   	lines_before=$(dmesg | wc -l)
> > -	stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q
> > -	dmesg | tail -n $((lines_before + 1)) | grep -q "rv: monitor wwnr
> > does not allow event"
> > +	stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q &
> > +	load_pid=$!
> > +	timeout 5 dmesg -w | tail -n +$((lines_before + 1)) | grep -m 1 -q
> > "rv: monitor wwnr does not allow event"
> 
> 
> Minor nit: could we add a small delay (e.g., sleep 0.1) before dmesg?

I don't get what the benefit would be.

The intent of that line is to run a continuous dmesg (-w) for up to 5s (timeout)
but stopping if an occurrence is found (grep -m 1 would close the pipe).

Do you see a case in which this wouldn't happen?

> 
> Reviewed-by: Wen Yang <wen.yang@linux.dev>
> 
> > +	ret=$?
> > +	kill "$load_pid"
> > +	wait "$load_pid"
> > +	return $ret
> >   }
> >   
> >   echo 1 > monitors/wwnr/enable
> >   echo printk > monitors/wwnr/reactors
> >   
> > -load
> > -
> >   echo 0 > monitoring_on
> >   ! load || false
> >   echo 1 > monitoring_on
> >   
> > -load
> > -
> >   echo 0 > reacting_on
> >   ! load || false
> >   echo 1 > reacting_on
> >   
> > +load
> > +
> >   echo nop > monitors/wwnr/reactors
> >   echo 0 > monitors/wwnr/enable

Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:
> Simplify the test and run the steps expecting no reaction before the one
> expecting reactions.

This requirement is not clear from the code. I fear a new developer who
wants to introduce a new test case will easily trip over this, or even
us a few months from now.

Instead of this, how about each test case discards or ignores all
previous reactions?

Nam

Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test

[Gabriele Monaco]

On Fri, 2026-07-10 at 08:18 +0200, Nam Cao wrote:
> Gabriele Monaco <gmonaco@redhat.com> writes:
> > Simplify the test and run the steps expecting no reaction before the one
> > expecting reactions.
> 
> This requirement is not clear from the code. I fear a new developer who
> wants to introduce a new test case will easily trip over this, or even
> us a few months from now.
> 
> Instead of this, how about each test case discards or ignores all
> previous reactions?

Fair points, the issue is that we are hammering wwnr, on some systems we may
still see printks for long after the monitor was disabled, I tried that grep -m1
trick to mitigate that.

To ignore all previous reactions reliably we need to wait for the buffers to
flush, something like waiting for a full second for nothing new on the
ringbuffer perhaps. I could think of that or at least make this more clear with
comments.

Thanks,
Gabriele

[PATCH v3 17/17] selftests/verification: Add selftests for deadline and stall monitors

[Gabriele Monaco]

Add selftests to verify deadline monitors don't fail under expected
conditions and the stall monitor report violations only when expected.

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 .../verification/test.d/rv_deadline.tc        | 21 ++++++++++++
 .../selftests/verification/test.d/rv_stall.tc | 33 +++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 tools/testing/selftests/verification/test.d/rv_deadline.tc
 create mode 100644 tools/testing/selftests/verification/test.d/rv_stall.tc

diff --git a/tools/testing/selftests/verification/test.d/rv_deadline.tc b/tools/testing/selftests/verification/test.d/rv_deadline.tc
new file mode 100644
index 0000000000..b583096bed
--- /dev/null
+++ b/tools/testing/selftests/verification/test.d/rv_deadline.tc
@@ -0,0 +1,21 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# description: Test deadline monitors trigger no reaction
+# requires: available_reactors deadline:monitor printk:reactor stress-ng:program
+
+load() { # returns true if there was a reaction
+	local lines_before
+	lines_before=$(dmesg | wc -l)
+	stress-ng --cpu 2 --sched deadline --sched-period 100000000 --sched-deadline 100000000 --sched-runtime 20000000 -t 5 &
+	stress-ng --cpu 2 --sched rr --sched-prio 50 --cyclic 1 --cyclic-policy rr --cyclic-prio 50 -t 5 &
+	wait
+	dmesg | tail -n +$((lines_before + 1)) | grep -q "rv: monitor [a-z]\+ does not allow event"
+}
+
+echo 1 > monitors/deadline/enable
+echo printk > monitors/deadline/reactors
+
+! load || false
+
+echo nop > monitors/deadline/reactors
+echo 0 > monitors/deadline/enable
diff --git a/tools/testing/selftests/verification/test.d/rv_stall.tc b/tools/testing/selftests/verification/test.d/rv_stall.tc
new file mode 100644
index 0000000000..8e9bcbb441
--- /dev/null
+++ b/tools/testing/selftests/verification/test.d/rv_stall.tc
@@ -0,0 +1,33 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# description: Test stall monitor
+# requires: available_reactors stall:monitor printk:reactor stress-ng:program
+
+THRESHOLD=/sys/module/stall/parameters/threshold_jiffies
+
+load() { # returns true if there was a reaction
+	local lines_before cpu
+	cpu=$(($(nproc) - 1))
+	lines_before=$(dmesg | wc -l)
+	stress-ng --cpu 1 --taskset "$cpu" --sched rr --sched-prio 1 -t 3 &
+	stress-ng --cpu 5 --taskset "$cpu" -t 3 &
+	wait
+	dmesg | tail -n +$((lines_before + 1)) | grep -q "rv: monitor stall does not allow event"
+}
+
+echo 5000 > $THRESHOLD
+echo 1 > monitors/stall/enable
+echo printk > monitors/stall/reactors
+
+! load || false
+
+echo 0 > monitors/stall/enable
+echo 70 > $THRESHOLD
+echo 1 > monitors/stall/enable
+
+load
+
+echo nop > monitors/stall/reactors
+echo 0 > monitors/stall/enable
+
+echo 1000 > $THRESHOLD
-- 
2.54.0

Re: [PATCH v3 17/17] selftests/verification: Add selftests for deadline and stall monitors

[Wen Yang]

On 6/25/26 20:14, Gabriele Monaco wrote:
> Add selftests to verify deadline monitors don't fail under expected
> conditions and the stall monitor report violations only when expected.
> 
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
>   .../verification/test.d/rv_deadline.tc        | 21 ++++++++++++
>   .../selftests/verification/test.d/rv_stall.tc | 33 +++++++++++++++++++
>   2 files changed, 54 insertions(+)
>   create mode 100644 tools/testing/selftests/verification/test.d/rv_deadline.tc
>   create mode 100644 tools/testing/selftests/verification/test.d/rv_stall.tc
> 
> diff --git a/tools/testing/selftests/verification/test.d/rv_deadline.tc b/tools/testing/selftests/verification/test.d/rv_deadline.tc
> new file mode 100644
> index 0000000000..b583096bed
> --- /dev/null
> +++ b/tools/testing/selftests/verification/test.d/rv_deadline.tc
> @@ -0,0 +1,21 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# description: Test deadline monitors trigger no reaction
> +# requires: available_reactors deadline:monitor printk:reactor stress-ng:program
> +
> +load() { # returns true if there was a reaction
> +	local lines_before
> +	lines_before=$(dmesg | wc -l)
> +	stress-ng --cpu 2 --sched deadline --sched-period 100000000 --sched-deadline 100000000 --sched-runtime 20000000 -t 5 &
> +	stress-ng --cpu 2 --sched rr --sched-prio 50 --cyclic 1 --cyclic-policy rr --cyclic-prio 50 -t 5 &
> +	wait
> +	dmesg | tail -n +$((lines_before + 1)) | grep -q "rv: monitor [a-z]\+ does not allow event"
> +}
> +
> +echo 1 > monitors/deadline/enable
> +echo printk > monitors/deadline/reactors
> +
> +! load || false
> +
> +echo nop > monitors/deadline/reactors
> +echo 0 > monitors/deadline/enable
> diff --git a/tools/testing/selftests/verification/test.d/rv_stall.tc b/tools/testing/selftests/verification/test.d/rv_stall.tc
> new file mode 100644
> index 0000000000..8e9bcbb441
> --- /dev/null
> +++ b/tools/testing/selftests/verification/test.d/rv_stall.tc
> @@ -0,0 +1,33 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# description: Test stall monitor
> +# requires: available_reactors stall:monitor printk:reactor stress-ng:program
> +
> +THRESHOLD=/sys/module/stall/parameters/threshold_jiffies
> +
> +load() { # returns true if there was a reaction
> +	local lines_before cpu
> +	cpu=$(($(nproc) - 1))
> +	lines_before=$(dmesg | wc -l)
> +	stress-ng --cpu 1 --taskset "$cpu" --sched rr --sched-prio 1 -t 3 &
> +	stress-ng --cpu 5 --taskset "$cpu" -t 3 &
> +	wait
> +	dmesg | tail -n +$((lines_before + 1)) | grep -q "rv: monitor stall does not allow event"
> +}
> +
> +echo 5000 > $THRESHOLD
> +echo 1 > monitors/stall/enable
> +echo printk > monitors/stall/reactors
> +
> +! load || false
> +
> +echo 0 > monitors/stall/enable
> +echo 70 > $THRESHOLD
> +echo 1 > monitors/stall/enable
> +
> +load
> +
> +echo nop > monitors/stall/reactors
> +echo 0 > monitors/stall/enable
> +
> +echo 1000 > $THRESHOLD

A little worried:
If the system had a different value configured before the test ran,
this silently changes system state for anything running after it.


--
Best wishes,
Wen

Re: [PATCH v3 17/17] selftests/verification: Add selftests for deadline and stall monitors

[Gabriele Monaco]

On Mon, 2026-06-29 at 00:58 +0800, Wen Yang wrote:
> > +echo nop > monitors/stall/reactors
> > +echo 0 > monitors/stall/enable
> > +
> > +echo 1000 > $THRESHOLD
> 
> A little worried:
> If the system had a different value configured before the test ran,
> this silently changes system state for anything running after it.

Yeah that's a valid concern, I could restore the initial value, but to make it
absolutely safe I should also trap and restore it even if the test exits
prematurely (e.g. on failure or signal).

It shouldn't add too much complexity.

Thanks,
Gabriele