Re: [PATCH 07/12] usb: usbtest: use irqsave() in USB's complete callback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
>  drivers/usb/misc/usbtest.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index 9e1142b8b91b..3b56fb34906c 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -1082,11 +1082,12 @@ static void ctrl_complete(struct urb *urb)
>  	struct usb_ctrlrequest	*reqp;
>  	struct subcase		*subcase;
>  	int			status = urb->status;
> +	unsigned long		flags;
>  
>  	reqp = (struct usb_ctrlrequest *)urb->setup_packet;
>  	subcase = container_of(reqp, struct subcase, setup);
>  
> -	spin_lock(&ctx->lock);
> +	spin_lock_irqsave(&ctx->lock, flags);
>  	ctx->count--;
>  	ctx->pending--;
>  
> @@ -1152,9 +1153,9 @@ static void ctrl_complete(struct urb *urb)
>  
>  				if (u == urb || !u->dev)
>  					continue;
> -				spin_unlock(&ctx->lock);
> +				spin_unlock_irqrestore(&ctx->lock, flags);
>  				status = usb_unlink_urb(u);
> -				spin_lock(&ctx->lock);
> +				spin_lock_irqsave(&ctx->lock, flags);

My feeling tends to be that when a lock is temporarily dropped to take 
a single action like this, there's no need to restore the 
interrupt-enable flag.  These two calls can remain 
spin_unlock()/spin_lock().

Aside from this:

Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

Alan Stern

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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux