Re: [PATCH 6/9] lockdep: Introduce wait-type checks

["bigeasy@linutronix.de" <bigeasy@linutronix.de>]

On 2025-11-17 13:42:40 [+0000], Sverdlin, Alexander wrote:
> Hello Peter and all,
> 
> I'm trying to understand the splat I've got with v6.18-rc1:
> 
> =============================
> [ BUG: Invalid wait context ]
> 6.18.0-rc1+git476309497a53 #1 Tainted: G           O       
> -----------------------------
> some-user-space-process/1251 is trying to lock:
> ffff000005cbc548 (&counter->events_list_lock){....}-{3:3}, at: counter_push_event+0x68/0x430 [counter]
> other info that might help us debug this:
> context-{2:2}
> no locks held by some-user-space-process/1251.
> stack backtrace:
> CPU: 0 UID: 0 PID: 1251 Comm: some-user-space-process Tainted: G           O        6.18.0-rc1+git476309497a53 #1 PREEMPT 
> Call trace:
>  show_stack+0x20/0x38 (C)
>  dump_stack_lvl+0x8c/0xd0
>  dump_stack+0x18/0x28
>  __lock_acquire+0x91c/0x1f78
>  lock_acquire+0x1c4/0x338
>  _raw_spin_lock_irqsave+0x68/0x98
>  counter_push_event+0x68/0x430 [counter]
>  interrupt_cnt_isr+0x40/0x78 [interrupt_cnt]
>  __handle_irq_event_percpu+0xa4/0x398
>  handle_irq_event+0x54/0xb8
>  handle_simple_irq+0xe4/0x128
>  handle_irq_desc+0x48/0x68
>  generic_handle_domain_irq+0x24/0x38
>  gpio_irq_handler+0xa0/0x140
>  handle_irq_desc+0x48/0x68
>  generic_handle_domain_irq+0x24/0x38
>  gic_handle_irq+0x54/0x128
>  call_on_irq_stack+0x30/0x70
>  do_interrupt_handler+0x88/0x98
>  el0_interrupt+0x5c/0x270
>  __el0_irq_handler_common+0x18/0x28
>  el0t_64_irq_handler+0x10/0x20
>  el0t_64_irq+0x198/0x1a0
> 
> This is not PREEMPT_RT, but still CONFIG_PROVE_RAW_LOCK_NESTING=y config...
> The relevant logic in the current v6.18-rcX is still largely unchanged
> from the Subject commit and...
> 
The validation is not limited to PREEMPT_RT enabled.
…
> > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > index 32406ef0d6a2d..4a9abf8bd9d15 100644
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
…
> > +	 * Set appropriate wait type for the context; for IRQs we have to take
> > +	 * into account force_irqthread as that is implied by PREEMPT_RT.
> > +	 */
> > +	if (curr->hardirq_context) {
> > +		/*
> > +		 * Check if force_irqthreads will run us threaded.
> > +		 */
> > +		if (curr->hardirq_threaded)
> > +			curr_inner = LD_WAIT_CONFIG;
> > +		else
> > +			curr_inner = LD_WAIT_SPIN;
> 
> ... even though it's "if (curr->hardirq_threaded || curr->irq_config)" in the
> meanwhile, it is unclear to me, why is it conditional at all and not always
> "LD_WAIT_CONFIG" in hardirq context?
> 
> With !PREEMPT_RT but CONFIG_PROVE_RAW_LOCK_NESTING=y we always end up
> with LD_WAIT_SPIN here and it always warns if we take "spinlock_t" in hardirq
> context (as in the splat above). But the latter should be legal under all
> circumstances, isn't it?

Nope. The validation is performed always. The architectures which don't
support PREEMPT_RT are excluded for $reasons that is why
PROVE_RAW_LOCK_NESTING remains an option there.

A spinlock_t is a sleeping lock in a PREEMPT_RT context and the
interrupt hander are threaded. In this case a spinlock_t is okay.
However, if a spinlock_t is used in a non-forced-threaded environment
(such as in a primary handler with IRQF_NO_THREAD) then this handler
will not be threaded. Therefore spinlock_t is not a valid class.

Sebastian