Re: [PATCH] usb: dwc3: gadget: Let the interrupt handler disable bottom halves.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux