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

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

Hi Andy,

On 5-May-26 11:43, Andy Shevchenko wrote:
> On Tue, May 05, 2026 at 11:39:51AM +0200, Hans de Goede wrote:
>> 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.
> 
> Is it upstream? Can you share the commit ID or patch in ML (if it's ready)
> for that? (Just for the record.)

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cee3dba7b7416c02ff3

Regards,

Hans