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

[Hans de Goede <hansg@kernel.org>]

Hi Andy,

On 5-May-26 11:10, Andy Shevchenko wrote:
> 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?

I worked around this issue in the affected driver. I have not looked
further into actually dropping IRQF_NO_THREAD from the Intel pinctrl/
GPIO drivers.

I do think that dropping IRQF_NO_THREAD from the Intel pinctrl/GPIO
drivers is probably a good idea, but this will need someone to drive
this forward including dealing with any regressions this may lead to.

Regards,

Hans