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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Mon, Jun 23, 2025 at 09:15:17AM +0300, Mika Westerberg wrote:
> On Sat, Jun 21, 2025 at 10:49:33AM +0200, Hans de Goede wrote:

> > 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.

Hans, any conclusion on this?

-- 
With Best Regards,
Andy Shevchenko