MUSB interrupt storm on device removal

[Alan Stern <stern@rowland.harvard.edu>]

On Wed, 23 Jan 2019, Bin Liu wrote:

> On Wed, Jan 23, 2019 at 12:42:36PM -0500, Alan Stern wrote:
> > On Wed, 23 Jan 2019, Bin Liu wrote:
> > 
> > > > One possibility is to giveback URBs with certain errors (such as
> > > > -EPROTO) only at a frame boundary, or at 1-ms intervals.  This feels 
> > > > like a very artificial solution, though.
> > > 
> > > My plan is to add an error counter in musb driver endpoint struct, if
> > > -EPROTO has happened consequentially for a certain times, for example 3,
> > > giveback URBs with -EPIPE instead -EPROTO. This is the simplest solution
> > > I can think of, though I hate expending struct unnecessarily, this is
> > > one of the cases.
> > 
> > But -EPIPE is documented to mean that the device replied with a STALL.  
> > You would be violating the documentation.
> 
> (sigh...)

Yeah, I know.

If you really want to do this, you could update the documentation along 
with the driver.  I don't recommend it, but it's a possibility.

> > > > > > I do see now that of all USB drivers we have two drivers that handles
> > > > > > -EPROTO by resubmitting after a delay, while a handful explicitly deals
> > > > > > with -EPROTO by simply stopping to resubmit (some probably bail out on
> > > > > > all errors, but the majority appear to resubmit on -EPROTO).
> > > > 
> > > > Any driver which immediately retries an URB after getting -EPROTO or
> > > > -EILSEQ or -ETIME, and has no mechanism for backing off or limiting the
> > > > retries, is buggy.  As far as I can see, that's all there is to it.
> > > 
> > > Agreed, but given that majority appear to resubmit on -EPROTO as Johan
> > > said, I think better to handle it in HCD.
> > 
> > Do all those other drivers handle -EPIPE correctly?  A logical way to 
> > respond to -EPIPE is to issue a Clear-Halt request; what will happen 
> > when that also fails?
> > 
> > I think it makes more sense to continue using -EPROTO but slow down the 
> > exchange of packets so that there is no interrupt storm.
> 
> the musb driver doesn't use SOF interrupts, so SOF interrupt is
> disabled. Another way to add delay for -EPROTO would be moving such URBs
> into a new linked list and using a timer to reschedule the list. The
> musb driver already has enough mess, I really don't want to add this
> logic if there is other simpler option...

I agree.  However, it might make sense.

In theory you could enable SOF interrupts whenever one of these errors 
occurs, just temporarily.  But I suppose that's not much better than 
using a timer...

> > > > > Thanks for the info.
> > > > > I will handle this case in musb driver.
> > > > 
> > > > Why doesn't the same problem occur with other types of host controller?
> > > 
> > > Not sure, I am on musb for most of the times. Maybe other HCD doesn't
> > > giveback URBs with -EPROTO in such error case.
> > 
> > ehci-hcd also uses -EPROTO.
> 
> Is it possible to test the use case on ehci?
> 
> - connect a multi-ports usb serial device to a hub;
> - open multiple ports with cat command;
> - remove the usb serial device from the hub;
> - console lockup happens?

I don't have a multi-port serial device to test with.  Can the test be 
carried out with a simple single-port device?

It shouldn't be too hard to find a PC with an EHCI controller.  xHCI 
might use -EPROTO too, I don't know.

Alan Stern

> > > musb controller has a register bit telling the controller has tried the
> > > transaction 3 times but didn't receive any response, then the musb
> > > driver just giveback this URB with -EPROTO.
> > 
> > Same with EHCI.
> 
> Regards,
> -Bin.