RE: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler

From: John Youn
Date: Wed Dec 28 2016 - 20:39:57 EST




> -----Original Message-----
> From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Felipe Balbi
> Sent: Wednesday, December 28, 2016 8:19 AM
> To: Janusz Dziedzic <janusz.dziedzic@xxxxxxxxx>
> Cc: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>; Baolin Wang
> <baolin.wang@xxxxxxxxxx>; Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>; USB
> <linux-usb@xxxxxxxxxxxxxxx>; LKML <linux-kernel@xxxxxxxxxxxxxxx>; Linaro
> Kernel Mailman List <linaro-kernel@xxxxxxxxxxxxxxxx>; Mark Brown
> <broonie@xxxxxxxxxx>
> Subject: Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt
> handler and irq thread handler
>
>
> Hi,
>
> Janusz Dziedzic <janusz.dziedzic@xxxxxxxxx> writes:
> >>>> On some platfroms(like x86 platform), when one core is running the
> USB gadget
> >>>> irq thread handler by dwc3_thread_interrupt(), meanwhile another
> core also can
> >>>> respond other interrupts from dwc3 controller and modify the event
> buffer by
> >>>> dwc3_interrupt() function, that will cause getting the wrong event
> count in
> >>>> irq thread handler to make the USB function abnormal.
> >>>>
> >>>> We should add spin_lock/unlock() in dwc3_check_event_buf() to avoid
> this race.
> >>>
> >>> Why not spin_lock_irq ones? This lock seems to be used in both
> >>> normal and interrupt threads. Or, I missed anything?
> >>
> >> this is top half handler. Interrupts are already disabled.
> >>
> > BTW,
> > We don't use spin_lock in top half handler.
> > Maybe we should/can switch all spin_lock_irqsave() to simple
> > spin_lock() in the thread/callbacks?
>
> in theory, yes we've masked all interrupts from this controller for the
> duration of the thread handler. However this breaks networking
> gadgets. I can only guess network stack has a hard requirement to run
> with IRQs disabled.
>

Hi,

Is this version 3.00a of the core?

That version has a STAR where the interrupts cannot be masked. That results in similar symptoms to what you're seeing here.

Regards,
John