On Mon, 7 Jan 2019, Greg KH wrote:
> On Tue, Nov 20, 2018 at 03:34:38PM +0100, Nicolas Saenz Julienne wrote:
> > The hub sends hot-plug events to the host trough it's interrupt URB. The
> > driver takes care of completing the URB and re-submitting it. Completion
> > errors are handled in the hub_event() work, yet submission errors are
> > ignored, rendering the device unresponsive. All further events are lost.
> >
> > It is fairly hard to find this issue in the wild, since you have to time
> > the USB hot-plug event with the URB submission failure. For instance it
> > could be the system running out of memory or some malfunction in the USB
> > controller driver. Nevertheless, it's pretty reasonable to think it'll
> > happen sometime. One can trigger this issue using eBPF's function
> > override feature (see BCC's inject.py script).
> >
> > This patch adds a retry routine to the event of a submission error. The
> > HUB driver will try to re-submit the URB once every second until it's
> > successful or the HUB is disconnected.
> >
> > As some USB subsystems already take care of this issue, the
> > implementation was inspired from usbhid/hid_core.c's.
> >
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
> >
> > ---
>
> What ever happened to this patch? Is it still needed? Oliver and Alan,
> any objections about it anymore?
I have just one very minor nit to pick (see below). Mostly I've been
waiting to hear from Oliver.
Alan Stern
>
> thanks,
>
> greg k-h
>
> >
> > v3: As per Oliver's request:
> > - Take care of race condition between disconnect and irq
> >
> > v2: as per Alan's and Oliver's comments:
> > - Rename timer
> > - Delete the timer on disconnect
> > - Don't reset HUB nor exponential slowdown the timer, 1s fixed retry
> > period
> > - Check for -ESHUTDOWN prior kicking the timer
> >
> > drivers/usb/core/hub.c | 45 ++++++++++++++++++++++++++++++++++++------
> > drivers/usb/core/hub.h | 2 ++
> > 2 files changed, 41 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index c6077d582d29..630490a35301 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -607,6 +607,38 @@ static int hub_port_status(struct usb_hub *hub, int port1,
> > status, change, NULL);
> > }
> >
> > +static void hub_resubmit_irq_urb(struct usb_hub *hub)
> > +{
> > + unsigned long flags;
> > + int status;
> > +
> > + spin_lock_irqsave(&hub->irq_urb_lock, flags);
> > +
> > + if (hub->quiescing) {
> > + spin_unlock_irqrestore(&hub->irq_urb_lock, flags);
> > + return;
> > + }
> > +
> > + status = usb_submit_urb(hub->urb, GFP_ATOMIC);
> > + if (status && status != -ENODEV && status != -EPERM &&
> > + status != -ESHUTDOWN) {
> > + dev_err(hub->intfdev, "resubmit --> %d\n", status);
> > + mod_timer(&hub->irq_urb_retry,
> > + jiffies + msecs_to_jiffies(MSEC_PER_SEC));
This can be written more simply as:
mod_timer(&hub->irq_urb_retry, jiffies + HZ);
