On Mo, 2018-11-19 at 15:02 +0100, Nicolas Saenz Julienne wrote:
>
> +static void hub_retry_irq_urb(struct timer_list *t)
> +{
> + struct usb_hub *hub = from_timer(hub, t, irq_urb_retry);
> + int status;
> +
> + if (hub->disconnected || hub->quiescing)
> + return;
> +
> + dev_err(hub->intfdev, "retrying int urb\n");
> + status = usb_submit_urb(hub->urb, GFP_ATOMIC);
> + if (status && status != -ENODEV && status != -EPERM &&
> + status != -ESHUTDOWN)
> + mod_timer(&hub->irq_urb_retry,
> + jiffies + msecs_to_jiffies(MSEC_PER_SEC));
> +}
> +
> static void kick_hub_wq(struct usb_hub *hub)
> {
> struct usb_interface *intf;
> @@ -713,8 +729,12 @@ static void hub_irq(struct urb *urb)
> return;
>
> status = usb_submit_urb(hub->urb, GFP_ATOMIC);
> - if (status != 0 && status != -ENODEV && status != -EPERM)
> + if (status != 0 && 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));
> + }
> }
>
> /* USB 2.0 spec Section 11.24.2.3 */
> @@ -1268,6 +1288,7 @@ static void hub_quiesce(struct usb_hub *hub, enum hub_quiescing_type type)
> }
>
> /* Stop hub_wq and related activity */
> + del_timer_sync(&hub->irq_urb_retry);
That is a race condition. You kill the timer here, but the URB may
still be in flight. And if it fails, it will restart the error
handler. You have to introduce a flag or poison the URB.
> usb_kill_urb(hub->urb);
> if (hub->has_indicators)
> cancel_delayed_work_sync(&hub->leds);
>
Regards
Oliver
