On Mon, 2026-08-03 at 02:43 +0800, [email protected] wrote:
> From: Wen Yang <[email protected]>
>
> 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 <[email protected]>
> Cc: Thomas Weißschuh <[email protected]>
> ---
> 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);
