[PATCH] usb-serial: possible irq lock inversion (PPP vs. usb/serial)

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

 



=========================================================
[ INFO: possible irq lock inversion dependency detected ]
---------------------------------------------------------
ksoftirqd/0/3 just changed the state of lock:
 (&ap->xmit_lock){-+..}, at: [<f9337224>] ppp_async_push+0x2f/0x3b3 [ppp_async]
but this lock took another, soft-irq-unsafe lock in the past:
 (&port->lock){--..}

and interrupts could create inverse lock ordering between them.


other info that might help us debug this:
no locks held by ksoftirqd/0/3.

the first lock's dependencies:
-> (&ap->xmit_lock){-+..} ops: 0 {
   initial-use  at:
                        [<c043bf43>] lock_acquire+0x4b/0x6c
                        [<c06086a8>] _spin_lock_bh+0x1e/0x2d
                        [<f9337224>] ppp_async_push+0x2f/0x3b3 [ppp_async]
                        [<f93375b8>] ppp_async_send+0x10/0x3d [ppp_async]
                        [<f932f071>] ppp_channel_push+0x3a/0x94 [ppp_generic]
                        [<f9330395>] ppp_write+0xd5/0xe1 [ppp_generic]
                        [<c0471f23>] vfs_write+0xab/0x157
                        [<c0472568>] sys_write+0x3b/0x60
                        [<c0403faf>] syscall_call+0x7/0xb
   in-softirq-W at:
                        [<c043bf43>] lock_acquire+0x4b/0x6c
                        [<c06086a8>] _spin_lock_bh+0x1e/0x2d
                        [<f9337224>] ppp_async_push+0x2f/0x3b3 [ppp_async]
                        [<f9337aea>] ppp_async_process+0x48/0x5b [ppp_async]
                        [<c04294b4>] tasklet_action+0x65/0xca
                        [<c04293d5>] __do_softirq+0x78/0xf2
                        [<c040662f>] do_softirq+0x5a/0xbe
   hardirq-on-W at:
                        [<c043bf43>] lock_acquire+0x4b/0x6c
                        [<c06086a8>] _spin_lock_bh+0x1e/0x2d
                        [<f9337224>] ppp_async_push+0x2f/0x3b3 [ppp_async]
                        [<f93375b8>] ppp_async_send+0x10/0x3d [ppp_async]
                        [<f932f071>] ppp_channel_push+0x3a/0x94 [ppp_generic]
                        [<f9330395>] ppp_write+0xd5/0xe1 [ppp_generic]
                        [<c0471f23>] vfs_write+0xab/0x157
                        [<c0472568>] sys_write+0x3b/0x60
                        [<c0403faf>] syscall_call+0x7/0xb
 }
 ... key      at: [<f933b208>] __key.19284+0x0/0xffffce72 [ppp_async]
 -> (&port->lock){--..} ops: 0 {
    initial-use  at:
                          [<c043bf43>] lock_acquire+0x4b/0x6c
                          [<c060867b>] _spin_lock+0x19/0x28
                          [<f9324478>] usb_serial_generic_write+0x79/0x23d [usbserial]
                          [<f9322531>] serial_write+0x8a/0x99 [usbserial]
                          [<c052dbed>] write_chan+0x22e/0x2a8
                          [<c052b530>] tty_write+0x148/0x1ce
                          [<c0471f23>] vfs_write+0xab/0x157
                          [<c0472568>] sys_write+0x3b/0x60
                          [<c0403faf>] syscall_call+0x7/0xb
    softirq-on-W at:
                          [<c043bf43>] lock_acquire+0x4b/0x6c
                          [<c060867b>] _spin_lock+0x19/0x28
                          [<f9324478>] usb_serial_generic_write+0x79/0x23d [usbserial]
                          [<f9322531>] serial_write+0x8a/0x99 [usbserial]
                          [<c052dbed>] write_chan+0x22e/0x2a8
                          [<c052b530>] tty_write+0x148/0x1ce
                          [<c0471f23>] vfs_write+0xab/0x157
                          [<c0472568>] sys_write+0x3b/0x60
                          [<c0403faf>] syscall_call+0x7/0xb
    hardirq-on-W at:
                          [<c043bf43>] lock_acquire+0x4b/0x6c
                          [<c060867b>] _spin_lock+0x19/0x28
                          [<f9324478>] usb_serial_generic_write+0x79/0x23d [usbserial]
                          [<f9322531>] serial_write+0x8a/0x99 [usbserial]
                          [<c052dbed>] write_chan+0x22e/0x2a8
                          [<c052b530>] tty_write+0x148/0x1ce
                          [<c0471f23>] vfs_write+0xab/0x157
                          [<c0472568>] sys_write+0x3b/0x60
                          [<c0403faf>] syscall_call+0x7/0xb
  }
  ... key      at: [<f932b08c>] __key.15523+0x0/0xffff9965 [usbserial]
 ... acquired at:
   [<c043bf43>] lock_acquire+0x4b/0x6c
   [<c060867b>] _spin_lock+0x19/0x28
   [<f9324478>] usb_serial_generic_write+0x79/0x23d [usbserial]
   [<f9322531>] serial_write+0x8a/0x99 [usbserial]
   [<f933729c>] ppp_async_push+0xa7/0x3b3 [ppp_async]
   [<f93375da>] ppp_async_send+0x32/0x3d [ppp_async]
   [<f932f071>] ppp_channel_push+0x3a/0x94 [ppp_generic]
   [<f9330395>] ppp_write+0xd5/0xe1 [ppp_generic]
   [<c0471f23>] vfs_write+0xab/0x157
   [<c0472568>] sys_write+0x3b/0x60
   [<c0403faf>] syscall_call+0x7/0xb

