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


Reply via email to