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 <[email protected]> > Signed-off-by: Sebastian Andrzej Siewior <[email protected]> > --- > 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 <[email protected]> Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