Change port->lock locking to softirq-safe, other locks down the
call-chain appear to be hardirq-safe so this should do.

(compile tested only due to lack of hardware)

Signed-off-by: Peter Zijlstra <[email protected]>
---
 drivers/usb/serial/cyberjack.c   |    6 +++---
 drivers/usb/serial/generic.c     |    6 +++---
 drivers/usb/serial/ipw.c         |    6 +++---
 drivers/usb/serial/ir-usb.c      |    6 +++---
 drivers/usb/serial/keyspan_pda.c |    6 +++---
 drivers/usb/serial/omninet.c     |    6 +++---
 drivers/usb/serial/safe_serial.c |    6 +++---
 7 files changed, 21 insertions(+), 21 deletions(-)

Index: linux-2.6-mm/drivers/usb/serial/cyberjack.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/cyberjack.c
+++ linux-2.6-mm/drivers/usb/serial/cyberjack.c
@@ -214,14 +214,14 @@ static int cyberjack_write (struct usb_s
 		return (0);
 	}
 
-	spin_lock(&port->lock);
+	spin_lock_bh(&port->lock);
 	if (port->write_urb_busy) {
-		spin_unlock(&port->lock);
+		spin_unlock_bh(&port->lock);
 		dbg("%s - already writing", __FUNCTION__);
 		return 0;
 	}
 	port->write_urb_busy = 1;
-	spin_unlock(&port->lock);
+	spin_unlock_bh(&port->lock);
 
 	spin_lock_irqsave(&priv->lock, flags);
 
