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

[PATCH v3 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}

Changes in v3:

Patch 1:
- Rewrite commit message to clarify that the bug is not
  PREEMPT_RT-specific: any preemptible task context can be interrupted
  by a timer, and the scheduler acquires rq->__lock (LD_WAIT_SPIN) on
  interrupt exit while rv_react() still holds its LD_WAIT_FREE override
  map.
- Rewrite the inline comment in rv_react() to make clear.

Patch 3:
- Fix commit message: missing EXPORT_SYMBOL_GPL() is caught by modpost
  at link time, not at load time.

Patch 4:
- Drop test_reactor_registered bool and the unregister_test_reactor()
  wrapper entirely.
- Remove test_register_unregister(); the teardown already covers the
  unregister path, and test_double_register() exercises the full
  register->verify->unregister-via-teardown flow.
- Replace the runtime KUNIT_ASSERT_EQ(strlen(...)) guard in
  test_name_too_long() with a _Static_assert on the name array size.
- In test_react_no_callback(): reuse the existing react_call_count
  atomic to assert that the callback was never invoked.
  This catches a broader class of regressions — e.g. 
  an inverted or missing NULL guard that does not cause a NULL dereference.

No functional change to patches 2-3; the series structure is unchanged.

Tested with CONFIG_PROVE_LOCKING=y and CONFIG_KUNIT=y.

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 context-sensitive lockdep wait type in rv_react()
  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

 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       |  22 +++--
 kernel/trace/rv/rv_reactors_kunit.c | 119 ++++++++++++++++++++++++++++
 6 files changed, 151 insertions(+), 9 deletions(-)
 create mode 100644 kernel/trace/rv/rv_reactors_kunit.c

base-commit: 785095112f4198de49760552374f364043c8dbdf
-- 
2.25.1

[PATCH v3 1/4] rv/reactors: use context-sensitive lockdep wait type in rv_react()

[wen.yang]

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

Reactors must not explicitly take locks, so they should comply with
LD_WAIT_FREE.  However, reactor callbacks can run with preemption
enabled on any kernel (not just PREEMPT_RT).  If a timer interrupt
fires during the callback, the interrupt exit path schedules and
acquires rq->__lock (LD_WAIT_SPIN) while the lockdep override map that
declared LD_WAIT_FREE is still held, triggering 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}

Fixes: 69d8895cb9a9 ("rv: Add explicit lockdep context for reactors")
Suggested-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 kernel/trace/rv/rv_reactors.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index 2f5fc8d18dea..5830229210e8 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -465,18 +465,28 @@ 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);
+	/*
+	 * 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 in preemptible contexts to avoid false-positive lockdep
+	 * reports; keep LD_WAIT_FREE in NMI/hardirq where preemption is
+	 * disabled and the scheduler cannot run.
+	 */
+	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map,        LD_WAIT_SPIN);
+	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map_atomic, LD_WAIT_FREE);
+	struct lockdep_map * __maybe_unused map;
 	va_list args;
 
 	if (!rv_reacting_on() || !monitor->react)
 		return;
 
+	map = (in_nmi() || in_hardirq()) ? &rv_react_map_atomic : &rv_react_map;
 	va_start(args, msg);
-
-	lock_map_acquire_try(&rv_react_map);
+	lock_map_acquire_try(map);
 	monitor->react(msg, args);
-	lock_map_release(&rv_react_map);
-
+	lock_map_release(map);
 	va_end(args);
 }
 EXPORT_SYMBOL_GPL(rv_react);
-- 
2.25.1

