The patch titled
usb-serial: possible irq lock inversion (PPP vs. usb/serial)
has been added to the -mm tree. Its filename is
usb-serial-possible-irq-lock-inversion-ppp-vs.patch
See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
out what to do about this
------------------------------------------------------
Subject: usb-serial: possible irq lock inversion (PPP vs. usb/serial)
From: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
=========================================================
[ 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.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
Cc: Greg KH <greg@xxxxxxxxx>
Cc: Paul Mackerras <paulus@xxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
---
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(-)
diff -puN drivers/usb/serial/cyberjack.c~usb-serial-possible-irq-lock-inversion-ppp-vs drivers/usb/serial/cyberjack.c
--- a/drivers/usb/serial/cyberjack.c~usb-serial-possible-irq-lock-inversion-ppp-vs
+++ a/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);
diff -puN drivers/usb/serial/generic.c~usb-serial-possible-irq-lock-inversion-ppp-vs drivers/usb/serial/generic.c
--- a/drivers/usb/serial/generic.c~usb-serial-possible-irq-lock-inversion-ppp-vs
+++ a/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;
diff -puN drivers/usb/serial/ipw.c~usb-serial-possible-irq-lock-inversion-ppp-vs drivers/usb/serial/ipw.c
--- a/drivers/usb/serial/ipw.c~usb-serial-possible-irq-lock-inversion-ppp-vs
+++ a/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);
diff -puN drivers/usb/serial/ir-usb.c~usb-serial-possible-irq-lock-inversion-ppp-vs drivers/usb/serial/ir-usb.c
--- a/drivers/usb/serial/ir-usb.c~usb-serial-possible-irq-lock-inversion-ppp-vs
+++ a/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);
diff -puN drivers/usb/serial/keyspan_pda.c~usb-serial-possible-irq-lock-inversion-ppp-vs drivers/usb/serial/keyspan_pda.c
--- a/drivers/usb/serial/keyspan_pda.c~usb-serial-possible-irq-lock-inversion-ppp-vs
+++ a/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
diff -puN drivers/usb/serial/omninet.c~usb-serial-possible-irq-lock-inversion-ppp-vs drivers/usb/serial/omninet.c
--- a/drivers/usb/serial/omninet.c~usb-serial-possible-irq-lock-inversion-ppp-vs
+++ a/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;
diff -puN drivers/usb/serial/safe_serial.c~usb-serial-possible-irq-lock-inversion-ppp-vs drivers/usb/serial/safe_serial.c
--- a/drivers/usb/serial/safe_serial.c~usb-serial-possible-irq-lock-inversion-ppp-vs
+++ a/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
_
Patches currently in -mm which might be from a.p.zijlstra@xxxxxxxxx are
origin.patch
mm-tracking-shared-dirty-pages.patch
mm-balance-dirty-pages.patch
mm-optimize-the-new-mprotect-code-a-bit.patch
mm-small-cleanup-of-install_page.patch
mm-fixup-do_wp_page.patch
mm-msync-cleanup.patch
mm-swap-write-failure-fixup.patch
forcedeth-hardirq-lockdep-warning.patch
usb-serial-possible-irq-lock-inversion-ppp-vs.patch
block_devc-mutex_lock_nested-fix.patch
remove-the-old-bd_mutex-lockdep-annotation.patch
new-bd_mutex-lockdep-annotation.patch
nfsd-lockdep-annotation.patch
-
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