Index: linux-2.6-mm/drivers/usb/serial/generic.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/generic.c
+++ linux-2.6-mm/drivers/usb/serial/generic.c
@@ -175,14 +175,14 @@ int usb_serial_generic_write(struct usb_
 
 	/* only do something if we have a bulk out endpoint */
 	if (serial->num_bulk_out) {
-		spin_lock(&port->lock);
+		spin_lock_bh(&port->lock);
 		if (port->write_urb_busy) {
-			spin_unlock(&port->lock);
+			spin_unlock_bh(&port->lock);
 			dbg("%s - already writing", __FUNCTION__);
 			return 0;
 		}
 		port->write_urb_busy = 1;
-		spin_unlock(&port->lock);
+		spin_unlock_bh(&port->lock);
 
 		count = (count > port->bulk_out_size) ? port->bulk_out_size : count;
 
Index: linux-2.6-mm/drivers/usb/serial/ipw.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/ipw.c
+++ linux-2.6-mm/drivers/usb/serial/ipw.c
@@ -394,14 +394,14 @@ static int ipw_write(struct usb_serial_p
 		return 0;
 	}
 
-	spin_lock(&port->lock);
+	spin_lock_bh(&port->lock);
 	if (port->write_urb_busy) {
-		spin_unlock(&port->lock);
+		spin_unlock_bh(&port->lock);
 		dbg("%s - already writing", __FUNCTION__);
 		return 0;
 	}
 	port->write_urb_busy = 1;
-	spin_unlock(&port->lock);
+	spin_unlock_bh(&port->lock);
 
 	count = min(count, port->bulk_out_size);
 	memcpy(port->bulk_out_buffer, buf, count);
Index: linux-2.6-mm/drivers/usb/serial/ir-usb.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/ir-usb.c
+++ linux-2.6-mm/drivers/usb/serial/ir-usb.c
@@ -342,14 +342,14 @@ static int ir_write (struct usb_serial_p
 	if (count == 0)
 		return 0;
 
-	spin_lock(&port->lock);
+	spin_lock_bh(&port->lock);
 	if (port->write_urb_busy) {
-		spin_unlock(&port->lock);
+		spin_unlock_bh(&port->lock);
 		dbg("%s - already writing", __FUNCTION__);
 		return 0;
 	}
 	port->write_urb_busy = 1;
-	spin_unlock(&port->lock);
+	spin_unlock_bh(&port->lock);
 
 	transfer_buffer = port->write_urb->transfer_buffer;
 	transfer_size = min(count, port->bulk_out_size - 1);
Index: linux-2.6-mm/drivers/usb/serial/keyspan_pda.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/keyspan_pda.c
+++ linux-2.6-mm/drivers/usb/serial/keyspan_pda.c
@@ -518,13 +518,13 @@ static int keyspan_pda_write(struct usb_
 	   the TX urb is in-flight (wait until it completes)
 	   the device is full (wait until it says there is room)
 	*/
-	spin_lock(&port->lock);
+	spin_lock_bh(&port->lock);
 	if (port->write_urb_busy || priv->tx_throttled) {
-		spin_unlock(&port->lock);
+		spin_unlock_bh(&port->lock);
 		return 0;
 	}
 	port->write_urb_busy = 1;
-	spin_unlock(&port->lock);
+	spin_unlock_bh(&port->lock);
 
 	/* At this point the URB is in our control, nobody else can submit it
 	   again (the only sudden transition was the one from EINPROGRESS to
Index: linux-2.6-mm/drivers/usb/serial/omninet.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/omninet.c
+++ linux-2.6-mm/drivers/usb/serial/omninet.c
@@ -256,14 +256,14 @@ static int omninet_write (struct usb_ser
 		return (0);
 	}
 
-	spin_lock(&wport->lock);
+	spin_lock_bh(&wport->lock);
 	if (wport->write_urb_busy) {
-		spin_unlock(&wport->lock);
+		spin_unlock_bh(&wport->lock);
 		dbg("%s - already writing", __FUNCTION__);
 		return 0;
 	}
 	wport->write_urb_busy = 1;
-	spin_unlock(&wport->lock);
+	spin_unlock_bh(&wport->lock);
 
 	count = (count > OMNINET_BULKOUTSIZE) ? OMNINET_BULKOUTSIZE : count;
 
Index: linux-2.6-mm/drivers/usb/serial/safe_serial.c
===================================================================
--- linux-2.6-mm.orig/drivers/usb/serial/safe_serial.c
+++ linux-2.6-mm/drivers/usb/serial/safe_serial.c
@@ -298,14 +298,14 @@ static int safe_write (struct usb_serial
 		dbg ("%s - write request of 0 bytes", __FUNCTION__);
 		return (0);
 	}
-	spin_lock(&port->lock);
+	spin_lock_bh(&port->lock);
 	if (port->write_urb_busy) {
-		spin_unlock(&port->lock);
+		spin_unlock_bh(&port->lock);
 		dbg("%s - already writing", __FUNCTION__);
 		return 0;
 	}
 	port->write_urb_busy = 1;
-	spin_unlock(&port->lock);
+	spin_unlock_bh(&port->lock);
 
 	packet_length = port->bulk_out_size;	// get max packetsize
 


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux