* [RFC] pinctrl: intel: Stop setting IRQF_NO_THREAD ?
@ 2025-06-21 8:49 Hans de Goede
2025-06-23 6:15 ` Mika Westerberg
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2025-06-21 8:49 UTC (permalink / raw)
To: Mika Westerberg, Andy Shevchenko; +Cc: Linus Walleij, linux-gpio
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 ...
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 ?
Regards,
Hans
p.s. For the 2 different code paths approach I believe we can
use intel_pinctrl_probe_by_uid() to identify platforms which use
a shared IRQ. AFAICT on all platforms which use
intel_pinctrl_probe_by_hid() there will only be a single GPIO
controller instance.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] pinctrl: intel: Stop setting IRQF_NO_THREAD ?
2025-06-21 8:49 [RFC] pinctrl: intel: Stop setting IRQF_NO_THREAD ? Hans de Goede
@ 2025-06-23 6:15 ` Mika Westerberg
2026-05-05 9:10 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Mika Westerberg @ 2025-06-23 6:15 UTC (permalink / raw)
To: Hans de Goede; +Cc: Andy Shevchenko, Linus Walleij, linux-gpio
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] pinctrl: intel: Stop setting IRQF_NO_THREAD ?
2025-06-23 6:15 ` Mika Westerberg
@ 2026-05-05 9:10 ` Andy Shevchenko
2026-05-05 9:39 ` Hans de Goede
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2026-05-05 9:10 UTC (permalink / raw)
To: Mika Westerberg; +Cc: Hans de Goede, Andy Shevchenko, Linus Walleij, linux-gpio
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] pinctrl: intel: Stop setting IRQF_NO_THREAD ?
2026-05-05 9:10 ` Andy Shevchenko
@ 2026-05-05 9:39 ` Hans de Goede
2026-05-05 9:43 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2026-05-05 9:39 UTC (permalink / raw)
To: Andy Shevchenko, Mika Westerberg
Cc: Andy Shevchenko, Linus Walleij, linux-gpio
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] pinctrl: intel: Stop setting IRQF_NO_THREAD ?
2026-05-05 9:39 ` Hans de Goede
@ 2026-05-05 9:43 ` Andy Shevchenko
2026-05-05 9:47 ` Hans de Goede
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2026-05-05 9:43 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mika Westerberg, Andy Shevchenko, Linus Walleij, linux-gpio
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.)
> 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.
Thanks for clarifying the state of affairs!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] pinctrl: intel: Stop setting IRQF_NO_THREAD ?
2026-05-05 9:43 ` Andy Shevchenko
@ 2026-05-05 9:47 ` Hans de Goede
0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2026-05-05 9:47 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mika Westerberg, Andy Shevchenko, Linus Walleij, linux-gpio
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-05 9:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-21 8:49 [RFC] pinctrl: intel: Stop setting IRQF_NO_THREAD ? Hans de Goede
2025-06-23 6:15 ` Mika Westerberg
2026-05-05 9:10 ` Andy Shevchenko
2026-05-05 9:39 ` Hans de Goede
2026-05-05 9:43 ` Andy Shevchenko
2026-05-05 9:47 ` Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox