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

[PATCH v2 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 a preemptible kernel (e.g. 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 v2:
- Use a context-sensitive override map: LD_WAIT_SPIN in preemptible
  contexts, LD_WAIT_FREE in NMI/hardirq where the scheduler cannot run
  (patch 1).
- Merge the v1 printk/panic test files into one generic rv_reactors_kunit
  covering registration and dispatch, as the v1 tests were mostly not
  reactor-specific (patches 2-4).
- Export rv_register_reactor()/rv_unregister_reactor() so the tristate
  KUnit module can link against them (patch 3).
- Propagate rv_register_reactor() errors from the built-in reactor init
  functions instead of silently ignoring them (patch 2).
- KUnit tests are tristate (usable with CONFIG_KUNIT=m) and verify the
  callback is actually invoked (atomic counter), the exact -EINVAL on
  duplicate registration, and use mdelay(5) to exercise the lockdep
  context under load.  A teardown action unregisters the test reactor
  even if a test aborts.

Patch 1 fixes the lockdep warning.  Patches 2-3 make the registration
API usable by the test module and propagate its errors.  Patch 4 adds
the KUnit tests.

Tested with CONFIG_PROVE_LOCKING=y and CONFIG_KUNIT=y.



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

base-commit: 984b5a36fd12d1849511a45fda32036fe2b0d004
-- 
2.25.1

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

[wen.yang]

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

The single DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE) in
rv_react() declares wait_type_inner = LD_WAIT_FREE for every execution
context.  In a preemptible context (e.g. CONFIG_PREEMPT_RT or a KUnit
test running on a task), a timer interrupt can fire during a reactor
callback; the interrupt exit path then schedules and acquires rq->__lock
(LD_WAIT_SPIN) while the override map is still held.  Since the map
declares the context to be wait-free, lockdep reports 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 two lockdep override maps, selected by execution context:

  - Preemptible context (task, softirq, PREEMPT_RT irq thread): the
    scheduler may preempt, so use LD_WAIT_SPIN, the tightest wait type
    the scheduler itself uses, to suppress the spurious warning.

  - NMI/hardirq context: preemption is disabled and the scheduler cannot
    run, so the false positive cannot arise.  Keep LD_WAIT_FREE here to
    preserve the original constraint that reactors must not take raw
    spinlocks in atomic context.

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

diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index 2f5fc8d18dea..cd571b1649f5 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -465,18 +465,25 @@ 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);
+	/*
+	 * A reactor callback can be preempted; the scheduler then takes
+	 * rq->__lock (LD_WAIT_SPIN).  Advertise that in preemptible contexts
+	 * to avoid a spurious lockdep report, and keep LD_WAIT_FREE in atomic
+	 * ones where 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 v2 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>
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 v2 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 load time.

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 cd571b1649f5..228ed043bd73 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 v2 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.

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 | 143 ++++++++++++++++++++++++++++
 3 files changed, 156 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..32c25dcf0cb6
--- /dev/null
+++ b/kernel/trace/rv/rv_reactors_kunit.c
@@ -0,0 +1,143 @@
+// 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",
+};
+
+/*
+ * rv_unregister_reactor() on a reactor that was never registered would
+ * list_del() an uninitialized list_head, so track registration and only
+ * unregister in the teardown if it is still on the list.
+ */
+static bool test_reactor_registered;
+
+static int unregister_test_reactor(void)
+{
+	int ret = 0;
+
+	if (test_reactor_registered) {
+		ret = rv_unregister_reactor(&test_reactor);
+		test_reactor_registered = false;
+	}
+
+	return ret;
+}
+
+/*
+ * The teardown action guarantees the reactor is unregistered even if a
+ * test fails mid-way, so a leftover entry cannot corrupt later tests.
+ */
+static void reactor_teardown(void *arg)
+{
+	unregister_test_reactor();
+}
+
+static void register_test_reactor(struct kunit *test)
+{
+	KUNIT_ASSERT_EQ(test, rv_register_reactor(&test_reactor), 0);
+	test_reactor_registered = true;
+	KUNIT_ASSERT_EQ(test,
+			kunit_add_action_or_reset(test, reactor_teardown, NULL), 0);
+}
+
+static void test_register_unregister(struct kunit *test)
+{
+	register_test_reactor(test);
+
+	KUNIT_EXPECT_EQ(test, unregister_test_reactor(), 0);
+}
+
+static void test_double_register(struct kunit *test)
+{
+	register_test_reactor(test);
+
+	KUNIT_EXPECT_EQ(test, rv_register_reactor(&test_reactor), -EINVAL);
+
+	KUNIT_EXPECT_EQ(test, unregister_test_reactor(), 0);
+}
+
+static void test_name_too_long(struct kunit *test)
+{
+	/* Name length of MAX_RV_REACTOR_NAME_SIZE (32) must be rejected. */
+	static struct rv_reactor long_reactor = {
+		.name = "kunit_reactor_name_too_long_xxx_",
+	};
+
+	KUNIT_ASSERT_EQ(test, (int)strlen(long_reactor.name),
+			MAX_RV_REACTOR_NAME_SIZE);
+	KUNIT_EXPECT_EQ(test, rv_register_reactor(&long_reactor), -EINVAL);
+}
+
+static struct kunit_case rv_reactor_registration_cases[] = {
+	KUNIT_CASE(test_register_unregister),
+	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",
+	};
+
+	/* rv_react() must silently return when monitor->react is NULL. */
+	rv_react(&monitor, "no callback");
+}
+
+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 v2 3/4] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

[Gabriele Monaco]

On Mon, 2026-08-03 at 02:43 +0800, wen.yang@linux.dev wrote:
> 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 load time.

Nit, but I believe you meant /link/ time (modpost). Linking phase would fail at
the end of the build and you wouldn't have anything (.ko) to load.

Anyway patch looks good:

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

Thanks,
Gabriele

> 
> 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 cd571b1649f5..228ed043bd73 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.

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

[Gabriele Monaco]

On Mon, 2026-08-03 at 02:43 +0800, wen.yang@linux.dev wrote:
> 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>

> 
> Suggested-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)

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

[Gabriele Monaco]

On Mon, 2026-08-03 at 02:43 +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.
> 
> 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 | 143 ++++++++++++++++++++++++++++
>  3 files changed, 156 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..32c25dcf0cb6
> --- /dev/null
> +++ b/kernel/trace/rv/rv_reactors_kunit.c
> @@ -0,0 +1,143 @@
> +// 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",
> +};
> +
> +/*
> + * rv_unregister_reactor() on a reactor that was never registered would
> + * list_del() an uninitialized list_head, so track registration and only
> + * unregister in the teardown if it is still on the list.
> + */
> +static bool test_reactor_registered;

I'm not too fond of adding more logic than necessary in the test, but if you
really want to keep the dynamic teardown cannot this variable stay somehow in
test-specific memory (test->priv)?

That feels to me a bit cleaner, although the reactor and obviously the reactors
list are still static, so we probably wouldn't gain a huge deal.

What do you think?

> +
> +static int unregister_test_reactor(void)
> +{
> +	int ret = 0;
> +
> +	if (test_reactor_registered) {
> +		ret = rv_unregister_reactor(&test_reactor);
> +		test_reactor_registered = false;

rv_unregister_reactor() cannot fail and probably never will (I'm not quite sure
it should return int).
But setting test_reactor_registered to false here is assuming it didn't fail
while the rest of the test doesn't make this assumption.

Maybe let's make it void and stop tracking a result that can never change (and
that real reactors already have to ignore).

If we do that you may even stop unregistering manually and rely only on the
teardown, and perhaps get rid of test_reactor_registered. Even
test_register_unregister() would be mostly already covered by the next one.
Just throwing in the idea.

> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * The teardown action guarantees the reactor is unregistered even if a
> + * test fails mid-way, so a leftover entry cannot corrupt later tests.
> + */
> +static void reactor_teardown(void *arg)
> +{
> +	unregister_test_reactor();
> +}
> +
> +static void register_test_reactor(struct kunit *test)
> +{
> +	KUNIT_ASSERT_EQ(test, rv_register_reactor(&test_reactor), 0);
> +	test_reactor_registered = true;
> +	KUNIT_ASSERT_EQ(test,
> +			kunit_add_action_or_reset(test, reactor_teardown,
> NULL), 0);
> +}
> +
> +static void test_register_unregister(struct kunit *test)
> +{
> +	register_test_reactor(test);
> +
> +	KUNIT_EXPECT_EQ(test, unregister_test_reactor(), 0);
> +}
> +
> +static void test_double_register(struct kunit *test)
> +{
> +	register_test_reactor(test);
> +
> +	KUNIT_EXPECT_EQ(test, rv_register_reactor(&test_reactor), -EINVAL);
> +
> +	KUNIT_EXPECT_EQ(test, unregister_test_reactor(), 0);
> +}
> +
> +static void test_name_too_long(struct kunit *test)
> +{
> +	/* Name length of MAX_RV_REACTOR_NAME_SIZE (32) must be rejected. */
> +	static struct rv_reactor long_reactor = {
> +		.name = "kunit_reactor_name_too_long_xxx_",
> +	};
> +
> +	KUNIT_ASSERT_EQ(test, (int)strlen(long_reactor.name),
> +			MAX_RV_REACTOR_NAME_SIZE);

You probably want KUNIT_ASSERT_GE, or even better
_Static_assert(sizeof(long_name) - 1 >= MAX_RV_REACTOR_NAME_SIZE)

(you'd need to save the name to a static array first for sizeof() to work as
expected)

> +	KUNIT_EXPECT_EQ(test, rv_register_reactor(&long_reactor), -EINVAL);
> +}
> +
> +static struct kunit_case rv_reactor_registration_cases[] = {
> +	KUNIT_CASE(test_register_unregister),
> +	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",
> +	};
> +
> +	/* rv_react() must silently return when monitor->react is NULL. */
> +	rv_react(&monitor, "no callback");

So what are we testing here? That the kernel doesn't panic? It's fair to expect
a silent return, but we cannot really validate that.

If that's your intent, maybe specify it better, since, in fact, KUnit cannot
validate this. Something like:

  "The only possible failure in this test case is a kernel panic"

Thanks,
Gabriele

> +}
> +
> +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 v2 1/4] rv/reactors: use context-sensitive lockdep wait type in rv_react()

[Gabriele Monaco]

On Mon, 2026-08-03 at 02:43 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
> 
> The single DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE) in
> rv_react() declares wait_type_inner = LD_WAIT_FREE for every execution
> context.  In a preemptible context (e.g. CONFIG_PREEMPT_RT or a KUnit
> test running on a task), a timer interrupt can fire during a reactor

We are obviously not doing this for KUnit tests, but aren't tracepoint handlers
also running with preemption enabled on non-PREEMPT_RT kernels now?
So technically this is a problem with any configuration if events don't run with
preemption disabled for other reasons.

Or is the issue with spinlocks only popping out on PREEMPT_RT because they
become sleeping locks?
Is lockdep really happy to allow an interrupt/schedule taking spinlocks under
LD_WAIT_FREE on non-PREEMPT_RT?

> callback; the interrupt exit path then schedules and acquires rq->__lock
> (LD_WAIT_SPIN) while the override map is still held.  Since the map
> declares the context to be wait-free, lockdep reports 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 two lockdep override maps, selected by execution context:
> 
>   - Preemptible context (task, softirq, PREEMPT_RT irq thread): the
>     scheduler may preempt, so use LD_WAIT_SPIN, the tightest wait type
>     the scheduler itself uses, to suppress the spurious warning.
> 
>   - NMI/hardirq context: preemption is disabled and the scheduler cannot
>     run, so the false positive cannot arise.  Keep LD_WAIT_FREE here to
>     preserve the original constraint that reactors must not take raw
>     spinlocks in atomic context.

So here you're describing at length the solution but not really why you're doing
that. A reader that didn't follow the discussion might think the requirement is
indeed context-dependant, it isn't.

I'd write very bluntly something like:

  "Reactors are not supposed to explicitly take locks, reactor code must comply
with LD_WAIT_FREE. However reactors may run with interrupts and preemption
enabled, so the interrupting code may not satisfy this constraint. Relax it if
we are running from a context that cannot be interrupted to avoid false
positives."

I would write something like that also in the comment, to make clear that
reactors really should be LD_WAIT_FREE, but we are asserting that as best
effort.

What do you think?
Thanks,
Gabriele

> Fixes: 69d8895cb9a9 ("rv: Add explicit lockdep context for reactors")
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  kernel/trace/rv/rv_reactors.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index 2f5fc8d18dea..cd571b1649f5 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -465,18 +465,25 @@ 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);
> +	/*
> +	 * A reactor callback can be preempted; the scheduler then takes
> +	 * rq->__lock (LD_WAIT_SPIN).  Advertise that in preemptible contexts
> +	 * to avoid a spurious lockdep report, and keep LD_WAIT_FREE in
> atomic
> +	 * ones where 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);