From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lennert Buytenhek Subject: Re: [PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown Date: Fri, 4 Jan 2013 21:25:09 +0100 Message-ID: <20130104202509.GX27530@wantstofly.org> References: <1357308422-19639-1-git-send-email-lkundrak@v3.sk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Lubomir Rintel To: Lubomir Rintel Return-path: Received: from fw.wantstofly.org ([80.101.37.227]:63851 "EHLO mail.wantstofly.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755120Ab3ADUZN (ORCPT ); Fri, 4 Jan 2013 15:25:13 -0500 Content-Disposition: inline In-Reply-To: <1357308422-19639-1-git-send-email-lkundrak@v3.sk> Sender: netdev-owner@vger.kernel.org List-ID: 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: G W > --------------------------------- > 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, I don't know.