rv/reactors: fix lockdep warning and add KUnit tests
Subsystems (3)
v4 cover rv/reactors: fix lockdep warning and add KUnit tests
From: Wen Yang <[email protected]>
We occasionally hit a lockdep "Invalid wait context" warning in production when a reactor callback is preempted by a timer interrupt. On interrupt exit the scheduler takes rq->__lock (LD_WAIT_SPIN) while rv_react() still holds its wait-type-override map, which declared LD_WAIT_FREE. On any kernel where the task context has preemption enabled (not just CONFIG_PREEMPT_RT) this triggers a spurious lockdep report:
[ BUG: Invalid wait context ]
1 lock held by kunit_try_catch/209:
#0: (rv_react_map-wait-type-override){+.+.}-{1:1}
kunit_try_catch/209 is trying to lock:
ffff8a743ed3e8a0 (&rq->__lock){-...}-{2:2}
This series switches to a single LD_WAIT_SPIN override, hardens reactor registration error handling, exports the registration helpers for modules, and adds KUnit coverage.
Changes in v4:
- Patch 1: replace the context-sensitive two-map wait type with a single LD_WAIT_SPIN override, and document the constraint.
- Patch 4: use a plain int instead of atomic_t for the test counter, trim redundant comments.
v3: https://lore.kernel.org/lkml/[email protected]/
v2: https://lore.kernel.org/lkml/[email protected]/
v1: https://lore.kernel.org/lkml/[email protected]/
Wen Yang (4):
rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type rv/reactors: propagate rv_register_reactor() error from reactor init rv/reactors: export rv_register_reactor() and rv_unregister_reactor() rv/reactors: add KUnit tests for reactor registration and dispatch
Documentation/trace/rv/monitor_synthesis.rst | 20 ++++ kernel/trace/rv/Kconfig | 12 +++ kernel/trace/rv/Makefile | 1 + kernel/trace/rv/reactor_panic.c | 3 +- kernel/trace/rv/reactor_printk.c | 3 +- kernel/trace/rv/rv_reactors.c | 8 +- kernel/trace/rv/rv_reactors_kunit.c | 105 +++++++++++++++++++ 7 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 kernel/trace/rv/rv_reactors_kunit.c
base-commit: 785095112f4198de49760552374f364043c8dbdf
Signature · 1 line
2.25.1
PATCH v4 1/4 rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
From: Wen Yang <[email protected]>
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.
--- Documentation/trace/rv/monitor_synthesis.rst | 20 ++++++++++++++++++++ kernel/trace/rv/rv_reactors.c | 6 +++++- 2 files changed, 25 insertions(+), 1 deletion(-)
Documentation/trace/rv/monitor_synthesis.rst +20 -0
@@ -365,6 +365,26 @@ but higher overhead. The timer wheel (``HA_TIMER_WHEEL``) is a good alternative for monitors with several instances (e.g. per-task) that achieves lower overhead with increased latency, yet without compromising precision. +Reactors +-------- + +A reactor is a callback triggered by a monitor when a violation is +detected. Reactors are registered via ``/sys/kernel/tracing/rv/reactors/`` +and enabled per monitor. + +Reactor Locking Rules ++++++++++++++++++++++ + +A reactor callback may be invoked from various contexts (process, +softirq, hardirq, NMI) depending on the tracepoint to which its +monitor is attached. + +Lockdep uses a fixed wait type: ``LD_WAIT_SPIN``. This allows +``raw_spinlock_t`` but disallows sleepable locks. ``LD_WAIT_FREE`` is +not viable in preemptible contexts because scheduler preemption takes +``rq->__lock`` (``LD_WAIT_SPIN``), which would cause false-positive +warnings. + Final remarks -------------
kernel/trace/rv/rv_reactors.c +5 -1
@@ -465,7 +465,11 @@ int init_rv_reactors(struct dentry *root_dir)void rv_react (struct rv_monitor *monitor,const char *msg, ...) { - static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE); +/* +* Use LD_WAIT_SPIN uniformly for deterministic lockdep checking. +* See Documentation/trace/rv/runtime-verification.rst. +*/ + static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_SPIN);va_list args;if (!rv_reacting_on () || !monitor->react)
Signature · 1 line
2.25.1
[email protected]
AI reviewRe: [PATCH v4 1/4] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The documentation for reactor locking rules was added to the wrong file (`monitor_synthesis.rst`…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The documentation for reactor locking rules was added to the wrong file (monitor_synthesis.rst instead of runtime-verification.rst), causing an inconsistency between the commit message, source code comments, and the actual documentation. --
commit 26073218a7c3b83225869704d414a262b5e15e47
Author: Wen Yang <[email protected]>
rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
This commit changes the lockdep wait type override in rv_react() from LD_WAIT_FREE to LD_WAIT_SPIN to prevent spurious "Invalid wait context" warnings. It also adds documentation explaining the reactor locking rules.
Quoted 17 lines
Documentation/trace/rv/monitor_synthesis.rst +9
-0@@ -365,6 +365,26 @@ but higher overhead. The timer wheel (``HA_TIMER_WHEEL``) is a good alternative for monitors with several instances (e.g. per-task) that achieves lower overhead with increased latency, yet without compromising precision. +Reactors +-------- + +A reactor is a callback triggered by a monitor when a violation is +detected. Reactors are registered via ``/sys/kernel/tracing/rv/reactors/`` +and enabled per monitor. + +Reactor Locking Rules +++++++++++++++++++++
[ ... ]
kernel/trace/rv/rv_reactors.c int init_rv_reactors(struct dentry *root_dir) · line 471
Quoted 13 lines
kernel/trace/rv/rv_reactors.c +4
-1@@ -465,7 +465,11 @@ int init_rv_reactors(struct dentry *root_dir)void rv_react (struct rv_monitor *monitor,const char *msg, ...) { - static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE); +/* +* Use LD_WAIT_SPIN uniformly for deterministic lockdep checking. +* See Documentation/trace/rv/runtime-verification.rst. +*/
[Severity: Low]
Is the documentation for reactor locking rules added to the wrong file?
The comment added here in rv_react() points to Documentation/trace/rv/runtime-verification.rst for the explanation of the LD_WAIT_SPIN override. However, the documentation was actually added to Documentation/trace/rv/monitor_synthesis.rst.
Could the comment or the file location be updated so readers following the code comment will look in the correct documentation file?
Quoted 2 lines
kernel/trace/rv/rv_reactors.c +1
-0+ static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_SPIN);va_list args;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Gabriele Monaco
maintainerRe: [PATCH v4 1/4] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
In reply to [email protected]Quoted 12 lines · On Fri, 2026-08-28 at 02:22 +0800, [email protected] wrote:
From: Wen Yang <[email protected]>
Documentation/trace/rv/monitor_synthesis.rst +0
-0@@ -365,6 +365,26 @@ but higher overhead. The timer wheel (``HA_TIMER_WHEEL``)is a good alternative
for monitors with several instances (e.g. per-task) that achieves lower overhead with increased latency, yet without compromising precision.
Documentation/trace/rv/monitor_synthesis.rst but higher overhead. The timer wheel (``HA_TIMER_WHEEL``) is a good alternative · line 373
Quoted 6 lines
Documentation/trace/rv/monitor_synthesis.rst +6
-0+Reactors +-------- + +A reactor is a callback triggered by a monitor when a violation is +detected. Reactors are registered via ``/sys/kernel/tracing/rv/reactors/`` +and enabled per monitor.
This is not correct, they stay in /sys/kernel/tracing/rv/monitors/MONITOR/reactors and it is already explained in Documentation/trace/rv/runtime-verification.rst .
You don't need to add any further section here (you're more than welcome to update the existing one obviously, but not as part of this patch/series).
monitor_synthesis.rst is not about reactors.
At the moment we don't really have a good spot under Documentation to describe the /implementation/ of reactors.
I'd say for the purpose of this patch, in-code documentation (i.e. a comment) is sufficient, we can revise this later if needed.
Quoted 4 lines
Documentation/trace/rv/monitor_synthesis.rst +4
-0+ +Reactor Locking Rules ++++++++++++++++++++++ +
Documentation/trace/rv/monitor_synthesis.rst but higher overhead. The timer wheel (``HA_TIMER_WHEEL``) is a good alternative · line 387
Quoted 10 lines
Documentation/trace/rv/monitor_synthesis.rst +10
-0+A reactor callback may be invoked from various contexts (process, +softirq, hardirq, NMI) depending on the tracepoint to which its +monitor is attached. + +Lockdep uses a fixed wait type: ``LD_WAIT_SPIN``. This allows +``raw_spinlock_t`` but disallows sleepable locks. ``LD_WAIT_FREE`` is +not viable in preemptible contexts because scheduler preemption takes +``rq->__lock`` (``LD_WAIT_SPIN``), which would cause false-positive +warnings. +
The code looks fine but this documentation is missing the point. We don't need to explain what lockdep does but why we use a specific class and what we actually wanted to use. Let's readapt the comment you added in v3
/*
* Reactors must not explicitly take locks, so they should be
* LD_WAIT_FREE. However, reactor callbacks can run with preemption
* enabled, meaning the preempting code (e.g. the scheduler taking
* rq->__lock at LD_WAIT_SPIN) may violate that constraint. Use
* LD_WAIT_SPIN to avoid false-positive lockdep reports.
* But you should still NOT be using locks in reactors. */
Quoted 11 lines
Final remarks
-------------kernel/trace/rv/rv_reactors.c +0
-0@@ -465,7 +465,11 @@ int init_rv_reactors(struct dentry *root_dir)
voidrv_react(struct rv_monitor *monitor, const char *msg, ...){
kernel/trace/rv/rv_reactors.c int init_rv_reactors(struct dentry *root_dir) · line 471
Quoted 5 lines
kernel/trace/rv/rv_reactors.c +4
-1- static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE); +/* +* Use LD_WAIT_SPIN uniformly for deterministic lockdep checking. +* See Documentation/trace/rv/runtime-verification.rst. +*/
See above, I would only add an explicit comment here of /why/ we do LD_WAIT_SPIN although we would like LD_WAIT_FREE.
Thanks,
Gabriele
Quoted 4 lines
kernel/trace/rv/rv_reactors.c +1
-0+ static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_SPIN);
va_listargs;
if (!rv_reacting_on() || !monitor->react)
PATCH v4 2/4 rv/reactors: propagate rv_register_reactor() error from reactor init
In reply to [email protected]From: Wen Yang <[email protected]>
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.
--- kernel/trace/rv/reactor_panic.c | 3 +-- kernel/trace/rv/reactor_printk.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
kernel/trace/rv/reactor_panic.c +1 -2
@@ -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)
kernel/trace/rv/reactor_printk.c +1 -2
@@ -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)
Signature · 1 line
2.25.1
PATCH v4 3/4 rv/reactors: export rv_register_reactor() and rv_unregister_reactor()
In reply to [email protected]From: Wen Yang <[email protected]>
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).
--- kernel/trace/rv/rv_reactors.c | 2 ++ 1 file changed, 2 insertions(+)
kernel/trace/rv/rv_reactors.c +2 -0
@@ -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.
Signature · 1 line
2.25.1
[email protected]
AI reviewRe: [PATCH v4 3/4] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Exporting `rv_register_reactor()` and `rv_unregister_reactor()` allows dynamically loaded modules…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Exporting rv_register_reactor() and rv_unregister_reactor() allows dynamically loaded modules to register reactors. However, rv_unregister_reactor() fails to detach the reactor from monitors currently using it, and struct rv_reactor lacks an owner field to pin the module in memory. This leads to a dangling pointer and use-after-free when a reactor module is unloaded. --
commit f153a63763986ecb001f784d45c5262d45c8d0a5
Author: Wen Yang <[email protected]>
Subject: rv/reactors: export rv_register_reactor() and rv_unregister_reactor()
This commit exports rv_register_reactor() and rv_unregister_reactor() so that dynamically loaded modules and KUnit tests can register and unregister reactors without link-time errors.
Quoted 9 lines
kernel/trace/rv/rv_reactors.c +1
-0@@ -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);
[ ... ]
kernel/trace/rv/rv_reactors.c int rv_unregister_reactor(struct rv_reactor *reactor) · line 331
Quoted 5 lines
kernel/trace/rv/rv_reactors.c +1
-0@@ -327,6 +328,7 @@ int rv_unregister_reactor(struct rv_reactor *reactor)list_del (&reactor->list);return 0 ; } +EXPORT_SYMBOL_GPL (rv_unregister_reactor);
[Severity: High]
By exporting these functions, dynamic modules can now register and unregister reactors. However, when a module is unloaded and calls rv_unregister_reactor(), does this leave dangling pointers in monitors that are currently using it?
Looking at how monitors assign reactors, struct rv_reactor lacks an owner field to pin the module in memory:
kernel/trace/rv/rv_reactors.c:monitor_swap_reactors_single() {
...
mon->reactor = reactor;
mon->react = reactor->react;
...
}Since rv_unregister_reactor() only removes the reactor from the global list without clearing active monitor references, won't rv_react() later execute a dangling pointer when an event triggers?
kernel/trace/rv/rv_reactors.c:rv_react() {
...
monitor->react(msg, args);
...
}Could this result in a use-after-free pointing to unmapped module memory if an authorized user unloads a reactor module that is still attached to an active monitor?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
PATCH v4 4/4 rv/reactors: add KUnit tests for reactor registration and dispatch
In reply to [email protected]From: Wen Yang <[email protected]>
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.
--- kernel/trace/rv/Kconfig | 12 ++++ kernel/trace/rv/Makefile | 1 + kernel/trace/rv/rv_reactors_kunit.c | 105 ++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 kernel/trace/rv/rv_reactors_kunit.c
kernel/trace/rv/Kconfig +12 -0
@@ -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
kernel/trace/rv/Makefile +1 -0
@@ -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
kernel/trace/rv/rv_reactors_kunit.c new file +105 -0
105 lines not on this page · raw message
Signature · 1 line
2.25.1
[email protected]
AI reviewRe: [PATCH v4 4/4] rv/reactors: add KUnit tests for reactor registration and dispatch
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The test `test_react_callback_invoked` spuriously fails if the global reactor dispatch is…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test test_react_callback_invoked spuriously fails if the global reactor dispatch is disabled via sysfs.
- [Medium] The 5ms busy-wait in mock_react is mathematically insufficient to guarantee a timer interrupt on systems with HZ < 200, making the intended lockdep coverage flaky. --
commit 2f80d797b5152331b6b88a29924dc870e4e2b6a0
Author: Wen Yang <[email protected]>
rv/reactors: add KUnit tests for reactor registration and dispatch
This commit adds KUnit tests for the RV reactor register/unregister lifecycle. It includes coverage for duplicate registration, name-length rejection, and verifying that rv_react() correctly dispatches to the callback.
Quoted 5 lines
kernel/trace/rv/rv_reactors_kunit.c new file +0
-0
[ ... ]
kernel/trace/rv/rv_reactors_kunit.c line 64
Quoted 5 lines
kernel/trace/rv/rv_reactors_kunit.c new file +5
-0+__printf(1, 0) static void mock_react(const char *msg, va_list args) +{ + react_call_count++; +/* Busy-wait so a timer interrupt fires inside rv_react(). */ +mdelay (5 );
[Severity: Medium]
Will this 5ms busy-wait reliably trigger a timer interrupt on systems where HZ < 200?
On systems configured with HZ=100, where ticks occur every 10ms, a 5ms delay will only span a timer tick around half the time. Can this lead to flaky test coverage when trying to exercise the LD_WAIT_SPIN lockdep wait context?
Quoted 1 line
+}
[ ... ]
kernel/trace/rv/rv_reactors_kunit.c line 88
Quoted 10 lines
kernel/trace/rv/rv_reactors_kunit.c new file +10
-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 );
[Severity: Medium]
Can this assertion spuriously fail if the global reacting_on state is disabled?
If the test executes while the global state is disabled, such as via /sys/kernel/tracing/rv/reacting_on, rv_react() short-circuits:
kernel/trace/rv/rv_reactors.c:rv_react() {
...
if (!rv_reacting_on() || !monitor->react)
return;
...
}Will this cause the test to fail by not invoking the callback at all?
Quoted 1 line
+}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4