On 2022-02-17 15:08:55 [+0100], Marek Szyprowski wrote:
> Hi All,
Hi,
> >> Marek, does this work for you?
> >
> > Yes, this fixed the issue. Thanks!
> >
> > Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
>
> I've just noticed that there is one more issue left to fix (the $subject
> patch is already applied) - this one comes from threaded irq (if I got
> the stack trace right):
netif_rx() did only set the matching softirq bit and not more. Based on
that I don't see why NOHZ shouldn't complain about a pending softirq
once the CPU goes idle. Therefore I think the change I made is good
since it uncovered that.
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 147 at kernel/softirq.c:363
> __local_bh_enable_ip+0xa8/0x1c0
…
> CPU: 0 PID: 147 Comm: irq/150-dwc3 Not tainted 5.17.0-rc4-next-20220217+
> #4557
> Hardware name: Samsung TM2E board (DT)
> pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : __local_bh_enable_ip+0xa8/0x1c0
> lr : netif_rx+0xa4/0x2c0
> ...
>
> Call trace:
> __local_bh_enable_ip+0xa8/0x1c0
> netif_rx+0xa4/0x2c0
> rx_complete+0x214/0x250
> usb_gadget_giveback_request+0x58/0x170
> dwc3_gadget_giveback+0xe4/0x200
> dwc3_gadget_endpoint_trbs_complete+0x100/0x388
> dwc3_thread_interrupt+0x46c/0xe20
So dwc3_thread_interrupt() disables interrupts here. Felipe dropped it
and then added it back in
e5f68b4a3e7b0 ("Revert "usb: dwc3: gadget: remove unnecessary _irqsave()"")
I would suggest to revert it (the above commit) and fixing the lockdep
splat in the gadget driver and other. I don't see the g_ether warning
Felipe mentioned. It might come from f_ncm (since it uses a timer) or
something else in the network stack (that uses a timeout timer).
But not now.
As much as I hate it, I suggest:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 183b90923f51b..a0c883f19a417 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -4160,9 +4160,11 @@ static irqreturn_t dwc3_thread_interrupt(int irq, void *_evt)
unsigned long flags;
irqreturn_t ret = IRQ_NONE;
+ local_bh_disable();
spin_lock_irqsave(&dwc->lock, flags);
ret = dwc3_process_event_buf(evt);
spin_unlock_irqrestore(&dwc->lock, flags);
+ local_bh_enable();
return ret;
}
In the long run I would drop that irqsave (along with bh_disable() since
netif_rx() covers that) and make sure the there is no lockdep warning
popping up.
Marek, could you please give it a try?
Sebastian
