[PATCH v4 0/4] rv/reactors: fix lockdep warning and add KUnit tests

[PATCH v4 0/4] rv/reactors: fix lockdep warning and add KUnit tests

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

We occasionally hit a lockdep "Invalid wait context" warning in
production when a reactor callback is preempted by a timer interrupt.
On interrupt exit the scheduler takes rq->__lock (LD_WAIT_SPIN) while
rv_react() still holds its wait-type-override map, which declared
LD_WAIT_FREE.  On any kernel where the task context has preemption
enabled (not just CONFIG_PREEMPT_RT) this triggers a spurious lockdep
report:

    [ BUG: Invalid wait context ]
    1 lock held by kunit_try_catch/209:
     #0: (rv_react_map-wait-type-override){+.+.}-{1:1}
    kunit_try_catch/209 is trying to lock:
    ffff8a743ed3e8a0 (&rq->__lock){-...}-{2:2}

This series switches to a single LD_WAIT_SPIN override, hardens reactor
registration error handling, exports the registration helpers for
modules, and adds KUnit coverage.

Changes in v4:
  - Patch 1: replace the context-sensitive two-map wait type with a
    single LD_WAIT_SPIN override, and document the constraint.
  - Patch 4: use a plain int instead of atomic_t for the test counter,
    trim redundant comments.

v3: https://lore.kernel.org/lkml/cover.1786294920.git.wen.yang@linux.dev/
v2: https://lore.kernel.org/lkml/cover.1785695669.git.wen.yang@linux.dev/
v1: https://lore.kernel.org/lkml/cover.1781541556.git.wen.yang@linux.dev/

Wen Yang (4):
  rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
  rv/reactors: propagate rv_register_reactor() error from reactor init
  rv/reactors: export rv_register_reactor() and rv_unregister_reactor()
  rv/reactors: add KUnit tests for reactor registration and dispatch

 Documentation/trace/rv/monitor_synthesis.rst |  20 ++++
 kernel/trace/rv/Kconfig                      |  12 +++
 kernel/trace/rv/Makefile                     |   1 +
 kernel/trace/rv/reactor_panic.c              |   3 +-
 kernel/trace/rv/reactor_printk.c             |   3 +-
 kernel/trace/rv/rv_reactors.c                |   8 +-
 kernel/trace/rv/rv_reactors_kunit.c          | 105 +++++++++++++++++++
 7 files changed, 147 insertions(+), 5 deletions(-)
 create mode 100644 kernel/trace/rv/rv_reactors_kunit.c

base-commit: 785095112f4198de49760552374f364043c8dbdf
-- 
2.25.1

[PATCH v4 1/4] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

rv_react() overrides the lockdep wait type to LD_WAIT_FREE to enforce
that reactor callbacks take no locks. But callbacks run in the context
of the triggering tracepoint, which can be preemptible task context on
any kernel. A timer interrupt firing during the callback makes the
interrupt-exit path schedule and take rq->__lock (LD_WAIT_SPIN) while
the LD_WAIT_FREE override is still held, producing a spurious
"Invalid wait context" warning:

    [ BUG: Invalid wait context ]
    context-{5:5}
    1 lock held by kunit_try_catch/209:
     #0: (rv_react_map-wait-type-override){+.+.}-{1:1}
    kunit_try_catch/209 is trying to lock:
    ffff8a743ed3e8a0 (&rq->__lock){-...}-{2:2}

Use LD_WAIT_SPIN instead of LD_WAIT_FREE, which causes false-positive
warnings in preemptible contexts due to scheduler preemption taking
rq->__lock. Add documentation to runtime-verification.rst.

Fixes: 69d8895cb9a9 ("rv: Add explicit lockdep context for reactors")
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 Documentation/trace/rv/monitor_synthesis.rst | 20 ++++++++++++++++++++
 kernel/trace/rv/rv_reactors.c                |  6 +++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/rv/monitor_synthesis.rst b/Documentation/trace/rv/monitor_synthesis.rst
index 2c1b5a0ae154..aab4b0342d5f 100644
--- a/Documentation/trace/rv/monitor_synthesis.rst
+++ b/Documentation/trace/rv/monitor_synthesis.rst
@@ -365,6 +365,26 @@ but higher overhead. The timer wheel (``HA_TIMER_WHEEL``) is a good alternative
 for monitors with several instances (e.g. per-task) that achieves lower
 overhead with increased latency, yet without compromising precision.
 
+Reactors
+--------
+
+A reactor is a callback triggered by a monitor when a violation is
+detected. Reactors are registered via ``/sys/kernel/tracing/rv/reactors/``
+and enabled per monitor.
+
+Reactor Locking Rules
++++++++++++++++++++++
+
+A reactor callback may be invoked from various contexts (process,
+softirq, hardirq, NMI) depending on the tracepoint to which its
+monitor is attached.
+
+Lockdep uses a fixed wait type: ``LD_WAIT_SPIN``. This allows
+``raw_spinlock_t`` but disallows sleepable locks. ``LD_WAIT_FREE`` is
+not viable in preemptible contexts because scheduler preemption takes
+``rq->__lock`` (``LD_WAIT_SPIN``), which would cause false-positive
+warnings.
+
 Final remarks
 -------------
 
diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index 2f5fc8d18dea..afc97d097109 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -465,7 +465,11 @@ int init_rv_reactors(struct dentry *root_dir)
 
 void rv_react(struct rv_monitor *monitor, const char *msg, ...)
 {
-	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE);
+	/*
+	 * Use LD_WAIT_SPIN uniformly for deterministic lockdep checking.
+	 * See Documentation/trace/rv/runtime-verification.rst.
+	 */
+	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_SPIN);
 	va_list args;
 
 	if (!rv_reacting_on() || !monitor->react)
-- 
2.25.1

Re: [PATCH v4 1/4] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The documentation for reactor locking rules was added to the wrong file (`monitor_synthesis.rst` instead of `runtime-verification.rst`), causing an inconsistency between the commit message, source code comments, and the actual documentation.
--

commit 26073218a7c3b83225869704d414a262b5e15e47
Author: Wen Yang <wen.yang@linux.dev>

rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type

This commit changes the lockdep wait type override in rv_react() from
LD_WAIT_FREE to LD_WAIT_SPIN to prevent spurious "Invalid wait context"
warnings. It also adds documentation explaining the reactor locking rules.

> diff --git a/Documentation/trace/rv/monitor_synthesis.rst b/Documentation/trace/rv/monitor_synthesis.rst
> index 2c1b5a0ae1545..aab4b0342d5f5 100644
> --- a/Documentation/trace/rv/monitor_synthesis.rst
> +++ b/Documentation/trace/rv/monitor_synthesis.rst
> @@ -365,6 +365,26 @@ but higher overhead. The timer wheel (``HA_TIMER_WHEEL``) is a good alternative
>  for monitors with several instances (e.g. per-task) that achieves lower
>  overhead with increased latency, yet without compromising precision.
>  
> +Reactors
> +--------
> +
> +A reactor is a callback triggered by a monitor when a violation is
> +detected. Reactors are registered via ``/sys/kernel/tracing/rv/reactors/``
> +and enabled per monitor.
> +
> +Reactor Locking Rules
> +++++++++++++++++++++

[ ... ]

> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index 2f5fc8d18dea4..afc97d0971091 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -465,7 +465,11 @@ int init_rv_reactors(struct dentry *root_dir)
>  
>  void rv_react(struct rv_monitor *monitor, const char *msg, ...)
>  {
> -	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE);
> +	/*
> +	 * Use LD_WAIT_SPIN uniformly for deterministic lockdep checking.
> +	 * See Documentation/trace/rv/runtime-verification.rst.
> +	 */

[Severity: Low]
Is the documentation for reactor locking rules added to the wrong file?

The comment added here in rv_react() points to
Documentation/trace/rv/runtime-verification.rst for the explanation of the
LD_WAIT_SPIN override. However, the documentation was actually added to
Documentation/trace/rv/monitor_synthesis.rst.

Could the comment or the file location be updated so readers following the
code comment will look in the correct documentation file?

> +	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_SPIN);
>  	va_list args;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787854397.git.wen.yang@linux.dev?part=1

Re: [PATCH v4 1/4] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type

[Gabriele Monaco]

