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

On Mon, 2026-08-10 at 01:10 +0800, [email protected] wrote:
> From: Wen Yang <[email protected]>
> 
> Reactors must not explicitly take locks, so they should comply with
> LD_WAIT_FREE.  However, reactor callbacks can run with preemption
> enabled on any kernel (not just PREEMPT_RT).  If a timer interrupt
> fires during the callback, the interrupt exit path schedules and
> acquires rq->__lock (LD_WAIT_SPIN) while the lockdep override map that
> declared LD_WAIT_FREE is still held, triggering a spurious
> "Invalid wait context" warning:
> 
>     [ BUG: Invalid wait context ]
>     context-{5:5}
>     1 lock held by kunit_try_catch/209:
>      #0: (rv_react_map-wait-type-override){+.+.}-{1:1}
>     kunit_try_catch/209 is trying to lock:
>     ffff8a743ed3e8a0 (&rq->__lock){-...}-{2:2}
> 
> Fixes: 69d8895cb9a9 ("rv: Add explicit lockdep context for reactors")
> Suggested-by: Gabriele Monaco <[email protected]>

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

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

Reviewed-by: Gabriele Monaco <[email protected]>

Thanks,
Gabriele

> Signed-off-by: Wen Yang <[email protected]>
> Cc: Thomas Weißschuh <[email protected]>
> ---
>  kernel/trace/rv/rv_reactors.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index 2f5fc8d18dea..5830229210e8 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -465,18 +465,28 @@ int init_rv_reactors(struct dentry *root_dir)
>  
>  void rv_react(struct rv_monitor *monitor, const char *msg, ...)
>  {
> -	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE);
> +	/*
> +	 * Reactors must not explicitly take locks, so they should be
> +	 * LD_WAIT_FREE.  However, reactor callbacks can run with preemption
> +	 * enabled, meaning the preempting code (e.g. the scheduler taking
> +	 * rq->__lock at LD_WAIT_SPIN) may violate that constraint.  Use
> +	 * LD_WAIT_SPIN in preemptible contexts to avoid false-positive
> lockdep
> +	 * reports; keep LD_WAIT_FREE in NMI/hardirq where preemption is
> +	 * disabled and the scheduler cannot run.
> +	 */
> +	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map,        LD_WAIT_SPIN);
> +	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map_atomic, LD_WAIT_FREE);
> +	struct lockdep_map * __maybe_unused map;
>  	va_list args;
>  
>  	if (!rv_reacting_on() || !monitor->react)
>  		return;
>  
> +	map = (in_nmi() || in_hardirq()) ? &rv_react_map_atomic :
> &rv_react_map;
>  	va_start(args, msg);
> -
> -	lock_map_acquire_try(&rv_react_map);
> +	lock_map_acquire_try(map);
>  	monitor->react(msg, args);
> -	lock_map_release(&rv_react_map);
> -
> +	lock_map_release(map);
>  	va_end(args);
>  }
>  EXPORT_SYMBOL_GPL(rv_react);