Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}() | Date | Wed, 11 May 2022 00:52:29 +0200 |
| |
On Tue, May 10 2022 at 15:15, Mark Rutland wrote: > On Tue, May 10, 2022 at 02:13:20PM +0200, Lukas Wunner wrote: >> For gpio-dln2.c, I believe it from inspection. >> >> For smsc95xx.c, I'm actually seeing it go wrong in practice, >> unedited dmesg splat is included below FWIW. > > Thanks; having the trace makes this much easier to analyse.
which confirmes what I talked about before:
>> WARNING: CPU: 3 PID: 75 at kernel/irq/irqdesc.c:702 generic_handle_domain_irq+0x88/0x94 >> generic_handle_domain_irq from smsc95xx_status+0x54/0xb0 >> smsc95xx_status from intr_complete+0x80/0x84 >> intr_complete from __usb_hcd_giveback_urb+0xa4/0x12c >> __usb_hcd_giveback_urb from usb_hcd_giveback_urb+0x118/0x11c >> usb_hcd_giveback_urb from completion_tasklet_func+0x7c/0xc8 >> completion_tasklet_func from tasklet_callback+0x20/0x24 >> tasklet_callback from tasklet_action_common.constprop.0+0x148/0x220 >> tasklet_action_common.constprop.0 from tasklet_hi_action+0x28/0x30 >> tasklet_hi_action from __do_softirq+0x154/0x3e8 >> __do_softirq from __local_bh_enable_ip+0x12c/0x1a8 >> __local_bh_enable_ip from irq_forced_thread_fn+0x7c/0xac >> irq_forced_thread_fn from irq_thread+0x16c/0x228 >> irq_thread from kthread+0x100/0x140
So what happens here:
interrupt -> wakeup threaded handler
threaded handler runs local_bh_disable(); .... schedules tasklet ... local_bh_enable() do_softirq() run_tasklet() urb_completion() smsc95xx_status() generic_handle_domain_irq()
That interrupt in question is an interrupt, which is not handled by the primary CPU interrupt chips. It's a synthetic interrupt which is generated from the received USB packet.
+ /* USB interrupts are received in softirq (tasklet) context. + * Switch to hardirq context to make genirq code happy. + */ + local_irq_save(flags); + __irq_enter_raw(); + if (intdata & INT_ENP_PHY_INT_) - ; + generic_handle_domain_irq(pdata->irqdomain, PHY_HWIRQ);
This __irq_enter_raw() is really wrong. This is _not_ running in hard interrupt context. Pretending so creates more problems than it solves. It breaks context tracking, confuses lockdep ...
We also have demultiplexed interrupts which are nested in a threaded interrupt handler and share the thread context. No, we are not going to pretend that they run in hard interrupt context either.
So we need a clear distinction between interrupts which really happen in hard interrupt context and those which are synthetic and can be invoked from pretty much any context.
Anything else is just a recipe for disaster and endless supply of half baken hacks.
Thanks,
tglx
| |