On 18.02.2022 18:32, Sebastian Andrzej Siewior wrote: > The interrupt service routine registered for the gadget is a primary > handler which mask the interrupt source and a threaded handler which > handles the source of the interrupt. Since the threaded handler is > voluntary threaded, the IRQ-core does not disable bottom halves before > invoke the handler like it does for the forced-threaded handler. > > Due to changes in networking it became visible that a network gadget's > completions handler may schedule a softirq which remains unprocessed. > The gadget's completion handler is usually invoked either in hard-IRQ or > soft-IRQ context. In this context it is enough to just raise the softirq > because the softirq itself will be handled once that context is left. > In the case of the voluntary threaded handler, there is nothing that > will process pending softirqs. Which means it remain queued until > another random interrupt (on this CPU) fires and handles it on its exit > path or another thread locks and unlocks a lock with the bh suffix. > Worst case is that the CPU goes idle and the NOHZ complains about > unhandled softirqs. > > Disable bottom halves before acquiring the lock (and disabling > interrupts) and enable them after dropping the lock. This ensures that > any pending softirqs will handled right away. > > Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> > Link: https://lkml.kernel.org/r/c2a64979-73d1-2c22-e048-c275c9f81558@xxxxxxxxxxx > Fixes: e5f68b4a3e7b0 ("Revert "usb: dwc3: gadget: remove unnecessary _irqsave()"") > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> > --- > drivers/usb/dwc3/gadget.c | 2 ++ > 1 file changed, 2 insertions(+) > > 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; > } Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
