[05/12] usb: usblp: use irqsave() in USB's complete callback

Commit Message

Sebastian Andrzej Siewior June 24, 2018, 10:08 p.m. UTC

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: Pete Zaitcev <[e-mail]>
Cc: Greg Kroah-Hartman <[e-mail]>
Signed-off-by: Sebastian Andrzej Siewior <[e-mail]>
---
 drivers/usb/class/usblp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Pete Zaitcev June 25, 2018, 3:58 p.m. UTC | #1

On Mon, 25 Jun 2018 00:08:38 +0200
Sebastian Andrzej Siewior <[e-mail]> wrote:

> Use the _irqsave() variant of the locking primitives.

>  	usblp->wcomplete = 1;
>  	wake_up(&usblp->wwait);
> -	spin_unlock(&usblp->lock);
> +	spin_unlock_irqrestore(&usblp->lock, flags);

Code is correct, addresses the stated problem, no forgotten parts
found left to fix.

Acked-by: Pete Zaitcev <[e-mail]>

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

Patch

diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
index d058d7a31e7c..407a7a6198a2 100644
--- a/drivers/usb/class/usblp.c
+++ b/drivers/usb/class/usblp.c
@@ -292,6 +292,7 @@  static void usblp_bulk_read(struct urb *urb)
 {
 	struct usblp *usblp = urb->context;
 	int status = urb->status;
+	unsigned long flags;
 
 	if (usblp->present && usblp->used) {
 		if (status)
@@ -299,14 +300,14 @@  static void usblp_bulk_read(struct urb *urb)
 			    "nonzero read bulk status received: %d\n",
 			    usblp->minor, status);
 	}
-	spin_lock(&usblp->lock);
+	spin_lock_irqsave(&usblp->lock, flags);
 	if (status < 0)
 		usblp->rstatus = status;
 	else
 		usblp->rstatus = urb->actual_length;
 	usblp->rcomplete = 1;
 	wake_up(&usblp->rwait);
-	spin_unlock(&usblp->lock);
+	spin_unlock_irqrestore(&usblp->lock, flags);
 
 	usb_free_urb(urb);
 }
@@ -315,6 +316,7 @@  static void usblp_bulk_write(struct urb *urb)
 {
 	struct usblp *usblp = urb->context;
 	int status = urb->status;
+	unsigned long flags;
 
 	if (usblp->present && usblp->used) {
 		if (status)
@@ -322,7 +324,7 @@  static void usblp_bulk_write(struct urb *urb)
 			    "nonzero write bulk status received: %d\n",
 			    usblp->minor, status);
 	}
-	spin_lock(&usblp->lock);
+	spin_lock_irqsave(&usblp->lock, flags);
 	if (status < 0)
 		usblp->wstatus = status;
 	else
@@ -330,7 +332,7 @@  static void usblp_bulk_write(struct urb *urb)
 	usblp->no_paper = 0;
 	usblp->wcomplete = 1;
 	wake_up(&usblp->wwait);
-	spin_unlock(&usblp->lock);
+	spin_unlock_irqrestore(&usblp->lock, flags);
 
 	usb_free_urb(urb);
 }