* [PATCH v5 1/5] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
2026-09-06 17:10 [PATCH v5 0/5] rv/reactors: fix lockdep warning and add tests wen.yang
@ 2026-09-06 17:10 ` wen.yang
2026-09-06 17:18 ` sashiko-bot
2026-09-06 17:10 ` [PATCH v5 2/5] rv/reactors: propagate rv_register_reactor() error from reactor init wen.yang
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: wen.yang @ 2026-09-06 17:10 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Nam Cao, linux-trace-kernel, linux-kernel, Wen Yang,
Thomas Weißschuh
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>
---
kernel/trace/rv/rv_reactors.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index 2f5fc8d18dea..ff7d478227c3 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -465,7 +465,15 @@ 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 to avoid false-positive lockdep reports.
+ * But you should still NOT be using locks in reactors.
+ */
+ static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_SPIN);
va_list args;
if (!rv_reacting_on() || !monitor->react)
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v5 1/5] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
2026-09-06 17:10 ` [PATCH v5 1/5] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type wen.yang
@ 2026-09-06 17:18 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-09-06 17:18 UTC (permalink / raw)
To: wen.yang; +Cc: linux-trace-kernel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit message claims to add documentation, but the patch does not modify any documentation files.
--
commit 9f88f295cb2517cf173e24803d40575bacb352d6
Author: Wen Yang <wen.yang@linux.dev>
rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
This patch changes the reactor lockdep wait type override from LD_WAIT_FREE
to LD_WAIT_SPIN to prevent false-positive "Invalid wait context" warnings.
This resolves issues when preempting code, like the scheduler, takes spinlocks
while a reactor callback is running.
> 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.
[Severity: Low]
The commit message explicitly claims to add documentation to
runtime-verification.rst to describe the LD_WAIT_SPIN logic change made in
rv_react() (kernel/trace/rv/rv_reactors.c).
However, looking at the patch, no documentation files are actually modified.
Was the documentation update for this change accidentally dropped from the
commit?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788705281.git.wen.yang@linux.dev?part=1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 2/5] rv/reactors: propagate rv_register_reactor() error from reactor init
2026-09-06 17:10 [PATCH v5 0/5] rv/reactors: fix lockdep warning and add tests wen.yang
2026-09-06 17:10 ` [PATCH v5 1/5] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type wen.yang
@ 2026-09-06 17:10 ` wen.yang
2026-09-06 17:10 ` [PATCH v5 3/5] rv/reactors: export rv_register_reactor() and rv_unregister_reactor() wen.yang
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: wen.yang @ 2026-09-06 17:10 UTC (permalink / raw)
To: Gabriele Monaco; +Cc: Nam Cao, linux-trace-kernel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v5 3/5] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()
2026-09-06 17:10 [PATCH v5 0/5] rv/reactors: fix lockdep warning and add tests wen.yang
2026-09-06 17:10 ` [PATCH v5 1/5] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type wen.yang
2026-09-06 17:10 ` [PATCH v5 2/5] rv/reactors: propagate rv_register_reactor() error from reactor init wen.yang
@ 2026-09-06 17:10 ` wen.yang
2026-09-11 6:53 ` Gabriele Monaco
2026-09-06 17:10 ` [PATCH v5 4/5] rv/reactors: add KUnit tests for reactor registration and dispatch wen.yang
2026-09-06 17:10 ` [PATCH v5 5/5] selftests/verification: Test loadable module-based reactor wen.yang
4 siblings, 1 reply; 10+ messages in thread
From: wen.yang @ 2026-09-06 17:10 UTC (permalink / raw)
To: Gabriele Monaco; +Cc: Nam Cao, linux-trace-kernel, linux-kernel, 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).
Commit 3d3800b4f7f4 ("rv: Remove reactor's reference counter") noted
that if module-based reactors are supported, try_module_get()/module_put()
should be used. Add struct module *owner to struct rv_reactor so a module
cat set owner = THIS_MODULE; pin the module in monitor_swap_reactors_gingle()
and release it when a monitor detaches or is unregistered.
In-tree reactors leave owner = NULL and are unaffected.
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
include/linux/rv.h | 3 +++
kernel/trace/rv/rv.c | 5 +++++
kernel/trace/rv/rv_reactors.c | 38 +++++++++++++++++++++++++++++------
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/include/linux/rv.h b/include/linux/rv.h
index 541ba404926a..ff3289ba4f02 100644
--- a/include/linux/rv.h
+++ b/include/linux/rv.h
@@ -128,10 +128,13 @@ union rv_task_monitor {
};
#ifdef CONFIG_RV_REACTORS
+struct module;
+
struct rv_reactor {
const char *name;
const char *description;
__printf(1, 0) void (*react)(const char *msg, va_list args);
+ struct module *owner;
struct list_head list;
};
#endif
diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
index 29f155c6968b..458b17c005b3 100644
--- a/kernel/trace/rv/rv.c
+++ b/kernel/trace/rv/rv.c
@@ -803,6 +803,11 @@ int rv_unregister_monitor(struct rv_monitor *monitor)
guard(mutex)(&rv_interface_lock);
rv_disable_monitor(monitor);
+#ifdef CONFIG_RV_REACTORS
+ if (monitor->reactor)
+ module_put(monitor->reactor->owner);
+
+#endif
list_del(&monitor->list);
destroy_monitor_dir(monitor);
diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index ff7d478227c3..136eb7f47c4a 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -62,6 +62,7 @@
*/
#include <linux/lockdep.h>
+#include <linux/module.h>
#include <linux/slab.h>
#include "rv.h"
@@ -159,7 +160,7 @@ static const struct seq_operations monitor_reactors_seq_ops = {
.show = monitor_reactor_show
};
-static void monitor_swap_reactors_single(struct rv_monitor *mon,
+static int monitor_swap_reactors_single(struct rv_monitor *mon,
struct rv_reactor *reactor,
bool nested)
{
@@ -167,29 +168,39 @@ static void monitor_swap_reactors_single(struct rv_monitor *mon,
/* nothing to do */
if (mon->reactor == reactor)
- return;
+ return 0;
+
+ if (reactor->owner && !try_module_get(reactor->owner))
+ return -EBUSY;
monitor_enabled = mon->enabled;
if (monitor_enabled)
rv_disable_monitor(mon);
+ if (mon->reactor)
+ module_put(mon->reactor->owner);
mon->reactor = reactor;
mon->react = reactor->react;
/* enable only once if iterating through a container */
if (monitor_enabled && !nested)
rv_enable_monitor(mon);
+
+ return 0;
}
-static void monitor_swap_reactors(struct rv_monitor *mon, struct rv_reactor *reactor)
+static int monitor_swap_reactors(struct rv_monitor *mon, struct rv_reactor *reactor)
{
struct rv_monitor *p = mon;
+ int ret;
if (rv_is_container_monitor(mon))
list_for_each_entry_continue(p, &rv_monitors_list, list) {
if (p->parent != mon)
break;
- monitor_swap_reactors_single(p, reactor, true);
+ ret = monitor_swap_reactors_single(p, reactor, true);
+ if (ret)
+ return ret;
}
/*
* This call enables and disables the monitor if they were active.
@@ -197,7 +208,7 @@ static void monitor_swap_reactors(struct rv_monitor *mon, struct rv_reactor *rea
* All nested monitors are enabled also if they were off, we may refine
* this logic in the future.
*/
- monitor_swap_reactors_single(mon, reactor, false);
+ return monitor_swap_reactors_single(mon, reactor, false);
}
static ssize_t
@@ -236,10 +247,14 @@ monitor_reactors_write(struct file *file, const char __user *user_buf,
guard(mutex)(&rv_interface_lock);
list_for_each_entry(reactor, &rv_reactors_list, list) {
+ int ret;
+
if (strcmp(ptr, reactor->name) != 0)
continue;
- monitor_swap_reactors(mon, reactor);
+ ret = monitor_swap_reactors(mon, reactor);
+ if (ret)
+ return ret;
return count;
}
@@ -314,6 +329,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 +343,7 @@ int rv_unregister_reactor(struct rv_reactor *reactor)
list_del(&reactor->list);
return 0;
}
+EXPORT_SYMBOL_GPL(rv_unregister_reactor);
/*
* reacting_on interface.
@@ -421,6 +438,15 @@ int reactor_populate_monitor(struct rv_monitor *mon, struct dentry *root)
* Configure as the rv_nop reactor.
*/
mon->reactor = get_reactor_rdef_by_name("nop");
+ if (WARN_ON(!mon->reactor)) {
+ rv_remove(tmp);
+ return -EINVAL;
+ }
+
+ if (mon->reactor->owner && !try_module_get(mon->reactor->owner)) {
+ rv_remove(tmp);
+ return -EBUSY;
+ }
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v5 3/5] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()
2026-09-06 17:10 ` [PATCH v5 3/5] rv/reactors: export rv_register_reactor() and rv_unregister_reactor() wen.yang
@ 2026-09-11 6:53 ` Gabriele Monaco
0 siblings, 0 replies; 10+ messages in thread
From: Gabriele Monaco @ 2026-09-11 6:53 UTC (permalink / raw)
To: wen.yang; +Cc: Nam Cao, linux-trace-kernel, linux-kernel
On Mon, 2026-09-07 at 01:10 +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 link time(modpost).
>
> Commit 3d3800b4f7f4 ("rv: Remove reactor's reference counter") noted
> that if module-based reactors are supported, try_module_get()/module_put()
> should be used. Add struct module *owner to struct rv_reactor so a module
> cat set owner = THIS_MODULE; pin the module in monitor_swap_reactors_gingle()
> and release it when a monitor detaches or is unregistered.
You needed these symbols in KUnit and we are exporting them for /potential/
future support of reactors as modules. I don't see any technical reason why we
shouldn't support this, but they are currently /not/ supported.
I know sashiko and other LLMs complain about this, and they have a point, but
you can ignore them. At most state in this commit message that this does NOT add
support for reactors as modules.
Let's focus this series on its original intent (fix a lockdep warning and add
some KUnit tests that expose a reproducer), then if adding support for reactors
as modules is so simple, you can do it in another series.
If you really want to /also/ add support for reactors as modules in this series,
you need to make that very explicit (not just a vague line in the changelog, but
rather rewrite the entire cover letter and commit message).
And mind that this would mean your series needs to go through another round of
review and serious testing: you are adding a new feature.
> In-tree reactors leave owner = NULL and are unaffected.
>
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Please, whenever you significantly change an already reviewed patch, remove the
reviewed-by, so I can quickly see I need to review it again.
Thanks,
Gabriele
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> ---
> include/linux/rv.h | 3 +++
> kernel/trace/rv/rv.c | 5 +++++
> kernel/trace/rv/rv_reactors.c | 38 +++++++++++++++++++++++++++++------
> 3 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/rv.h b/include/linux/rv.h
> index 541ba404926a..ff3289ba4f02 100644
> --- a/include/linux/rv.h
> +++ b/include/linux/rv.h
> @@ -128,10 +128,13 @@ union rv_task_monitor {
> };
>
> #ifdef CONFIG_RV_REACTORS
> +struct module;
> +
> struct rv_reactor {
> const char *name;
> const char *description;
> __printf(1, 0) void (*react)(const char *msg, va_list args);
> + struct module *owner;
> struct list_head list;
> };
> #endif
> diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
> index 29f155c6968b..458b17c005b3 100644
> --- a/kernel/trace/rv/rv.c
> +++ b/kernel/trace/rv/rv.c
> @@ -803,6 +803,11 @@ int rv_unregister_monitor(struct rv_monitor *monitor)
> guard(mutex)(&rv_interface_lock);
>
> rv_disable_monitor(monitor);
> +#ifdef CONFIG_RV_REACTORS
> + if (monitor->reactor)
> + module_put(monitor->reactor->owner);
> +
> +#endif
> list_del(&monitor->list);
> destroy_monitor_dir(monitor);
>
> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index ff7d478227c3..136eb7f47c4a 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -62,6 +62,7 @@
> */
>
> #include <linux/lockdep.h>
> +#include <linux/module.h>
> #include <linux/slab.h>
>
> #include "rv.h"
> @@ -159,7 +160,7 @@ static const struct seq_operations
> monitor_reactors_seq_ops = {
> .show = monitor_reactor_show
> };
>
> -static void monitor_swap_reactors_single(struct rv_monitor *mon,
> +static int monitor_swap_reactors_single(struct rv_monitor *mon,
> struct rv_reactor *reactor,
> bool nested)
> {
> @@ -167,29 +168,39 @@ static void monitor_swap_reactors_single(struct
> rv_monitor *mon,
>
> /* nothing to do */
> if (mon->reactor == reactor)
> - return;
> + return 0;
> +
> + if (reactor->owner && !try_module_get(reactor->owner))
> + return -EBUSY;
>
> monitor_enabled = mon->enabled;
> if (monitor_enabled)
> rv_disable_monitor(mon);
>
> + if (mon->reactor)
> + module_put(mon->reactor->owner);
> mon->reactor = reactor;
> mon->react = reactor->react;
>
> /* enable only once if iterating through a container */
> if (monitor_enabled && !nested)
> rv_enable_monitor(mon);
> +
> + return 0;
> }
>
> -static void monitor_swap_reactors(struct rv_monitor *mon, struct rv_reactor
> *reactor)
> +static int monitor_swap_reactors(struct rv_monitor *mon, struct rv_reactor
> *reactor)
> {
> struct rv_monitor *p = mon;
> + int ret;
>
> if (rv_is_container_monitor(mon))
> list_for_each_entry_continue(p, &rv_monitors_list, list) {
> if (p->parent != mon)
> break;
> - monitor_swap_reactors_single(p, reactor, true);
> + ret = monitor_swap_reactors_single(p, reactor, true);
> + if (ret)
> + return ret;
> }
> /*
> * This call enables and disables the monitor if they were active.
> @@ -197,7 +208,7 @@ static void monitor_swap_reactors(struct rv_monitor *mon,
> struct rv_reactor *rea
> * All nested monitors are enabled also if they were off, we may
> refine
> * this logic in the future.
> */
> - monitor_swap_reactors_single(mon, reactor, false);
> + return monitor_swap_reactors_single(mon, reactor, false);
> }
>
> static ssize_t
> @@ -236,10 +247,14 @@ monitor_reactors_write(struct file *file, const char
> __user *user_buf,
> guard(mutex)(&rv_interface_lock);
>
> list_for_each_entry(reactor, &rv_reactors_list, list) {
> + int ret;
> +
> if (strcmp(ptr, reactor->name) != 0)
> continue;
>
> - monitor_swap_reactors(mon, reactor);
> + ret = monitor_swap_reactors(mon, reactor);
> + if (ret)
> + return ret;
>
> return count;
> }
> @@ -314,6 +329,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 +343,7 @@ int rv_unregister_reactor(struct rv_reactor *reactor)
> list_del(&reactor->list);
> return 0;
> }
> +EXPORT_SYMBOL_GPL(rv_unregister_reactor);
>
> /*
> * reacting_on interface.
> @@ -421,6 +438,15 @@ int reactor_populate_monitor(struct rv_monitor *mon,
> struct dentry *root)
> * Configure as the rv_nop reactor.
> */
> mon->reactor = get_reactor_rdef_by_name("nop");
> + if (WARN_ON(!mon->reactor)) {
> + rv_remove(tmp);
> + return -EINVAL;
> + }
> +
> + if (mon->reactor->owner && !try_module_get(mon->reactor->owner)) {
> + rv_remove(tmp);
> + return -EBUSY;
> + }
>
> return 0;
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 4/5] rv/reactors: add KUnit tests for reactor registration and dispatch
2026-09-06 17:10 [PATCH v5 0/5] rv/reactors: fix lockdep warning and add tests wen.yang
` (2 preceding siblings ...)
2026-09-06 17:10 ` [PATCH v5 3/5] rv/reactors: export rv_register_reactor() and rv_unregister_reactor() wen.yang
@ 2026-09-06 17:10 ` wen.yang
2026-09-06 17:23 ` sashiko-bot
2026-09-06 17:10 ` [PATCH v5 5/5] selftests/verification: Test loadable module-based reactor wen.yang
4 siblings, 1 reply; 10+ messages in thread
From: wen.yang @ 2026-09-06 17:10 UTC (permalink / raw)
To: Gabriele Monaco; +Cc: Nam Cao, linux-trace-kernel, linux-kernel, 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.
The dispatch tests rely o reacting_on being enabled, since rv_react()
returns early when it if off.
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 | 110 ++++++++++++++++++++++++++++
3 files changed, 123 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..a408edcdde04
--- /dev/null
+++ b/kernel/trace/rv/rv_reactors_kunit.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for RV reactor registration and dispatch.
+ *
+ * The dispatch tests rely on reacting_on beinng enabled, since rv_react()
+ * returns early when it is off. It is on by default when the suites run
+ * built-in; as a module, re-enable it if disabled via
+ * /sys/kerne/tracing/rv/reacting_on.
+ */
+
+#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(20);
+}
+
+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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v5 4/5] rv/reactors: add KUnit tests for reactor registration and dispatch
2026-09-06 17:10 ` [PATCH v5 4/5] rv/reactors: add KUnit tests for reactor registration and dispatch wen.yang
@ 2026-09-06 17:23 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-09-06 17:23 UTC (permalink / raw)
To: wen.yang; +Cc: linux-trace-kernel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Typos in commit message and comments, and non-standard use of _Static_assert.
--
commit a14edcfd6c547b97a3d6e82ee3b87a759234cdc5
Author: Wen Yang <wen.yang@linux.dev>
rv/reactors: add KUnit tests for reactor registration and dispatch
This commit adds KUnit tests to cover the reactor register and unregister
lifecycle, including duplicate and name-length rejection. It also tests
the rv_react() dispatch logic.
> The dispatch tests rely o reacting_on being enabled, since rv_react()
> returns early when it if off.
[Severity: Low]
There appear to be a few typos in the commit message: "rely o" instead of
"rely on", and "it if off" instead of "it is off".
[ ... ]
> diff --git a/kernel/trace/rv/rv_reactors_kunit.c b/kernel/trace/rv/rv_reactors_kunit.c
> new file mode 100644
> index 0000000000000..a408edcdde04f
> --- /dev/null
> +++ b/kernel/trace/rv/rv_reactors_kunit.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit tests for RV reactor registration and dispatch.
> + *
> + * The dispatch tests rely on reacting_on beinng enabled, since rv_react()
> + * returns early when it is off. It is on by default when the suites run
> + * built-in; as a module, re-enable it if disabled via
> + * /sys/kerne/tracing/rv/reacting_on.
> + */
[Severity: Low]
There are a couple of typos in this file header comment: "beinng" instead of
"being", and "/sys/kerne/" instead of "/sys/kernel/".
[ ... ]
> +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)
[Severity: Low]
Does this code use the C11 keyword _Static_assert directly rather than the
kernel's standard static_assert macro? The kernel typically wraps this in
include/linux/build_bug.h as static_assert(), which is the standard
convention for compile-time assertions across the tree.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788705281.git.wen.yang@linux.dev?part=4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 5/5] selftests/verification: Test loadable module-based reactor
2026-09-06 17:10 [PATCH v5 0/5] rv/reactors: fix lockdep warning and add tests wen.yang
` (3 preceding siblings ...)
2026-09-06 17:10 ` [PATCH v5 4/5] rv/reactors: add KUnit tests for reactor registration and dispatch wen.yang
@ 2026-09-06 17:10 ` wen.yang
2026-09-06 17:19 ` sashiko-bot
4 siblings, 1 reply; 10+ messages in thread
From: wen.yang @ 2026-09-06 17:10 UTC (permalink / raw)
To: Gabriele Monaco; +Cc: Nam Cao, linux-trace-kernel, linux-kernel, Wen Yang
From: Wen Yang <wen.yang@linux.dev>
Add a selftest module that register an RV reactor and a test that
exercises the module pinning: rmmod must fail while a monitor is
attached to the reactor and succeed after it is detached. A trap
unloads the module on failure so it does not break the next insmod.
THe module is built through the kselftest TEST_GEN_MODS_DIR mechanism,
like the livepatch selftests.
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
tools/testing/selftests/verification/Makefile | 1 +
tools/testing/selftests/verification/config | 2 +
.../test.d/rv_reactor_loadable.tc | 46 +++++++++++++++++++
.../verification/test_modules/Makefile | 16 +++++++
.../test_modules/rv_test_reactor.c | 37 +++++++++++++++
5 files changed, 102 insertions(+)
create mode 100644 tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc
create mode 100644 tools/testing/selftests/verification/test_modules/Makefile
create mode 100644 tools/testing/selftests/verification/test_modules/rv_test_reactor.c
diff --git a/tools/testing/selftests/verification/Makefile b/tools/testing/selftests/verification/Makefile
index aa8790c22a71..7ff7382d11f0 100644
--- a/tools/testing/selftests/verification/Makefile
+++ b/tools/testing/selftests/verification/Makefile
@@ -3,6 +3,7 @@ all:
TEST_PROGS := verificationtest-ktap
TEST_FILES := test.d settings
+TEST_GEN_MODS_DIR := test_modules
EXTRA_CLEAN := $(OUTPUT)/logs/*
include ../lib.mk
diff --git a/tools/testing/selftests/verification/config b/tools/testing/selftests/verification/config
index 43072c1c38f4..ddd7581f07b9 100644
--- a/tools/testing/selftests/verification/config
+++ b/tools/testing/selftests/verification/config
@@ -1 +1,3 @@
CONFIG_RV=y
+CONFIG_MODULES=y
+CONFIG_MODULES_UNLOAD=y
diff --git a/tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc b/tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc
new file mode 100644
index 000000000000..ff1691661521
--- /dev/null
+++ b/tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc
@@ -0,0 +1,46 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# description: Test a loadable module-based reactor
+# requires: available_reactors insmod:program rmmod:program
+
+MODULE_KO="$FTRACETEST_ROOT/test_modules/rv_test_reactor.ko"
+[ -f "$MODULE_KO" ] || exit_unsupported
+[ -f /proc/modules ] || exit_unsupported
+monitor=$(ls monitors | head -n 1)
+[ -n "$monitor" ] || exit_unsupported
+
+cleanup() {
+ if grep -q '^test_reactors$' available_reactors; then
+ echo nop > "monitors/$monitor/reactors" || true
+ rmmod rv_test_reactor || true
+ fi
+}
+trap cleanup EXIT
+
+test_loadable_reactor() {
+ local monitor="$1"
+
+ insmod "$MODULE_KO"
+ grep -q test_reactor available_reactors
+
+ echo test_reactor > "monitors/$monitor/reactors"
+ grep -q "\[test_reactor\]" "monitors/$monitor/reactors"
+
+ echo 1 > "monitors/$monitor/enable"
+
+ if rmmod rv_test_reactor 2> /dev/null; then
+ echo "FAIL: rmmod succeeded while the reactor is attached to a monitor"
+ return 1
+ fi
+ grep -q test_reactor available_reactors
+
+ echo nop > "monitors/$monitor/reactors"
+ grep -q "\[nop\]" "monitors/$monitor/reactors"
+ grep -q 1 "monitors/$monitor/enable"
+
+ echo 0 > "monitors/$monitor/enable"
+ rmmod rv_test_reactor
+ ! grep -q test_reactor available_reactors
+}
+
+test_loadable_reactor "$monitor"
diff --git a/tools/testing/selftests/verification/test_modules/Makefile b/tools/testing/selftests/verification/test_modules/Makefile
new file mode 100644
index 000000000000..0af95207a9a9
--- /dev/null
+++ b/tools/testing/selftests/verification/test_modules/Makefile
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0
+
+TESTMODS_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
+KDIR ?= /lib/modules/$(shell uname -r)/build
+
+obj-m += rv_test_reactor.o
+
+modules:
+ifneq ("$(wildcard $(KDIR))", "")
+ $(Q)$(MAKE) -C $(KDIR) modules KBUILD_EXTMOD=$(TESTMODS_DIR)
+endif
+
+clean:
+ifneq ("$(wildcard $(KDIR))", "")
+ $(Q)$(MAKE) -C $(KDIR) clean KBUILD_EXTMOD=$(TESTMODS_DIR)
+endif
diff --git a/tools/testing/selftests/verification/test_modules/rv_test_reactor.c b/tools/testing/selftests/verification/test_modules/rv_test_reactor.c
new file mode 100644
index 000000000000..a243df81fa52
--- /dev/null
+++ b/tools/testing/selftests/verification/test_modules/rv_test_reactor.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Loadable RV reactor for the verification selftests. Register a
+ * reactor with owner = THIS_MODULE so the selftests can exercise the
+ * module pinning: unloading is refused while a monitor is attached
+ * to the reactor.
+ */
+
+#include <linux/module.h>
+#include <linux/rv.h>
+
+__printf(1, 0) static void rv_test_reaction(const char *msg, va_list args)
+{
+}
+
+static struct rv_reactor rv_test_reactor = {
+ .name = "test_reactor",
+ .description = "selftest reactor: exercise module-based reactors.",
+ .react = rv_test_reaction,
+ .owner = THIS_MODULE,
+};
+
+static int __init rv_test_reactor_init(void)
+{
+ return rv_register_reactor(&rv_test_reactor);
+}
+
+static void __exit rv_test_reactor_exit(void)
+{
+ rv_unregister_reactor(&rv_test_reactor);
+}
+
+module_init(rv_test_reactor_init);
+module_exit(rv_test_reactor_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Loadable RV reactor for verification selftests");
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v5 5/5] selftests/verification: Test loadable module-based reactor
2026-09-06 17:10 ` [PATCH v5 5/5] selftests/verification: Test loadable module-based reactor wen.yang
@ 2026-09-06 17:19 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-09-06 17:19 UTC (permalink / raw)
To: wen.yang; +Cc: linux-trace-kernel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Typo in the test cleanup trap prevents the script from properly detaching the reactor and unloading the module on test failure.
--
commit fcfb27a313bc6b0b71755c80b19e1e2cbf231cac
Author: Wen Yang <wen.yang@linux.dev>
selftests/verification: Test loadable module-based reactor
Add a selftest module that register an RV reactor and a test that
exercises the module pinning: rmmod must fail while a monitor is
attached to the reactor and succeed after it is detached. A trap
unloads the module on failure so it does not break the next insmod.
> diff --git a/tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc b/tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc
> new file mode 100644
> index 0000000000000..ff16916615215
> --- /dev/null
> +++ b/tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc
> @@ -0,0 +1,46 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# description: Test a loadable module-based reactor
> +# requires: available_reactors insmod:program rmmod:program
> +
> +MODULE_KO="$FTRACETEST_ROOT/test_modules/rv_test_reactor.ko"
> +[ -f "$MODULE_KO" ] || exit_unsupported
> +[ -f /proc/modules ] || exit_unsupported
> +monitor=$(ls monitors | head -n 1)
> +[ -n "$monitor" ] || exit_unsupported
> +
> +cleanup() {
> + if grep -q '^test_reactors$' available_reactors; then
[Severity: Medium]
Is there a typo in this grep pattern? Should it be test_reactor instead of
test_reactors?
If the test fails and triggers the cleanup trap, this condition will evaluate to
false, bypassing the cleanup logic. This leaves the test module loaded and the
reactor attached, which pollutes the test environment for subsequent selftests.
> + echo nop > "monitors/$monitor/reactors" || true
> + rmmod rv_test_reactor || true
> + fi
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788705281.git.wen.yang@linux.dev?part=5
^ permalink raw reply [flat|nested] 10+ messages in thread