Messages in this thread Patch in this message | | | Subject | [PATCH] usb-serial: possible irq lock inversion (PPP vs. usb/serial) | From | Peter Zijlstra <> | Date | Mon, 25 Sep 2006 12:51:41 +0200 |
| |
========================================================= [ 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 <a.p.zijlstra@chello.nl> --- 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |