Re: [RFC] pinctrl: intel: Stop setting IRQF_NO_THREAD ?

[Mika Westerberg <mika.westerberg@linux.intel.com>]

Hi,

On Sat, Jun 21, 2025 at 10:49:33AM +0200, Hans de Goede wrote:
> Hi,
> 
> While debugging the following lockdep report:
> 
> =============================
>  [ BUG: Invalid wait context ]
>  ...
>  swapper/10/0 is trying to lock:
>  ffff88819c271888 (&tp->xfer_wait){....}-{3:3},
>   at: __wake_up (kernel/sched/wait.c:106 kernel/sched/wait.c:127)
>  ...
>  Call Trace:
>  <IRQ>
>  ...
>  __raw_spin_lock_irqsave (./include/linux/spinlock_api_smp.h:111)
>  __wake_up (kernel/sched/wait.c:106 kernel/sched/wait.c:127)
>  vsc_tp_isr (drivers/misc/mei/vsc-tp.c:110) mei_vsc_hw
>  __handle_irq_event_percpu (kernel/irq/handle.c:158)
>  handle_irq_event (kernel/irq/handle.c:195 kernel/irq/handle.c:210)
>  handle_edge_irq (kernel/irq/chip.c:833)
>  ...
>  </IRQ>
> 
> I realized after a while that the root-cause here is the IRQF_NO_THREAD
> usage in pinctrl-intel.c. This was introduced in 1a7d1cb81eb2 ("pinctrl:
> intel: Prevent force threading of the interrupt handler") to avoid problems
> caused by using request_irq() for what should be a chained irq handler
> (which itself is a workaround because of a shared IRQ on some platforms).
> 
> Generally speaking using IRQF_NO_THREAD is undesirable for 2 reasons:
> 
> 1. It introduces extra latency on PREEMPT-RT kernels
> 2. Setting IRQF_NO_THREAD requires all interrupt handlers for GPIO
>    interrupts to use raw-spinlocks only since normal spinlocks can
>    sleep in PREEMPT-RT kernels and with IRQF_NO_THREAD the interrupt
>    handlers will run in an atomic context
> 
> 2. is what is causing the lockdep report above, by simply using a
> wake_up(&wq_head) call in an interrupt handler, since wait-queues
> use normal spinlocks not raw spinlocks.
> 
> I've tried just removing the IRQF_NO_THREAD flag and that fixes
> the lockdep report. I've also tried reproducing the problem for
> which the flag was added in commit 1a7d1cb81eb2 by using a kernel
> with CONFIG_IRQ_FORCED_THREADING and "threadirqs" on the kernel
> commandline. And the problem not reproduce. I'm not sure this is
> 100% proof that the flag is no longer necessary though ...

Can you try also with CONFIG_PREEMPT_RT and see if that triggers the issue?
If not then:

> So 2 questions:
> 
> 1. Should we maybe just drop the flag ?
> 2. Or should we have 2 different code-paths for GPIO controllers
> with/without shared IRQs and use a chained-irq approach for the
> not shared case, to at least reduce the usage of the flag ?

I would just drop the flag then.