Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932394Ab3CLJE0 (ORCPT ); Tue, 12 Mar 2013 05:04:26 -0400 Received: from h1446028.stratoserver.net ([85.214.92.142]:54394 "EHLO mail.ahsoftware.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932342Ab3CLJEU (ORCPT ); Tue, 12 Mar 2013 05:04:20 -0400 Message-ID: <513EEF83.9070806@ahsoftware.de> Date: Tue, 12 Mar 2013 10:04:03 +0100 From: Alexander Holler User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130219 Thunderbird/17.0.3 MIME-Version: 1.0 To: Lennert Buytenhek CC: Lubomir Rintel , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Lubomir Rintel Subject: Re: [PATCH] mv643xx_eth: Fix a possible deadlock upon ifdown References: <1357308422-19639-1-git-send-email-lkundrak@v3.sk> <20130104202509.GX27530@wantstofly.org> <5130E5A5.90303@ahsoftware.de> <20130312024932.GF850@wantstofly.org> In-Reply-To: <20130312024932.GF850@wantstofly.org> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6248 Lines: 154 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: 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] >> >> 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) [] (__sys_sendmsg+0x1c4/0x250) from [] (sys_sendmsg+0x3c/0x60) [] (sys_sendmsg+0x3c/0x60) from [] (ret_fast_syscall+0x0/0x38) mv643xx_eth_port mv643xx_eth_port.0 eth0: link up, 1000 Mb/s, full duplex, flow control disabled -- 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/