On Fri, 2026-08-28 at 02:22 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
> 
> diff --git a/Documentation/trace/rv/monitor_synthesis.rst
> b/Documentation/trace/rv/monitor_synthesis.rst
> index 2c1b5a0ae154..aab4b0342d5f 100644
> --- a/Documentation/trace/rv/monitor_synthesis.rst
> +++ b/Documentation/trace/rv/monitor_synthesis.rst
> @@ -365,6 +365,26 @@ but higher overhead. The timer wheel (``HA_TIMER_WHEEL``)
> is a good alternative
>  for monitors with several instances (e.g. per-task) that achieves lower
>  overhead with increased latency, yet without compromising precision.
>  
> +Reactors
> +--------
> +
> +A reactor is a callback triggered by a monitor when a violation is
> +detected. Reactors are registered via ``/sys/kernel/tracing/rv/reactors/``
> +and enabled per monitor.

This is not correct, they stay in
/sys/kernel/tracing/rv/monitors/MONITOR/reactors and it is already explained in
Documentation/trace/rv/runtime-verification.rst .

You don't need to add any further section here (you're more than welcome to
update the existing one obviously, but not as part of this patch/series).

monitor_synthesis.rst is not about reactors.
At the moment we don't really have a good spot under Documentation to describe
the /implementation/ of reactors.

I'd say for the purpose of this patch, in-code documentation (i.e. a comment) is
sufficient, we can revise this later if needed.

> +
> +Reactor Locking Rules
> ++++++++++++++++++++++
> +
> +A reactor callback may be invoked from various contexts (process,
> +softirq, hardirq, NMI) depending on the tracepoint to which its
> +monitor is attached.
> +
> +Lockdep uses a fixed wait type: ``LD_WAIT_SPIN``. This allows
> +``raw_spinlock_t`` but disallows sleepable locks. ``LD_WAIT_FREE`` is
> +not viable in preemptible contexts because scheduler preemption takes
> +``rq->__lock`` (``LD_WAIT_SPIN``), which would cause false-positive
> +warnings.
> +

The code looks fine but this documentation is missing the point. We don't need
to explain what lockdep does but why we use a specific class and what we
actually wanted to use. Let's readapt the comment you added in v3

	/*
	 * Reactors must not explicitly take locks, so they should be
	 * LD_WAIT_FREE.  However, reactor callbacks can run with preemption
	 * enabled, meaning the preempting code (e.g. the scheduler taking
	 * rq->__lock at LD_WAIT_SPIN) may violate that constraint.  Use
	 * LD_WAIT_SPIN to avoid false-positive lockdep reports.
         * But you should still NOT be using locks in reactors.
	 */

>  Final remarks
>  -------------
>  
> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index 2f5fc8d18dea..afc97d097109 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -465,7 +465,11 @@ int init_rv_reactors(struct dentry *root_dir)
>  
>  void rv_react(struct rv_monitor *monitor, const char *msg, ...)
>  {
> -	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE);
> +	/*
> +	 * Use LD_WAIT_SPIN uniformly for deterministic lockdep checking.
> +	 * See Documentation/trace/rv/runtime-verification.rst.
> +	 */

See above, I would only add an explicit comment here of /why/ we do LD_WAIT_SPIN
although we would like LD_WAIT_FREE.

Thanks,
Gabriele

> +	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_SPIN);
>  	va_list args;
>  
>  	if (!rv_reacting_on() || !monitor->react)

[PATCH v4 2/4] rv/reactors: propagate rv_register_reactor() error from reactor init

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

Both register_react_printk() and register_react_panic() ignore the
return value of rv_register_reactor() and always return 0.  If the
registration fails (e.g. a duplicate reactor name), the init functions
silently report success even though the reactor was not registered.

Propagate the error from rv_register_reactor() so a failed registration
is reported instead of being silently ignored.

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
 kernel/trace/rv/reactor_panic.c  | 3 +--
 kernel/trace/rv/reactor_printk.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/rv/reactor_panic.c b/kernel/trace/rv/reactor_panic.c
index 76537b8a4343..db7116ceafff 100644
--- a/kernel/trace/rv/reactor_panic.c
+++ b/kernel/trace/rv/reactor_panic.c
@@ -26,8 +26,7 @@ static struct rv_reactor rv_panic = {
 
 static int __init register_react_panic(void)
 {
-	rv_register_reactor(&rv_panic);
-	return 0;
+	return rv_register_reactor(&rv_panic);
 }
 
 static void __exit unregister_react_panic(void)
diff --git a/kernel/trace/rv/reactor_printk.c b/kernel/trace/rv/reactor_printk.c
index 48c934e315b3..002a10f6aa7b 100644
--- a/kernel/trace/rv/reactor_printk.c
+++ b/kernel/trace/rv/reactor_printk.c
@@ -25,8 +25,7 @@ static struct rv_reactor rv_printk = {
 
 static int __init register_react_printk(void)
 {
-	rv_register_reactor(&rv_printk);
-	return 0;
+	return rv_register_reactor(&rv_printk);
 }
 
 static void __exit unregister_react_printk(void)
-- 
2.25.1

[PATCH v4 3/4] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

rv_react() is exported to modules, but the reactor registration helpers
are not.  Export them with EXPORT_SYMBOL_GPL() so reactor modules and
the tristate KUnit test module can register and unregister reactors
without hitting undefined symbol errors at link time(modpost).

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
 kernel/trace/rv/rv_reactors.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index afc97d097109..606c530de2f0 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -314,6 +314,7 @@ int rv_register_reactor(struct rv_reactor *reactor)
 	guard(mutex)(&rv_interface_lock);
 	return __rv_register_reactor(reactor);
 }
+EXPORT_SYMBOL_GPL(rv_register_reactor);
 
 /**
  * rv_unregister_reactor - unregister a rv reactor.
@@ -327,6 +328,7 @@ int rv_unregister_reactor(struct rv_reactor *reactor)
 	list_del(&reactor->list);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(rv_unregister_reactor);
 
 /*
  * reacting_on interface.
-- 
2.25.1

Re: [PATCH v4 3/4] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Exporting `rv_register_reactor()` and `rv_unregister_reactor()` allows dynamically loaded modules to register reactors. However, `rv_unregister_reactor()` fails to detach the reactor from monitors currently using it, and `struct rv_reactor` lacks an `owner` field to pin the module in memory. This leads to a dangling pointer and use-after-free when a reactor module is unloaded.
--

commit f153a63763986ecb001f784d45c5262d45c8d0a5
Author: Wen Yang <wen.yang@linux.dev>
Subject: rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

This commit exports rv_register_reactor() and rv_unregister_reactor() so
that dynamically loaded modules and KUnit tests can register and unregister
reactors without link-time errors.

> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index afc97d0971091..606c530de2f04 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -314,6 +314,7 @@ int rv_register_reactor(struct rv_reactor *reactor)
>  	guard(mutex)(&rv_interface_lock);
>  	return __rv_register_reactor(reactor);
>  }
> +EXPORT_SYMBOL_GPL(rv_register_reactor);

[ ... ]

> @@ -327,6 +328,7 @@ int rv_unregister_reactor(struct rv_reactor *reactor)
>  	list_del(&reactor->list);
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(rv_unregister_reactor);

[Severity: High]
By exporting these functions, dynamic modules can now register and unregister
reactors. However, when a module is unloaded and calls rv_unregister_reactor(),
does this leave dangling pointers in monitors that are currently using it?

Looking at how monitors assign reactors, struct rv_reactor lacks an owner
field to pin the module in memory:

kernel/trace/rv/rv_reactors.c:monitor_swap_reactors_single() {
    ...
	mon->reactor = reactor;
	mon->react = reactor->react;
    ...
}

Since rv_unregister_reactor() only removes the reactor from the global list
without clearing active monitor references, won't rv_react() later execute a
dangling pointer when an event triggers?

kernel/trace/rv/rv_reactors.c:rv_react() {
    ...
	monitor->react(msg, args);
    ...
}

Could this result in a use-after-free pointing to unmapped module memory if
an authorized user unloads a reactor module that is still attached to an
active monitor?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787854397.git.wen.yang@linux.dev?part=3

[PATCH v4 4/4] rv/reactors: add KUnit tests for reactor registration and dispatch

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

Add KUnit tests covering the reactor register/unregister lifecycle
(including duplicate and name-length rejection) and rv_react() dispatch
(a no-op without a callback, exactly one invocation with one).  The
mdelay() callback keeps the CPU busy so a timer interrupt lands inside
rv_react()'s lockdep context, exercising the LD_WAIT_SPIN wait type
from the previous patch; a spurious lockdep splat there would show up
in the test output.

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
 kernel/trace/rv/Kconfig             |  12 ++++
 kernel/trace/rv/Makefile            |   1 +
 kernel/trace/rv/rv_reactors_kunit.c | 105 ++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 kernel/trace/rv/rv_reactors_kunit.c

diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
index efa930f94ea4..9bfd429ffdea 100644
--- a/kernel/trace/rv/Kconfig
+++ b/kernel/trace/rv/Kconfig
@@ -113,6 +113,18 @@ config RV_REACT_PANIC
 	  Enables the panic reactor. The panic reactor emits a printk()
 	  message if an exception is found and panic()s the system.
 
+config RV_REACTORS_KUNIT
+	tristate "KUnit tests for RV reactors" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	depends on RV_REACTORS
+	default KUNIT_ALL_TESTS
+	help
+	  Enable KUnit tests for RV reactor registration and dispatch.
+	  These tests verify the register/unregister lifecycle, duplicate
+	  rejection, and that rv_react() correctly invokes callbacks.
+
+	  If unsure, say N.
+
 config RV_MONITORS_KUNIT_TEST
 	tristate "KUnit tests for RV monitors" if !KUNIT_ALL_TESTS
 	depends on KUNIT && RV && RV_REACTORS
diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
index cdbf68c84f5a..c895d81dfdad 100644
--- a/kernel/trace/rv/Makefile
+++ b/kernel/trace/rv/Makefile
@@ -25,4 +25,5 @@ obj-$(CONFIG_RV_MON_WAKEUP) += monitors/wakeup/wakeup.o
 obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
 obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
 obj-$(CONFIG_RV_REACT_PANIC) += reactor_panic.o
+obj-$(CONFIG_RV_REACTORS_KUNIT) += rv_reactors_kunit.o
 obj-$(CONFIG_RV_MONITORS_KUNIT_TEST) += rv_monitors_test.o
diff --git a/kernel/trace/rv/rv_reactors_kunit.c b/kernel/trace/rv/rv_reactors_kunit.c
new file mode 100644
index 000000000000..2ccbe55ce408
--- /dev/null
+++ b/kernel/trace/rv/rv_reactors_kunit.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for RV reactor registration and dispatch.
+ */
+
+#include <kunit/test.h>
+#include <linux/rv.h>
+#include <linux/delay.h>
+#include "rv.h"
+
+static struct rv_reactor test_reactor = {
+	.name		= "kunit_test_reactor",
+	.description	= "KUnit test reactor",
+};
+
+static void reactor_teardown(void *arg)
+{
+	rv_unregister_reactor(&test_reactor);
+}
+
+static void register_test_reactor(struct kunit *test)
+{
+	KUNIT_ASSERT_EQ(test, rv_register_reactor(&test_reactor), 0);
+	KUNIT_ASSERT_EQ(test,
+			kunit_add_action_or_reset(test, reactor_teardown, NULL), 0);
+}
+
+static void test_double_register(struct kunit *test)
+{
+	register_test_reactor(test);
+	KUNIT_EXPECT_EQ(test, rv_register_reactor(&test_reactor), -EINVAL);
+}
+
+static const char long_reactor_name[] = "kunit_reactor_name_too_long_xxx_";
+_Static_assert(sizeof(long_reactor_name) - 1 >= MAX_RV_REACTOR_NAME_SIZE,
+	       "long_reactor_name must be at least MAX_RV_REACTOR_NAME_SIZE chars");
+
+static void test_name_too_long(struct kunit *test)
+{
+	static struct rv_reactor long_reactor = {
+		.name = long_reactor_name,
+	};
+
+	KUNIT_EXPECT_EQ(test, rv_register_reactor(&long_reactor), -EINVAL);
+}
+
+static struct kunit_case rv_reactor_registration_cases[] = {
+	KUNIT_CASE(test_double_register),
+	KUNIT_CASE(test_name_too_long),
+	{}
+};
+
+static struct kunit_suite rv_reactor_registration_suite = {
+	.name		= "rv_reactor_registration",
+	.test_cases	= rv_reactor_registration_cases,
+};
+
+static int react_call_count;
+
+__printf(1, 0) static void mock_react(const char *msg, va_list args)
+{
+	react_call_count++;
+	/* Busy-wait so a timer interrupt fires inside rv_react(). */
+	mdelay(5);
+}
+
+static void test_react_no_callback(struct kunit *test)
+{
+	struct rv_monitor monitor = {
+		.name = "kunit_null_react",
+	};
+
+	react_call_count = 0;
+	rv_react(&monitor, "no callback");
+
+	KUNIT_EXPECT_EQ(test, react_call_count, 0);
+}
+
+static void test_react_callback_invoked(struct kunit *test)
+{
+	struct rv_monitor monitor = {
+		.name	= "kunit_dispatch_monitor",
+		.react	= mock_react,
+	};
+
+	react_call_count = 0;
+	rv_react(&monitor, "callback invocation test");
+	KUNIT_EXPECT_EQ(test, react_call_count, 1);
+}
+
+static struct kunit_case rv_react_dispatch_cases[] = {
+	KUNIT_CASE(test_react_no_callback),
+	KUNIT_CASE(test_react_callback_invoked),
+	{}
+};
+
+static struct kunit_suite rv_react_dispatch_suite = {
+	.name		= "rv_react_dispatch",
+	.test_cases	= rv_react_dispatch_cases,
+};
+
+kunit_test_suites(&rv_reactor_registration_suite, &rv_react_dispatch_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("KUnit tests for RV reactor registration and dispatch");
-- 
2.25.1

Re: [PATCH v4 4/4] rv/reactors: add KUnit tests for reactor registration and dispatch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test `test_react_callback_invoked` spuriously fails if the global reactor dispatch is disabled via sysfs.
- [Medium] The 5ms busy-wait in `mock_react` is mathematically insufficient to guarantee a timer interrupt on systems with HZ < 200, making the intended lockdep coverage flaky.
--

commit 2f80d797b5152331b6b88a29924dc870e4e2b6a0
Author: Wen Yang <wen.yang@linux.dev>

rv/reactors: add KUnit tests for reactor registration and dispatch

This commit adds KUnit tests for the RV reactor register/unregister lifecycle.
It includes coverage for duplicate registration, name-length rejection, and
verifying that rv_react() correctly dispatches to the callback.

> diff --git a/kernel/trace/rv/rv_reactors_kunit.c b/kernel/trace/rv/rv_reactors_kunit.c
> new file mode 100644
> index 0000000000000..2ccbe55ce4087
> --- /dev/null
> +++ b/kernel/trace/rv/rv_reactors_kunit.c

[ ... ]

> +__printf(1, 0) static void mock_react(const char *msg, va_list args)
> +{
> +	react_call_count++;
> +	/* Busy-wait so a timer interrupt fires inside rv_react(). */
> +	mdelay(5);

[Severity: Medium]
Will this 5ms busy-wait reliably trigger a timer interrupt on systems where
HZ < 200?

On systems configured with HZ=100, where ticks occur every 10ms, a 5ms delay
will only span a timer tick around half the time. Can this lead to flaky test
coverage when trying to exercise the LD_WAIT_SPIN lockdep wait context?

> +}

[ ... ]

> +static void test_react_callback_invoked(struct kunit *test)
> +{
> +	struct rv_monitor monitor = {
> +		.name	= "kunit_dispatch_monitor",
> +		.react	= mock_react,
> +	};
> +
> +	react_call_count = 0;
> +	rv_react(&monitor, "callback invocation test");
> +	KUNIT_EXPECT_EQ(test, react_call_count, 1);

[Severity: Medium]
Can this assertion spuriously fail if the global reacting_on state is
disabled?

If the test executes while the global state is disabled, such as via
/sys/kernel/tracing/rv/reacting_on, rv_react() short-circuits:

kernel/trace/rv/rv_reactors.c:rv_react() {
	...
	if (!rv_reacting_on() || !monitor->react)
		return;
	...
}

Will this cause the test to fail by not invoking the callback at all?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787854397.git.wen.yang@linux.dev?part=4