Re: [PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown
Am 12.03.2013 10:04, schrieb Alexander Holler: I don't think so. Internally the driver calls mv643xx_eth_stop() therefore lockdep issues almost the same warning as when the interface is shut down (see below). And reading the code, I haven't seen how a Btw. I consider the shutdown of the interface for just changing the MTU more a problem than that lockdep-warning. E.g. I had serious problems with dhcpcd which sets the MTU to 1500 when an interface appears. What then happened was the following: (1) interface comes up (2) dhcpcd sets MTU to 1500 (3) a different MTU comes through DHCP (4) driver shuts down if, sets MTU, turns on IF back to (1) (endless loop) I'm not sure if dhcpcd still does that (it's some time since I discovered and reported that), but I consider it a crude way to shut down the IF to set the MTU. Regards, Alexander -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown
Am 12.03.2013 03:49, schrieb Lennert Buytenhek:
On Fri, Mar 01, 2013 at 06:30:13PM +0100, Alexander Holler wrote:
From: Lubomir Rintel
=
[ INFO: inconsistent lock state ]
3.7.0-6.luboskovo.fc19.armv5tel.kirkwood #1 Tainted: GW
-
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
NetworkManager/337 [HC0[0]:SC0[0]:HE1:SE1] takes:
(_xmit_ETHER#2){+.?...}, at: [] txq_reclaim+0x54/0x264 [mv643xx_eth]
I get the same annoying warning when the MTU gets changed (through dhcp).
That is actually an issue.
I don't think so. Internally the driver calls mv643xx_eth_stop()
therefore lockdep issues almost the same warning as when the interface
is shut down (see below). And reading the code, I haven't seen how a
deadlock could happen. I just wanted to mention that it happens too,
when the MTU gets changed, in case someone else will see that warning
after enabling lockdep and wonders if it actually is a problem.
It fixes a bug (the MTU change thing) and a non-bug (the lockdep
warning) at the expense of slowing down the much more common path,
and I don't like it for that reason.
I don't see how something important is slowed down. The patch only
affects code which is either used when the link goes down or when the
interface will be polled. I'm not sure where poll is used, but I believe
it is only used by netconsole, so nothing really important is slowed
down by the patch. At least if I understood everything correct. ;)
Can you make a __txq_reclaim() which is basically txq_reclaim()
without grabbing the tx queue lock, and then move the lock grabbing
to the caller?
E.g. make __txq_reclaim() have two callers, txq_reclaim() and
txq_reclaim_bh(), and then use the appropriate wrapper depending on
the context. (tx queue lock but no BH disable when called from
mv643xx_eth_poll(), tx queue lock plus BH disable for MTU change,
and no locking at all when called from ->ndo_stop(). Something
like that.)
Hmm, I'm have to take a deep look at the driver to understand what you
mean (it's some days ago I've last did), but I will try. ;)
For completeness, below is the full output (when the MTU will be changed
and lockdep is enabled), I've just removed the dates to shorten it.
Regards,
Alexander
=
[ INFO: inconsistent lock state ]
3.8.1-dockstar-00023-g524f9cf #280 Not tainted
-
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
ip/1169 [HC0[0]:SC0[0]:HE1:SE1] takes:
(_xmit_ETHER#2){+.?...}, at: [] txq_reclaim+0x44/0x1c8
{IN-SOFTIRQ-W} state was registered at:
[] __lock_acquire+0x5ac/0x17cc
[] lock_acquire+0x64/0x78
[] _raw_spin_lock+0x40/0x50
[] mv643xx_eth_poll+0x1e8/0x634
[] net_rx_action+0x9c/0x218
[] __do_softirq+0xb4/0x18c
[] irq_exit+0x54/0xb8
[] handle_IRQ+0x64/0x84
[] __irq_svc+0x38/0xa0
[] console_unlock+0x194/0x398
[] register_console+0x294/0x36c
[] init_netconsole+0x138/0x1d4
[] do_one_initcall+0x90/0x16c
[] kernel_init_freeable+0xe8/0x1b0
[] kernel_init+0x8/0xe4
[] ret_from_fork+0x14/0x2c
irq event stamp: 3539
hardirqs last enabled at (3539): []
__slab_free.isra.53+0x1ac/0x2f8
hardirqs last disabled at (3538): []
__slab_free.isra.53+0xcc/0x2f8
softirqs last enabled at (3246): []
mib_counters_update+0x56c/0x58c
softirqs last disabled at (3244): [] _raw_spin_lock_bh+0x14/0x5c
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
lock(_xmit_ETHER#2);
lock(_xmit_ETHER#2);
*** DEADLOCK ***
1 lock held by ip/1169:
#0: (rtnl_mutex){+.+.+.}, at: [] rtnetlink_rcv+0xc/0x24
stack backtrace:
[] (unwind_backtrace+0x0/0xe0) from []
(print_usage_bug.part.25+0x20c/0x26c)
[] (print_usage_bug.part.25+0x20c/0x26c) from []
(mark_lock+0x404/0x60c)
[] (mark_lock+0x404/0x60c) from []
(__lock_acquire+0x630/0x17cc)
[] (__lock_acquire+0x630/0x17cc) from []
(lock_acquire+0x64/0x78)
[] (lock_acquire+0x64/0x78) from []
(_raw_spin_lock+0x40/0x50)
[] (_raw_spin_lock+0x40/0x50) from []
(txq_reclaim+0x44/0x1c8)
[] (txq_reclaim+0x44/0x1c8) from []
(txq_deinit+0x28/0xc0)
[] (txq_deinit+0x28/0xc0) from []
(mv643xx_eth_stop+0x114/0x130)
[] (mv643xx_eth_stop+0x114/0x130) from []
(mv643xx_eth_change_mtu+0x4c/0x80)
[] (mv643xx_eth_change_mtu+0x4c/0x80) from []
(dev_set_mtu+0x3c/0x70)
[] (dev_set_mtu+0x3c/0x70) from []
(do_setlink+0x1ac/0x794)
[] (do_setlink+0x1ac/0x794) from []
(rtnl_newlink+0x24c/0x438)
[] (rtnl_newlink+0x24c/0x438) from []
(rtnetlink_rcv_msg+0x264/0x284)
[] (rtnetlink_rcv_msg+0x264/0x284) from []
(netlink_rcv_skb+0x50/0xac)
[] (netlink_rcv_skb+0x50/0xac) from []
(rtnetlink_rcv+0x18/0x24)
[] (rtnetlink_rcv+0x18/0x24) from []
(netlink_unicast+0x14c/0x1fc)
[] (netlink_unicast+0x14c/0x1fc) from []
(netlink_sendmsg+0x2e0/0x368)
[] (netlink_sendmsg+0x2e0/0x368) from []
(sock_sendmsg+0x80/0x9c)
[] (sock_sendmsg+0x80/0x9c) from []
(__sys_sendmsg+0x1c4/0x250)
[
Re: [PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown
On Fri, Mar 01, 2013 at 06:30:13PM +0100, Alexander Holler wrote:
> >>From: Lubomir Rintel
> >>
> >>=
> >>[ INFO: inconsistent lock state ]
> >>3.7.0-6.luboskovo.fc19.armv5tel.kirkwood #1 Tainted: GW
> >>-
> >>inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> >>NetworkManager/337 [HC0[0]:SC0[0]:HE1:SE1] takes:
> >> (_xmit_ETHER#2){+.?...}, at: [] txq_reclaim+0x54/0x264
> >> [mv643xx_eth]
>
> I get the same annoying warning when the MTU gets changed (through dhcp).
That is actually an issue.
> >Maybe I'm not reading it right, but I doubt that this is an actual
> >deadlock or that the patch is needed.
> >
> >txq_reclaim() indeed doesn't disable BHs, but that's because it's
> >always called in BH context. Almost always -- the only exception is
> >txq_deinit(), called from ->ndo_stop(), but by that time we've
> >already napi_disable()'d and netif_carrier_off()'d and free_irq()'d.
>
> Agreed. I've just read me through that too and don't think a
> deadlock is possible.
>
> >How to explain that to lockdep, though, I don't know.
>
> The patch helps with that. ;)
It fixes a bug (the MTU change thing) and a non-bug (the lockdep
warning) at the expense of slowing down the much more common path,
and I don't like it for that reason.
Can you make a __txq_reclaim() which is basically txq_reclaim()
without grabbing the tx queue lock, and then move the lock grabbing
to the caller?
E.g. make __txq_reclaim() have two callers, txq_reclaim() and
txq_reclaim_bh(), and then use the appropriate wrapper depending on
the context. (tx queue lock but no BH disable when called from
mv643xx_eth_poll(), tx queue lock plus BH disable for MTU change,
and no locking at all when called from ->ndo_stop(). Something
like that.)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown
Am 04.01.2013 21:25, schrieb Lennert Buytenhek:
On Fri, Jan 04, 2013 at 03:07:02PM +0100, Lubomir Rintel wrote:
From: Lubomir Rintel
=
[ INFO: inconsistent lock state ]
3.7.0-6.luboskovo.fc19.armv5tel.kirkwood #1 Tainted: GW
-
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
NetworkManager/337 [HC0[0]:SC0[0]:HE1:SE1] takes:
(_xmit_ETHER#2){+.?...}, at: [] txq_reclaim+0x54/0x264 [mv643xx_eth]
I get the same annoying warning when the MTU gets changed (through dhcp).
Maybe I'm not reading it right, but I doubt that this is an actual
deadlock or that the patch is needed.
txq_reclaim() indeed doesn't disable BHs, but that's because it's
always called in BH context. Almost always -- the only exception is
txq_deinit(), called from ->ndo_stop(), but by that time we've
already napi_disable()'d and netif_carrier_off()'d and free_irq()'d.
Agreed. I've just read me through that too and don't think a deadlock is
possible.
How to explain that to lockdep, though, I don't know.
The patch helps with that. ;)
Regards,
Alexander
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown
From: Lubomir Rintel Date: Fri, 4 Jan 2013 15:17:43 +0100 > @@ -943,7 +943,7 @@ static int txq_reclaim(struct tx_queue *txq, int budget, > int force) > struct netdev_queue *nq = netdev_get_tx_queue(mp->dev, txq->index); > int reclaimed; > > - __netif_tx_lock(nq, smp_processor_id()); > + __netif_tx_lock_bh(nq); I still don't understand why this change is necessary. The TX reclaim function is invoked in software interrupt context in all of the places where this lockdep warning might matter. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown
On Fri, Jan 04, 2013 at 03:07:02PM +0100, Lubomir Rintel wrote:
> From: Lubomir Rintel
>
> =
> [ INFO: inconsistent lock state ]
> 3.7.0-6.luboskovo.fc19.armv5tel.kirkwood #1 Tainted: GW
> -
> inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> NetworkManager/337 [HC0[0]:SC0[0]:HE1:SE1] takes:
> (_xmit_ETHER#2){+.?...}, at: [] txq_reclaim+0x54/0x264
> [mv643xx_eth]
> {IN-SOFTIRQ-W} state was registered at:
> [] __lock_acquire+0x5b4/0x17d0
> [] lock_acquire+0x160/0x1e0
> [] _raw_spin_lock+0x50/0x88
> [] sch_direct_xmit+0x4c/0x2d4
> [] dev_queue_xmit+0x4b8/0x8d8
> [] ip6_finish_output2+0x350/0x42c
> [] mld_sendpack+0x2d0/0x514
> [] mld_ifc_timer_expire+0x228/0x278
> [] call_timer_fn+0x140/0x33c
> [] run_timer_softirq+0x278/0x32c
> [] __do_softirq+0x16c/0x398
> [] irq_exit+0x5c/0xc0
> [] handle_IRQ+0x6c/0x8c
> [] __irq_svc+0x38/0x80
> [] lock_is_held+0x4/0x54
> [] __might_sleep+0x44/0x228
> [] down_read+0x28/0x88
> [] __copy_to_user_memcpy+0xa8/0x140
> [] seq_read+0x3ac/0x474
> [] vfs_read+0xac/0x184
> [] sys_read+0x40/0x6c
> [] ret_fast_syscall+0x0/0x38
> irq event stamp: 115119
> hardirqs last enabled at (115119): []
> _raw_spin_unlock_irqrestore+0x44/0x64
> hardirqs last disabled at (115118): []
> _raw_spin_lock_irqsave+0x28/0xa0
> softirqs last enabled at (114880): [] __do_softirq+0x2b8/0x398
> softirqs last disabled at (114873): [] irq_exit+0x5c/0xc0
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
>CPU0
>
> lock(_xmit_ETHER#2);
>
> lock(_xmit_ETHER#2);
>
> *** DEADLOCK ***
>
> 1 lock held by NetworkManager/337:
> #0: (rtnl_mutex){+.+.+.}, at: [] rtnetlink_rcv+0x14/0x2c
>
> stack backtrace:
> [] (unwind_backtrace+0x0/0x124) from []
> (print_usage_bug.part.29+0x20c/0x26c)
> [] (print_usage_bug.part.29+0x20c/0x26c) from []
> (mark_lock+0x404/0x60c)
> [] (mark_lock+0x404/0x60c) from []
> (__lock_acquire+0x638/0x17d0)
> [] (__lock_acquire+0x638/0x17d0) from []
> (lock_acquire+0x160/0x1e0)
> [] (lock_acquire+0x160/0x1e0) from []
> (_raw_spin_lock+0x50/0x88)
> [] (_raw_spin_lock+0x50/0x88) from []
> (txq_reclaim+0x54/0x264 [mv643xx_eth])
> [] (txq_reclaim+0x54/0x264 [mv643xx_eth]) from []
> (txq_deinit+0x30/0xec [mv643xx_eth])
> [] (txq_deinit+0x30/0xec [mv643xx_eth]) from []
> (mv643xx_eth_stop+0x124/0x140 [mv643xx_eth])
> [] (mv643xx_eth_stop+0x124/0x140 [mv643xx_eth]) from []
> (__dev_close_many+0xb0/0xec)
> [] (__dev_close_many+0xb0/0xec) from []
> (__dev_close+0x30/0x44)
> [] (__dev_close+0x30/0x44) from []
> (__dev_change_flags+0x94/0x120)
> [] (__dev_change_flags+0x94/0x120) from []
> (dev_change_flags+0x18/0x4c)
> [] (dev_change_flags+0x18/0x4c) from []
> (do_setlink+0x2cc/0x7ac)
> [] (do_setlink+0x2cc/0x7ac) from []
> (rtnl_newlink+0x26c/0x4a8)
> [] (rtnl_newlink+0x26c/0x4a8) from []
> (rtnetlink_rcv_msg+0x280/0x29c)
> [] (rtnetlink_rcv_msg+0x280/0x29c) from []
> (netlink_rcv_skb+0x58/0xb4)
> [] (netlink_rcv_skb+0x58/0xb4) from []
> (rtnetlink_rcv+0x20/0x2c)
> [] (rtnetlink_rcv+0x20/0x2c) from []
> (netlink_unicast+0x158/0x208)
> [] (netlink_unicast+0x158/0x208) from []
> (netlink_sendmsg+0x310/0x3c0)
> [] (netlink_sendmsg+0x310/0x3c0) from []
> (sock_sendmsg+0xa8/0xd0)
> [] (sock_sendmsg+0xa8/0xd0) from []
> (__sys_sendmsg+0x1d8/0x280)
> [] (__sys_sendmsg+0x1d8/0x280) from []
> (sys_sendmsg+0x44/0x68)
> [] (sys_sendmsg+0x44/0x68) from []
> (ret_fast_syscall+0x0/0x38)
> ---
> drivers/net/ethernet/marvell/mv643xx_eth.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c
> b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index 84c1326..67a3e78 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -943,7 +943,7 @@ static int txq_reclaim(struct tx_queue *txq, int budget,
> int force)
> struct netdev_queue *nq = netdev_get_tx_queue(mp->dev, txq->index);
> int reclaimed;
>
> - __netif_tx_lock(nq, smp_processor_id());
> + __netif_tx_lock_bh(nq);
>
> reclaimed = 0;
> while (reclaimed < budget && txq->tx_desc_count > 0) {
> @@ -989,7 +989,7 @@ static int txq_reclaim(struct tx_queue *txq, int budget,
> int force)
> dev_kfree_skb(skb);
> }
>
> - __netif_tx_unlock(nq);
> + __netif_tx_unlock_bh(nq);
>
> if (reclaimed < budget)
> mp->work_tx &= ~(1 << txq->index);
> --
Maybe I'm not reading it right, but I doubt that this is an actual
deadlock or that the patch is needed.
txq_reclaim() indeed doesn't disable BHs, but that's because it's
always called in BH context. Almost always -- the only exception is
txq_deinit(), called from ->ndo_stop(), but by that time we've
already napi_disable()'d and netif_carrier_off()'d and free_irq()'d.
How to explain that to lockdep, though,
[PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown
=
[ INFO: inconsistent lock state ]
3.7.0-6.luboskovo.fc19.armv5tel.kirkwood #1 Tainted: GW
-
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
NetworkManager/337 [HC0[0]:SC0[0]:HE1:SE1] takes:
(_xmit_ETHER#2){+.?...}, at: [] txq_reclaim+0x54/0x264 [mv643xx_eth]
{IN-SOFTIRQ-W} state was registered at:
[] __lock_acquire+0x5b4/0x17d0
[] lock_acquire+0x160/0x1e0
[] _raw_spin_lock+0x50/0x88
[] sch_direct_xmit+0x4c/0x2d4
[] dev_queue_xmit+0x4b8/0x8d8
[] ip6_finish_output2+0x350/0x42c
[] mld_sendpack+0x2d0/0x514
[] mld_ifc_timer_expire+0x228/0x278
[] call_timer_fn+0x140/0x33c
[] run_timer_softirq+0x278/0x32c
[] __do_softirq+0x16c/0x398
[] irq_exit+0x5c/0xc0
[] handle_IRQ+0x6c/0x8c
[] __irq_svc+0x38/0x80
[] lock_is_held+0x4/0x54
[] __might_sleep+0x44/0x228
[] down_read+0x28/0x88
[] __copy_to_user_memcpy+0xa8/0x140
[] seq_read+0x3ac/0x474
[] vfs_read+0xac/0x184
[] sys_read+0x40/0x6c
[] ret_fast_syscall+0x0/0x38
irq event stamp: 115119
hardirqs last enabled at (115119): []
_raw_spin_unlock_irqrestore+0x44/0x64
hardirqs last disabled at (115118): []
_raw_spin_lock_irqsave+0x28/0xa0
softirqs last enabled at (114880): [] __do_softirq+0x2b8/0x398
softirqs last disabled at (114873): [] irq_exit+0x5c/0xc0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
lock(_xmit_ETHER#2);
lock(_xmit_ETHER#2);
*** DEADLOCK ***
1 lock held by NetworkManager/337:
#0: (rtnl_mutex){+.+.+.}, at: [] rtnetlink_rcv+0x14/0x2c
stack backtrace:
[] (unwind_backtrace+0x0/0x124) from []
(print_usage_bug.part.29+0x20c/0x26c)
[] (print_usage_bug.part.29+0x20c/0x26c) from []
(mark_lock+0x404/0x60c)
[] (mark_lock+0x404/0x60c) from []
(__lock_acquire+0x638/0x17d0)
[] (__lock_acquire+0x638/0x17d0) from []
(lock_acquire+0x160/0x1e0)
[] (lock_acquire+0x160/0x1e0) from []
(_raw_spin_lock+0x50/0x88)
[] (_raw_spin_lock+0x50/0x88) from []
(txq_reclaim+0x54/0x264 [mv643xx_eth])
[] (txq_reclaim+0x54/0x264 [mv643xx_eth]) from []
(txq_deinit+0x30/0xec [mv643xx_eth])
[] (txq_deinit+0x30/0xec [mv643xx_eth]) from []
(mv643xx_eth_stop+0x124/0x140 [mv643xx_eth])
[] (mv643xx_eth_stop+0x124/0x140 [mv643xx_eth]) from []
(__dev_close_many+0xb0/0xec)
[] (__dev_close_many+0xb0/0xec) from []
(__dev_close+0x30/0x44)
[] (__dev_close+0x30/0x44) from []
(__dev_change_flags+0x94/0x120)
[] (__dev_change_flags+0x94/0x120) from []
(dev_change_flags+0x18/0x4c)
[] (dev_change_flags+0x18/0x4c) from []
(do_setlink+0x2cc/0x7ac)
[] (do_setlink+0x2cc/0x7ac) from []
(rtnl_newlink+0x26c/0x4a8)
[] (rtnl_newlink+0x26c/0x4a8) from []
(rtnetlink_rcv_msg+0x280/0x29c)
[] (rtnetlink_rcv_msg+0x280/0x29c) from []
(netlink_rcv_skb+0x58/0xb4)
[] (netlink_rcv_skb+0x58/0xb4) from []
(rtnetlink_rcv+0x20/0x2c)
[] (rtnetlink_rcv+0x20/0x2c) from []
(netlink_unicast+0x158/0x208)
[] (netlink_unicast+0x158/0x208) from []
(netlink_sendmsg+0x310/0x3c0)
[] (netlink_sendmsg+0x310/0x3c0) from []
(sock_sendmsg+0xa8/0xd0)
[] (sock_sendmsg+0xa8/0xd0) from []
(__sys_sendmsg+0x1d8/0x280)
[] (__sys_sendmsg+0x1d8/0x280) from []
(sys_sendmsg+0x44/0x68)
[] (sys_sendmsg+0x44/0x68) from []
(ret_fast_syscall+0x0/0x38)
Signed-off-by: Lubomir Rintel
---
I've sent an incorrect version; resending with a proper signoff and headers
this time; sorry about that.
drivers/net/ethernet/marvell/mv643xx_eth.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c
b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 84c1326..67a3e78 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -943,7 +943,7 @@ static int txq_reclaim(struct tx_queue *txq, int budget,
int force)
struct netdev_queue *nq = netdev_get_tx_queue(mp->dev, txq->index);
int reclaimed;
- __netif_tx_lock(nq, smp_processor_id());
+ __netif_tx_lock_bh(nq);
reclaimed = 0;
while (reclaimed < budget && txq->tx_desc_count > 0) {
@@ -989,7 +989,7 @@ static int txq_reclaim(struct tx_queue *txq, int budget,
int force)
dev_kfree_skb(skb);
}
- __netif_tx_unlock(nq);
+ __netif_tx_unlock_bh(nq);
if (reclaimed < budget)
mp->work_tx &= ~(1 << txq->index);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown
From: Lubomir Rintel
=
[ INFO: inconsistent lock state ]
3.7.0-6.luboskovo.fc19.armv5tel.kirkwood #1 Tainted: GW
-
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
NetworkManager/337 [HC0[0]:SC0[0]:HE1:SE1] takes:
(_xmit_ETHER#2){+.?...}, at: [] txq_reclaim+0x54/0x264 [mv643xx_eth]
{IN-SOFTIRQ-W} state was registered at:
[] __lock_acquire+0x5b4/0x17d0
[] lock_acquire+0x160/0x1e0
[] _raw_spin_lock+0x50/0x88
[] sch_direct_xmit+0x4c/0x2d4
[] dev_queue_xmit+0x4b8/0x8d8
[] ip6_finish_output2+0x350/0x42c
[] mld_sendpack+0x2d0/0x514
[] mld_ifc_timer_expire+0x228/0x278
[] call_timer_fn+0x140/0x33c
[] run_timer_softirq+0x278/0x32c
[] __do_softirq+0x16c/0x398
[] irq_exit+0x5c/0xc0
[] handle_IRQ+0x6c/0x8c
[] __irq_svc+0x38/0x80
[] lock_is_held+0x4/0x54
[] __might_sleep+0x44/0x228
[] down_read+0x28/0x88
[] __copy_to_user_memcpy+0xa8/0x140
[] seq_read+0x3ac/0x474
[] vfs_read+0xac/0x184
[] sys_read+0x40/0x6c
[] ret_fast_syscall+0x0/0x38
irq event stamp: 115119
hardirqs last enabled at (115119): []
_raw_spin_unlock_irqrestore+0x44/0x64
hardirqs last disabled at (115118): []
_raw_spin_lock_irqsave+0x28/0xa0
softirqs last enabled at (114880): [] __do_softirq+0x2b8/0x398
softirqs last disabled at (114873): [] irq_exit+0x5c/0xc0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
lock(_xmit_ETHER#2);
lock(_xmit_ETHER#2);
*** DEADLOCK ***
1 lock held by NetworkManager/337:
#0: (rtnl_mutex){+.+.+.}, at: [] rtnetlink_rcv+0x14/0x2c
stack backtrace:
[] (unwind_backtrace+0x0/0x124) from []
(print_usage_bug.part.29+0x20c/0x26c)
[] (print_usage_bug.part.29+0x20c/0x26c) from []
(mark_lock+0x404/0x60c)
[] (mark_lock+0x404/0x60c) from []
(__lock_acquire+0x638/0x17d0)
[] (__lock_acquire+0x638/0x17d0) from []
(lock_acquire+0x160/0x1e0)
[] (lock_acquire+0x160/0x1e0) from []
(_raw_spin_lock+0x50/0x88)
[] (_raw_spin_lock+0x50/0x88) from []
(txq_reclaim+0x54/0x264 [mv643xx_eth])
[] (txq_reclaim+0x54/0x264 [mv643xx_eth]) from []
(txq_deinit+0x30/0xec [mv643xx_eth])
[] (txq_deinit+0x30/0xec [mv643xx_eth]) from []
(mv643xx_eth_stop+0x124/0x140 [mv643xx_eth])
[] (mv643xx_eth_stop+0x124/0x140 [mv643xx_eth]) from []
(__dev_close_many+0xb0/0xec)
[] (__dev_close_many+0xb0/0xec) from []
(__dev_close+0x30/0x44)
[] (__dev_close+0x30/0x44) from []
(__dev_change_flags+0x94/0x120)
[] (__dev_change_flags+0x94/0x120) from []
(dev_change_flags+0x18/0x4c)
[] (dev_change_flags+0x18/0x4c) from []
(do_setlink+0x2cc/0x7ac)
[] (do_setlink+0x2cc/0x7ac) from []
(rtnl_newlink+0x26c/0x4a8)
[] (rtnl_newlink+0x26c/0x4a8) from []
(rtnetlink_rcv_msg+0x280/0x29c)
[] (rtnetlink_rcv_msg+0x280/0x29c) from []
(netlink_rcv_skb+0x58/0xb4)
[] (netlink_rcv_skb+0x58/0xb4) from []
(rtnetlink_rcv+0x20/0x2c)
[] (rtnetlink_rcv+0x20/0x2c) from []
(netlink_unicast+0x158/0x208)
[] (netlink_unicast+0x158/0x208) from []
(netlink_sendmsg+0x310/0x3c0)
[] (netlink_sendmsg+0x310/0x3c0) from []
(sock_sendmsg+0xa8/0xd0)
[] (sock_sendmsg+0xa8/0xd0) from []
(__sys_sendmsg+0x1d8/0x280)
[] (__sys_sendmsg+0x1d8/0x280) from []
(sys_sendmsg+0x44/0x68)
[] (sys_sendmsg+0x44/0x68) from []
(ret_fast_syscall+0x0/0x38)
---
drivers/net/ethernet/marvell/mv643xx_eth.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c
b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 84c1326..67a3e78 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -943,7 +943,7 @@ static int txq_reclaim(struct tx_queue *txq, int budget,
int force)
struct netdev_queue *nq = netdev_get_tx_queue(mp->dev, txq->index);
int reclaimed;
- __netif_tx_lock(nq, smp_processor_id());
+ __netif_tx_lock_bh(nq);
reclaimed = 0;
while (reclaimed < budget && txq->tx_desc_count > 0) {
@@ -989,7 +989,7 @@ static int txq_reclaim(struct tx_queue *txq, int budget,
int force)
dev_kfree_skb(skb);
}
- __netif_tx_unlock(nq);
+ __netif_tx_unlock_bh(nq);
if (reclaimed < budget)
mp->work_tx &= ~(1 << txq->index);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
