Skip to contentSkip to conversation

MUSB interrupt storm on device removal

DISCUSSION Johan Hovold 1 participant 1 message Jan 24, 2019 USB

lore
Share & tools
lore
#0 lore raw
Quoted 17 lines · Alan Stern
Quoted 15 lines · Bin Liu
Quoted 14 lines · Johan Hovold
Quoted 12 lines · Bin Liu
Quoted 8 lines · Johan Hovold
Quoted 4 lines · Greg Kroah-Hartman

That's not what any other host controller returns when a device is removed, so either you are going to have to fix all USB drives for this issue, or you need to fix the musb driver to not send this error for when a device is removed (hint, do the latter...)

Right, this needs to be handle at the HCD level.

Any reason usb_serial_generic_read_bulk_callback() doesn't handle -EPROTO in the same way as -EPIPE?

Quoted 8 lines
Quoted 3 lines
Quoted 3 lines

Since it is supposed to be intermittent unlike, for example, -ENOENT or -EPIPE (the latter which the device driver can recover from if it cares to implement clearing of halt).

Wait a minute. Nothing says any of those errors is supposed to be intermittent. As long as an error has a systematic cause (as opposed to random noise, for example), it will recur as often as the cause does.

Right, I should have said drivers tend to treat it as always being intermittent.

Quoted 4 lines

At least when -EPROTO errors are caused by device disconnect, we know that they will eventually go away when the upstream hub reports the port disconnect event. But until then, an interrupt storm is certainly possible.

Indeed, and this isn't the first time we've had this discussion either I realised.

In fact, I've been hit by this myself on BBB and musb when disconnecting a device connected through an external hub.

At the time, the immediate causes that were making the completion handler take longer than usual (unaligned copy of a large buffer and a printk respectively) could be fixed. The problem went away, but of course not the underlying issue.

Note that the problem I was seeing also went away both when switching to a different single-core SoC using ehci-omap, or when replacing the external hub. IIRC it wasn't the hub workqueue that was starved as a I first had thought, but the hub interrupt was never even received (or processed at least).

Unfortunately, I never got around to investigating this further.

Quoted 11 lines
Quoted 6 lines
Quoted 3 lines

My point was that unless you fix this at the HCD level, you will need to add complex recovery handling to every USB driver and completion handler (~500 of those). But perhaps that is what it needed.

okay, it probably make sense to handle the case in HCD because the number of HCD is much less.

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.

Quoted 8 lines
Quoted 4 lines
Quoted 4 lines

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.

I tend to agree with you on that, but adding complex back-off handling of intermittent errors to every USB driver and completion handler will be quite a bit of work. Unless the HCD drivers can assist, we'd at least want have some part of the implementation provided by shared code.

Also note that simply starting to bail out on any error now would risk introducing regressions for setups where intermittent errors do occur.

Quoted 4 lines
Quoted 2 lines

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?

I think we should get to the bottom of that. BBB is single core which may be part of the reason why it's affected, but it's definitely related to the controller as well.

Johan