On Mon, 25 Jun 2018, Sebastian Andrzej Siewior wrote:

> The USB completion callback does not disable interrupts while acquiring
> the lock. We want to remove the local_irq_disable() invocation from
> __usb_hcd_giveback_urb() and therefore it is required for the callback
> handler to disable the interrupts while acquiring the lock.
> The callback may be invoked either in IRQ or BH context depending on the
> USB host controller.
> Use the _irqsave() variant of the locking primitives.
> 
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Cc: Alan Stern <st...@rowland.harvard.edu>
> Signed-off-by: Sebastian Andrzej Siewior <bige...@linutronix.de>
> ---
>  drivers/usb/core/devio.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 476dcc5f2da3..cea35fa4e7a3 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -585,9 +585,10 @@ static void async_completed(struct urb *urb)
>       struct siginfo sinfo;
>       struct pid *pid = NULL;
>       const struct cred *cred = NULL;
> +     unsigned long flags;
>       int signr;
>  
> -     spin_lock(&ps->lock);
> +     spin_lock_irqsave(&ps->lock, flags);
>       list_move_tail(&as->asynclist, &ps->async_completed);
>       as->status = urb->status;
>       signr = as->signr;
> @@ -611,7 +612,7 @@ static void async_completed(struct urb *urb)
>               cancel_bulk_urbs(ps, as->bulk_addr);
>  
>       wake_up(&ps->wait);
> -     spin_unlock(&ps->lock);
> +     spin_unlock_irqrestore(&ps->lock, flags);
>  
>       if (signr) {
>               kill_pid_info_as_cred(sinfo.si_signo, &sinfo, pid, cred);
> @@ -1702,6 +1703,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
> struct usbdevfs_urb *uurb
>  
>       as->urb->context = as;
>       as->urb->complete = async_completed;
> +

This change is unrelated to the purpose of the patch.  It should be 
removed.

>       for (totlen = u = 0; u < number_of_packets; u++) {
>               as->urb->iso_frame_desc[u].offset = totlen;
>               as->urb->iso_frame_desc[u].length = isopkt[u].length;

Aside from that:

Acked-by: Alan Stern <st...@rowland.harvard.edu>

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to