diff options
author | Xiaotian Feng <xtfeng@gmail.com> | 2009-10-21 19:07:04 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2009-10-22 21:27:53 -0400 |
commit | 2bd9af046fdc10703b266b0f3b25423f0b7d703e (patch) | |
tree | c20f3117f3cf7af8d309ff919e4afd5ba3c48184 | |
parent | 0dc6d9cbe7df4d2c3cdf17cd2f78733102b0fea6 (diff) |
isdn: fix possible circular locking dependency
There's a circular locking dependency:
---> isdn_net_get_locked_lp
--->lock &nd->queue_lock
--->lock &nd->queue->xmit_lock
.....................
---->unlock &nd->queue_lock
---> isdn_net_writebuf_skb (called with &nd->queue->xmit_lock locked)
---->isdn_net_inc_frame_cnt
---->isdn_net_device_busy
----> lock &nd->queue_lock
This will trigger lockdep warnings:
=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.32-rc4-testing #7
-------------------------------------------------------
ipppd/28379 is trying to acquire lock:
(&netdev->queue_lock){......}, at: [<e62ad0fd>] isdn_net_device_busy+0x2c/0x74 [isdn]
but task is already holding lock:
(&netdev->local->xmit_lock){+.....}, at: [<e62aefc2>] isdn_net_write_super+0x3f/0x6e [isdn]
which lock already depends on the new lock.
.......
We don't need to lock nd->queue->xmit_lock to protect single
isdn_net_lp_busy(). This can fix above lockdep warnings.
Reported-and-tested-by: Tilman Schmidt <tilman@imap.cc>
Signed-off-by: Xiaotian Feng <xtfeng@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | drivers/isdn/i4l/isdn_net.h | 6 |
1 files changed, 3 insertions, 3 deletions
diff --git a/drivers/isdn/i4l/isdn_net.h b/drivers/isdn/i4l/isdn_net.h index 74032d0881e..7511f08effa 100644 --- a/drivers/isdn/i4l/isdn_net.h +++ b/drivers/isdn/i4l/isdn_net.h | |||
@@ -83,19 +83,19 @@ static __inline__ isdn_net_local * isdn_net_get_locked_lp(isdn_net_dev *nd) | |||
83 | 83 | ||
84 | spin_lock_irqsave(&nd->queue_lock, flags); | 84 | spin_lock_irqsave(&nd->queue_lock, flags); |
85 | lp = nd->queue; /* get lp on top of queue */ | 85 | lp = nd->queue; /* get lp on top of queue */ |
86 | spin_lock(&nd->queue->xmit_lock); | ||
87 | while (isdn_net_lp_busy(nd->queue)) { | 86 | while (isdn_net_lp_busy(nd->queue)) { |
88 | spin_unlock(&nd->queue->xmit_lock); | ||
89 | nd->queue = nd->queue->next; | 87 | nd->queue = nd->queue->next; |
90 | if (nd->queue == lp) { /* not found -- should never happen */ | 88 | if (nd->queue == lp) { /* not found -- should never happen */ |
91 | lp = NULL; | 89 | lp = NULL; |
92 | goto errout; | 90 | goto errout; |
93 | } | 91 | } |
94 | spin_lock(&nd->queue->xmit_lock); | ||
95 | } | 92 | } |
96 | lp = nd->queue; | 93 | lp = nd->queue; |
97 | nd->queue = nd->queue->next; | 94 | nd->queue = nd->queue->next; |
95 | spin_unlock_irqrestore(&nd->queue_lock, flags); | ||
96 | spin_lock(&lp->xmit_lock); | ||
98 | local_bh_disable(); | 97 | local_bh_disable(); |
98 | return lp; | ||
99 | errout: | 99 | errout: |
100 | spin_unlock_irqrestore(&nd->queue_lock, flags); | 100 | spin_unlock_irqrestore(&nd->queue_lock, flags); |
101 | return lp; | 101 | return lp; |