[PATCH v3 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.

Suggested-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
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 v3 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 5830229210e8..5c85e3efe42e 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

[PATCH v3 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 exercises the LD_WAIT_SPIN
lockdep context).  The Kconfig entry is tristate so the tests can be built
as a module when CONFIG_KUNIT=m; only RV_REACTORS is required.

Suggested-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 | 119 ++++++++++++++++++++++++++++
 3 files changed, 132 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..d5b731eb11f3
--- /dev/null
+++ b/kernel/trace/rv/rv_reactors_kunit.c
@@ -0,0 +1,119 @@
+// 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 <linux/atomic.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);
+}
+
+/*
+ * Use a fixed-size array so sizeof() gives the exact byte count at
+ * compile time.
+ */
+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 atomic_t react_call_count;
+
+__printf(1, 0) static void mock_react(const char *msg, va_list args)
+{
+	atomic_inc(&react_call_count);
+	/*
+	 * Hold the CPU for 5 ms so a timer interrupt is likely to fire
+	 * inside rv_react()'s lockdep context, exercising the LD_WAIT_SPIN
+	 * constraint.  mdelay() is a calibrated busy-wait with no scheduler
+	 * interaction.
+	 */
+	mdelay(5);
+}
+
+static void test_react_no_callback(struct kunit *test)
+{
+	struct rv_monitor monitor = {
+		.name = "kunit_null_react",
+	};
+
+	atomic_set(&react_call_count, 0);
+	rv_react(&monitor, "no callback");
+
+	/*
+	 * The only possible failure in this test case is a kernel panic.
+	 * NULL react guard: callback must NOT have been invoked
+	 */
+	KUNIT_EXPECT_EQ(test, atomic_read(&react_call_count), 0);
+}
+
+static void test_react_callback_invoked(struct kunit *test)
+{
+	struct rv_monitor monitor = {
+		.name	= "kunit_dispatch_monitor",
+		.react	= mock_react,
+	};
+
+	atomic_set(&react_call_count, 0);
+	rv_react(&monitor, "callback invocation test");
+	KUNIT_EXPECT_EQ(test, atomic_read(&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 v3 1/4] rv/reactors: use context-sensitive lockdep wait type in rv_react()

[Gabriele Monaco]

On Mon, 2026-08-10 at 01:10 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
> 
> Reactors must not explicitly take locks, so they should comply with
> LD_WAIT_FREE.  However, reactor callbacks can run with preemption
> enabled on any kernel (not just PREEMPT_RT).  If a timer interrupt
> fires during the callback, the interrupt exit path schedules and
> acquires rq->__lock (LD_WAIT_SPIN) while the lockdep override map that
> declared LD_WAIT_FREE is still held, triggering 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}
> 
> Fixes: 69d8895cb9a9 ("rv: Add explicit lockdep context for reactors")
> Suggested-by: Gabriele Monaco <gmonaco@redhat.com>

You came up with the idea, I at most suggested how I'd like it worded in the
comment.

Anyway, I'd appreciate comments/acks from the other folks in the loop, but from
my side this looks good.

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>

Thanks,
Gabriele

> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  kernel/trace/rv/rv_reactors.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index 2f5fc8d18dea..5830229210e8 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -465,18 +465,28 @@ 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);
> +	/*
> +	 * 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 in preemptible contexts to avoid false-positive
> lockdep
> +	 * reports; keep LD_WAIT_FREE in NMI/hardirq where preemption is
> +	 * disabled and the scheduler cannot run.
> +	 */
> +	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map,        LD_WAIT_SPIN);
> +	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map_atomic, LD_WAIT_FREE);
> +	struct lockdep_map * __maybe_unused map;
>  	va_list args;
>  
>  	if (!rv_reacting_on() || !monitor->react)
>  		return;
>  
> +	map = (in_nmi() || in_hardirq()) ? &rv_react_map_atomic :
> &rv_react_map;
>  	va_start(args, msg);
> -
> -	lock_map_acquire_try(&rv_react_map);
> +	lock_map_acquire_try(map);
>  	monitor->react(msg, args);
> -	lock_map_release(&rv_react_map);
> -
> +	lock_map_release(map);
>  	va_end(args);
>  }
>  EXPORT_SYMBOL_GPL(rv_react);

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

[Gabriele Monaco]

On Mon, 2026-08-10 at 01:10 +0800, wen.yang@linux.dev wrote:
> 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 exercises the LD_WAIT_SPIN
> lockdep context).  The Kconfig entry is tristate so the tests can be built
> as a module when CONFIG_KUNIT=m; only RV_REACTORS is required.
> 

Looks simple and good now.

> Suggested-by: Gabriele Monaco <gmonaco@redhat.com>

Same as before, you can leave only

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>

Thanks
Gabriele

> 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 | 119 ++++++++++++++++++++++++++++
>  3 files changed, 132 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..d5b731eb11f3
> --- /dev/null
> +++ b/kernel/trace/rv/rv_reactors_kunit.c
> @@ -0,0 +1,119 @@
> +// 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 <linux/atomic.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);
> +}
> +
> +/*
> + * Use a fixed-size array so sizeof() gives the exact byte count at
> + * compile time.
> + */
> +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 atomic_t react_call_count;
> +
> +__printf(1, 0) static void mock_react(const char *msg, va_list args)
> +{
> +	atomic_inc(&react_call_count);
> +	/*
> +	 * Hold the CPU for 5 ms so a timer interrupt is likely to fire
> +	 * inside rv_react()'s lockdep context, exercising the LD_WAIT_SPIN
> +	 * constraint.  mdelay() is a calibrated busy-wait with no scheduler
> +	 * interaction.
> +	 */
> +	mdelay(5);
> +}
> +
> +static void test_react_no_callback(struct kunit *test)
> +{
> +	struct rv_monitor monitor = {
> +		.name = "kunit_null_react",
> +	};
> +
> +	atomic_set(&react_call_count, 0);
> +	rv_react(&monitor, "no callback");
> +
> +	/*
> +	 * The only possible failure in this test case is a kernel panic.
> +	 * NULL react guard: callback must NOT have been invoked
> +	 */
> +	KUNIT_EXPECT_EQ(test, atomic_read(&react_call_count), 0);
> +}
> +
> +static void test_react_callback_invoked(struct kunit *test)
> +{
> +	struct rv_monitor monitor = {
> +		.name	= "kunit_dispatch_monitor",
> +		.react	= mock_react,
> +	};
> +
> +	atomic_set(&react_call_count, 0);
> +	rv_react(&monitor, "callback invocation test");
> +	KUNIT_EXPECT_EQ(test, atomic_read(&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");

Re: [PATCH v3 1/4] rv/reactors: use context-sensitive lockdep wait type in rv_react()

[Nam Cao]

Gabriele Monaco <gmonaco@redhat.com> writes:

> On Mon, 2026-08-10 at 01:10 +0800, wen.yang@linux.dev wrote:
>> From: Wen Yang <wen.yang@linux.dev>
>> 
>> Reactors must not explicitly take locks, so they should comply with
>> LD_WAIT_FREE.  However, reactor callbacks can run with preemption
>> enabled on any kernel (not just PREEMPT_RT).  If a timer interrupt
>> fires during the callback, the interrupt exit path schedules and
>> acquires rq->__lock (LD_WAIT_SPIN) while the lockdep override map that
>> declared LD_WAIT_FREE is still held, triggering a spurious
>> "Invalid wait context" warning:
...
> Anyway, I'd appreciate comments/acks from the other folks in the loop

Sorry, I do not know enough about lockdep to comment on this.

FWIW, I would rather just use LD_WAIT_SPIN and keep things
simple. Context-sensitive code paths "feels wrong" to me. Spinning
should either be allowed or forbidden. Making it dynamic "feels like" it
will bring further complications down the road.

But that's just my intuition.

Nam

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

[Nam Cao]

wen.yang@linux.dev writes:
> +/*
> + * Use a fixed-size array so sizeof() gives the exact byte count at
> + * compile time.
> + */

There is no need for a comment. This is obvious from the code itself.

> +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),
> +	{}
> +};

I am not sure about the usefulness of these test cases.
rv_register_reactor() is not an user API that we have to prepare
for corner case usage. We can expect its users to be sane since the
users are us.

But well, if Gabriele wants to keep them..

> +
> +static struct kunit_suite rv_reactor_registration_suite = {
> +	.name		= "rv_reactor_registration",
> +	.test_cases	= rv_reactor_registration_cases,
> +};
> +
> +static atomic_t react_call_count;

Do we really need atomic_t? Does int work?

> +
> +__printf(1, 0) static void mock_react(const char *msg, va_list args)
> +{
> +	atomic_inc(&react_call_count);
> +	/*
> +	 * Hold the CPU for 5 ms so a timer interrupt is likely to fire
> +	 * inside rv_react()'s lockdep context, exercising the LD_WAIT_SPIN
> +	 * constraint.  mdelay() is a calibrated busy-wait with no scheduler
> +	 * interaction.
> +	 */

The comment above mdelay()'s definition already explains what it does.

> +	mdelay(5);
> +}
> +
> +static void test_react_no_callback(struct kunit *test)
> +{
> +	struct rv_monitor monitor = {
> +		.name = "kunit_null_react",
> +	};
> +
> +	atomic_set(&react_call_count, 0);
> +	rv_react(&monitor, "no callback");
> +
> +	/*
> +	 * The only possible failure in this test case is a kernel panic.
> +	 * NULL react guard: callback must NOT have been invoked
> +	 */

Obvious comment.

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

So the test calls rv_react(), and validates that the reactor is called?
Honestly I am not sure how useful that is. Especially since Gabriele
already made the selftest which validates that the reactor is invoked.

Nam

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

[Nam Cao]

wen.yang@linux.dev writes:

> 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.
>
> Suggested-by: Gabriele Monaco <gmonaco@redhat.com>
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> Signed-off-by: Wen Yang <wen.yang@linux.dev>

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

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

[Gabriele Monaco]

On Mon, 2026-08-17 at 12:52 +0200, Nam Cao wrote:
> wen.yang@linux.dev writes:
> > +/*
> > + * Use a fixed-size array so sizeof() gives the exact byte count at
> > + * compile time.
> > + */
> 
> There is no need for a comment. This is obvious from the code itself.
> 
> > +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),
> > +	{}
> > +};
> 
> I am not sure about the usefulness of these test cases.
> rv_register_reactor() is not an user API that we have to prepare
> for corner case usage. We can expect its users to be sane since the
> users are us.
> 
> But well, if Gabriele wants to keep them..
> 

That's right, it can be a bit of an overkill, but I'm thinking one day reactors
could come from (out-of-tree) kernel modules, it wouldn't hurt to have this path
tested, especially if it doesn't require much effort.

> > +
> > +static struct kunit_suite rv_reactor_registration_suite = {
> > +	.name		= "rv_reactor_registration",
> > +	.test_cases	= rv_reactor_registration_cases,
> > +};
> > +
> > +static atomic_t react_call_count;
> 
> Do we really need atomic_t? Does int work?

Missed that, yes, it should work just fine since KUnit runs in a single thread.

> 
> > +
> > +__printf(1, 0) static void mock_react(const char *msg, va_list args)
> > +{
> > +	atomic_inc(&react_call_count);
> > +	/*
> > +	 * Hold the CPU for 5 ms so a timer interrupt is likely to fire
> > +	 * inside rv_react()'s lockdep context, exercising the LD_WAIT_SPIN
> > +	 * constraint.  mdelay() is a calibrated busy-wait with no
> > scheduler
> > +	 * interaction.
> > +	 */
> 
> The comment above mdelay()'s definition already explains what it does.
> 
> > +	mdelay(5);
> > +}
> > +
> > +static void test_react_no_callback(struct kunit *test)
> > +{
> > +	struct rv_monitor monitor = {
> > +		.name = "kunit_null_react",
> > +	};
> > +
> > +	atomic_set(&react_call_count, 0);
> > +	rv_react(&monitor, "no callback");
> > +
> > +	/*
> > +	 * The only possible failure in this test case is a kernel panic.
> > +	 * NULL react guard: callback must NOT have been invoked
> > +	 */
> 
> Obvious comment.
> 
> > +	KUNIT_EXPECT_EQ(test, atomic_read(&react_call_count), 0);
> > +}
> > +
> > +static void test_react_callback_invoked(struct kunit *test)
> > +{
> > +	struct rv_monitor monitor = {
> > +		.name	= "kunit_dispatch_monitor",
> > +		.react	= mock_react,
> > +	};
> > +
> > +	atomic_set(&react_call_count, 0);
> > +	rv_react(&monitor, "callback invocation test");
> > +	KUNIT_EXPECT_EQ(test, atomic_read(&react_call_count), 1);
> > +}
> 
> So the test calls rv_react(), and validates that the reactor is called?
> Honestly I am not sure how useful that is. Especially since Gabriele
> already made the selftest which validates that the reactor is invoked.

From what I understood, the value in this test suite isn't really about the
assertions (especially in test_react_no_callback). It triggers cases that should
not produce a splat (panic/lockdep) and a pass should consider that too.
This makes them not quite pure KUnit, but again, I think it doesn't hurt to have
this kind of test since KUnit output is in dmesg anyway, so a splat would hardly
slip.

What do you think?

Gabriele