diff options
author | Dave Young <hidave.darkstar@gmail.com> | 2008-04-01 23:59:06 -0700 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2008-04-01 23:59:06 -0700 |
commit | 1905f6c736cb618e07eca0c96e60e3c024023428 (patch) | |
tree | 321a987569bf380fe1960879d03dd5e98fab92cc /net | |
parent | 68845cb2c82275efd7390026bba70c320ca6ef86 (diff) | |
download | lwn-1905f6c736cb618e07eca0c96e60e3c024023428.tar.gz lwn-1905f6c736cb618e07eca0c96e60e3c024023428.zip |
bluetooth : __rfcomm_dlc_close lock fix
Lockdep warning will be trigged while rfcomm connection closing.
The locks taken in rfcomm_dev_add:
rfcomm_dev_lock --> d->lock
In __rfcomm_dlc_close:
d->lock --> rfcomm_dev_lock (in rfcomm_dev_state_change)
There's two way to fix it, one is in rfcomm_dev_add we first locking
d->lock then the rfcomm_dev_lock
The other (in this patch), remove the locking of d->lock for
rfcomm_dev_state_change because just locking "d->state = BT_CLOSED;"
is enough.
[ 295.002046] =======================================================
[ 295.002046] [ INFO: possible circular locking dependency detected ]
[ 295.002046] 2.6.25-rc7 #1
[ 295.002046] -------------------------------------------------------
[ 295.002046] krfcommd/2705 is trying to acquire lock:
[ 295.002046] (rfcomm_dev_lock){-.--}, at: [<f89a090a>] rfcomm_dev_state_change+0x6a/0xd0 [rfcomm]
[ 295.002046]
[ 295.002046] but task is already holding lock:
[ 295.002046] (&d->lock){--..}, at: [<f899c533>] __rfcomm_dlc_close+0x43/0xd0 [rfcomm]
[ 295.002046]
[ 295.002046] which lock already depends on the new lock.
[ 295.002046]
[ 295.002046]
[ 295.002046] the existing dependency chain (in reverse order) is:
[ 295.002046]
[ 295.002046] -> #1 (&d->lock){--..}:
[ 295.002046] [<c0149b23>] check_prev_add+0xd3/0x200
[ 295.002046] [<c0149ce5>] check_prevs_add+0x95/0xe0
[ 295.002046] [<c0149f6f>] validate_chain+0x23f/0x320
[ 295.002046] [<c014b7b1>] __lock_acquire+0x1c1/0x760
[ 295.002046] [<c014c349>] lock_acquire+0x79/0xb0
[ 295.002046] [<c03d6b99>] _spin_lock+0x39/0x80
[ 295.002046] [<f89a01c0>] rfcomm_dev_add+0x240/0x360 [rfcomm]
[ 295.002046] [<f89a047e>] rfcomm_create_dev+0x6e/0xe0 [rfcomm]
[ 295.002046] [<f89a0823>] rfcomm_dev_ioctl+0x33/0x60 [rfcomm]
[ 295.002046] [<f899facc>] rfcomm_sock_ioctl+0x2c/0x50 [rfcomm]
[ 295.002046] [<c0363d38>] sock_ioctl+0x118/0x240
[ 295.002046] [<c0194196>] vfs_ioctl+0x76/0x90
[ 295.002046] [<c0194446>] do_vfs_ioctl+0x56/0x140
[ 295.002046] [<c0194569>] sys_ioctl+0x39/0x60
[ 295.002046] [<c0104faa>] syscall_call+0x7/0xb
[ 295.002046] [<ffffffff>] 0xffffffff
[ 295.002046]
[ 295.002046] -> #0 (rfcomm_dev_lock){-.--}:
[ 295.002046] [<c0149a84>] check_prev_add+0x34/0x200
[ 295.002046] [<c0149ce5>] check_prevs_add+0x95/0xe0
[ 295.002046] [<c0149f6f>] validate_chain+0x23f/0x320
[ 295.002046] [<c014b7b1>] __lock_acquire+0x1c1/0x760
[ 295.002046] [<c014c349>] lock_acquire+0x79/0xb0
[ 295.002046] [<c03d6639>] _read_lock+0x39/0x80
[ 295.002046] [<f89a090a>] rfcomm_dev_state_change+0x6a/0xd0 [rfcomm]
[ 295.002046] [<f899c548>] __rfcomm_dlc_close+0x58/0xd0 [rfcomm]
[ 295.002046] [<f899d44f>] rfcomm_recv_ua+0x6f/0x120 [rfcomm]
[ 295.002046] [<f899e061>] rfcomm_recv_frame+0x171/0x1e0 [rfcomm]
[ 295.002046] [<f899e357>] rfcomm_run+0xe7/0x550 [rfcomm]
[ 295.002046] [<c013c18c>] kthread+0x5c/0xa0
[ 295.002046] [<c0105c07>] kernel_thread_helper+0x7/0x10
[ 295.002046] [<ffffffff>] 0xffffffff
[ 295.002046]
[ 295.002046] other info that might help us debug this:
[ 295.002046]
[ 295.002046] 2 locks held by krfcommd/2705:
[ 295.002046] #0: (rfcomm_mutex){--..}, at: [<f899e2eb>] rfcomm_run+0x7b/0x550 [rfcomm]
[ 295.002046] #1: (&d->lock){--..}, at: [<f899c533>] __rfcomm_dlc_close+0x43/0xd0 [rfcomm]
[ 295.002046]
[ 295.002046] stack backtrace:
[ 295.002046] Pid: 2705, comm: krfcommd Not tainted 2.6.25-rc7 #1
[ 295.002046] [<c0128a38>] ? printk+0x18/0x20
[ 295.002046] [<c014927f>] print_circular_bug_tail+0x6f/0x80
[ 295.002046] [<c0149a84>] check_prev_add+0x34/0x200
[ 295.002046] [<c0149ce5>] check_prevs_add+0x95/0xe0
[ 295.002046] [<c0149f6f>] validate_chain+0x23f/0x320
[ 295.002046] [<c014b7b1>] __lock_acquire+0x1c1/0x760
[ 295.002046] [<c014c349>] lock_acquire+0x79/0xb0
[ 295.002046] [<f89a090a>] ? rfcomm_dev_state_change+0x6a/0xd0 [rfcomm]
[ 295.002046] [<c03d6639>] _read_lock+0x39/0x80
[ 295.002046] [<f89a090a>] ? rfcomm_dev_state_change+0x6a/0xd0 [rfcomm]
[ 295.002046] [<f89a090a>] rfcomm_dev_state_change+0x6a/0xd0 [rfcomm]
[ 295.002046] [<f899c548>] __rfcomm_dlc_close+0x58/0xd0 [rfcomm]
[ 295.002046] [<f899d44f>] rfcomm_recv_ua+0x6f/0x120 [rfcomm]
[ 295.002046] [<f899e061>] rfcomm_recv_frame+0x171/0x1e0 [rfcomm]
[ 295.002046] [<c014abd9>] ? trace_hardirqs_on+0xb9/0x130
[ 295.002046] [<c03d6e89>] ? _spin_unlock_irqrestore+0x39/0x70
[ 295.002046] [<f899e357>] rfcomm_run+0xe7/0x550 [rfcomm]
[ 295.002046] [<c03d4559>] ? __sched_text_start+0x229/0x4c0
[ 295.002046] [<c0120000>] ? cpu_avg_load_per_task+0x20/0x30
[ 295.002046] [<f899e270>] ? rfcomm_run+0x0/0x550 [rfcomm]
[ 295.002046] [<c013c18c>] kthread+0x5c/0xa0
[ 295.002046] [<c013c130>] ? kthread+0x0/0xa0
[ 295.002046] [<c0105c07>] kernel_thread_helper+0x7/0x10
[ 295.002046] =======================
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net')
-rw-r--r-- | net/bluetooth/rfcomm/core.c | 2 | ||||
-rw-r--r-- | net/bluetooth/rfcomm/tty.c | 5 |
2 files changed, 1 insertions, 6 deletions
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c index 0c2c93735e93..eb62558e9b09 100644 --- a/net/bluetooth/rfcomm/core.c +++ b/net/bluetooth/rfcomm/core.c @@ -423,8 +423,8 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err) rfcomm_dlc_lock(d); d->state = BT_CLOSED; - d->state_change(d, err); rfcomm_dlc_unlock(d); + d->state_change(d, err); skb_queue_purge(&d->tx_queue); rfcomm_dlc_unlink(d); diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c index e4c779bb8d76..c3f749abb2d0 100644 --- a/net/bluetooth/rfcomm/tty.c +++ b/net/bluetooth/rfcomm/tty.c @@ -570,12 +570,7 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err) return; rfcomm_dev_del(dev); - /* We have to drop DLC lock here, otherwise - rfcomm_dev_put() will dead lock if it's - the last reference. */ - rfcomm_dlc_unlock(dlc); rfcomm_dev_put(dev); - rfcomm_dlc_lock(dlc); } } else tty_hangup(dev->tty); |