On Fri, Apr 01, 2022 at 12:47:00AM +0300, Tasos Sahanidis wrote:
> Since commit ae8709b296d8 ("USB: core: Make do_proc_control() and
> do_proc_bulk() killable") if a device has the USB_QUIRK_DELAY_CTRL_MSG
> quirk set, it will temporarily block all other URBs (e.g. interrupts)
> while sleeping due to a control.
>
> This results in noticeable delays when, for example, a userspace usbfs
> application is sending URB interrupts at a high rate to a keyboard and
> simultaneously updates the lock indicators using controls. Interrupts
> with direction set to IN are also affected by this, meaning that
> delivery of HID reports (containing scancodes) to the usbfs application
> is delayed as well.
>
> This patch fixes the regression by calling msleep() while the device
> mutex is unlocked, as was the case originally with usb_control_msg().
>
> Fixes: ae8709b296d8 ("USB: core: Make do_proc_control() and do_proc_bulk() killable")
> Signed-off-by: Tasos Sahanidis <tasos@xxxxxxxxxxxx>
> ---
Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> v2: Resent as v1 got mangled
> v3: Renamed from: "usb: core: Don't block while sleeping in do_proc_control()"
>
> drivers/usb/core/devio.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 6abb7294e919..b5b85bf80329 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1209,12 +1209,16 @@ static int do_proc_control(struct usb_dev_state *ps,
>
> usb_unlock_device(dev);
> i = usbfs_start_wait_urb(urb, tmo, &actlen);
> +
> + /* Linger a bit, prior to the next control message. */
> + if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
> + msleep(200);
> usb_lock_device(dev);
> snoop_urb(dev, NULL, pipe, actlen, i, COMPLETE, tbuf, actlen);
> if (!i && actlen) {
> if (copy_to_user(ctrl->data, tbuf, actlen)) {
> ret = -EFAULT;
> - goto recv_fault;
> + goto done;
> }
> }
> } else {
> @@ -1231,6 +1235,10 @@ static int do_proc_control(struct usb_dev_state *ps,
>
> usb_unlock_device(dev);
> i = usbfs_start_wait_urb(urb, tmo, &actlen);
> +
> + /* Linger a bit, prior to the next control message. */
> + if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
> + msleep(200);
> usb_lock_device(dev);
> snoop_urb(dev, NULL, pipe, actlen, i, COMPLETE, NULL, 0);
> }
> @@ -1242,10 +1250,6 @@ static int do_proc_control(struct usb_dev_state *ps,
> }
> ret = (i < 0 ? i : actlen);
>
> - recv_fault:
> - /* Linger a bit, prior to the next control message. */
> - if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
> - msleep(200);
> done:
> kfree(dr);
> usb_free_urb(urb);
> --
> 2.25.1
>
