From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH] cxgb4: disable BH when hold the adap_rcu_lock lock Date: Thu, 19 Jun 2014 06:54:49 -0700 Message-ID: <1403186089.1225.12.camel@edumazet-glaptop2.roam.corp.google.com> References: <53A0DDF5.80103@candelatech.com> <1403168810-9001-1-git-send-email-roy.qing.li@gmail.com> <1403185201.1225.10.camel@edumazet-glaptop2.roam.corp.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, greearb@candelatech.com, leedom@chelsio.com, hariprasad@chelsio.com To: roy.qing.li@gmail.com Return-path: Received: from mail-wi0-f176.google.com ([209.85.212.176]:62900 "EHLO mail-wi0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754545AbaFSNyx (ORCPT ); Thu, 19 Jun 2014 09:54:53 -0400 Received: by mail-wi0-f176.google.com with SMTP id n3so9400529wiv.9 for ; Thu, 19 Jun 2014 06:54:52 -0700 (PDT) In-Reply-To: <1403185201.1225.10.camel@edumazet-glaptop2.roam.corp.google.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 2014-06-19 at 06:40 -0700, Eric Dumazet wrote: > On Thu, 2014-06-19 at 17:06 +0800, roy.qing.li@gmail.com wrote: > > From: Li RongQing > > > > This lock is used in BH enabled condition and softirq context, so need to > > disable BH to avoid the dead lock: > > ================================= > > [ INFO: inconsistent lock state ] > > 3.14.7+ #24 Tainted: G C O > > --------------------------------- > > inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. > > radvd/3794 [HC0[0]:SC1[1]:HE1:SE0] takes: > > (adap_rcu_lock){+.?...}, at: [] clip_add+0x2c/0x116 [cxgb4] > > {SOFTIRQ-ON-W} state was registered at: > > [] __lock_acquire+0x34a/0xe48 > > [] lock_acquire+0x82/0x9d > > [] _raw_spin_lock+0x34/0x43 > > [] clip_add+0x2c/0x116 [cxgb4] > > [] cxgb4_inet6addr_handler+0x117/0x12c [cxgb4] > > [] notifier_call_chain+0x32/0x5c > > [] __atomic_notifier_call_chain+0x44/0x6e > > [] atomic_notifier_call_chain+0xf/0x11 > > [] inet6addr_notifier_call_chain+0x16/0x18 > > [] ipv6_add_addr+0x404/0x46e [ipv6] > > [] addrconf_add_linklocal+0x5f/0x95 [ipv6] > > [] addrconf_notify+0x632/0x841 [ipv6] > > [] notifier_call_chain+0x32/0x5c > > [] __raw_notifier_call_chain+0x9/0xb > > [] raw_notifier_call_chain+0xf/0x11 > > [] call_netdevice_notifiers_info+0x4e/0x56 > > [] call_netdevice_notifiers+0x11/0x13 > > [] netdev_state_change+0x1f/0x38 > > [] linkwatch_do_dev+0x3b/0x49 > > [] __linkwatch_run_queue+0x10b/0x144 > > [] linkwatch_event+0x20/0x27 > > [] process_one_work+0x1cb/0x2ee > > [] worker_thread+0x12e/0x1fc > > [] kthread+0xc4/0xcc > > [] ret_from_fork+0x7c/0xb0 > > irq event stamp: 3388 > > hardirqs last enabled at (3388): [] __local_bh_enable_ip+0xaa/0xd9 > > hardirqs last disabled at (3387): [] __local_bh_enable_ip+0x52/0xd9 > > softirqs last enabled at (3288): [] rcu_read_unlock_bh+0x0/0x2f [ipv6] > > softirqs last disabled at (3289): [] do_softirq_own_stack+0x1c/0x30 > > > > other info that might help us debug this: > > Possible unsafe locking scenario: > > > > CPU0 > > ---- > > lock(adap_rcu_lock); > > > > lock(adap_rcu_lock); > > > > *** DEADLOCK *** > > Sorry, I do not understand the problem. > > This lock should be taken from process context only. > > You did not provide full lockdep report, did you ? Right, appropriate fix is really : diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c index 2f8d6b910383..93d5287544ab 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c @@ -4057,22 +4057,19 @@ int cxgb4_unregister_uld(enum cxgb4_uld type) EXPORT_SYMBOL(cxgb4_unregister_uld); /* Check if netdev on which event is occured belongs to us or not. Return - * suceess (1) if it belongs otherwise failure (0). + * success (true) if it belongs otherwise failure (false). + * Called with rcu_read_lock() held. */ -static int cxgb4_netdev(struct net_device *netdev) +static bool cxgb4_netdev(const struct net_device *netdev) { struct adapter *adap; int i; - spin_lock(&adap_rcu_lock); list_for_each_entry_rcu(adap, &adap_rcu_list, rcu_node) for (i = 0; i < MAX_NPORTS; i++) - if (adap->port[i] == netdev) { - spin_unlock(&adap_rcu_lock); - return 1; - } - spin_unlock(&adap_rcu_lock); - return 0; + if (adap->port[i] == netdev) + return true; + return false; } static int clip_add(struct net_device *event_dev, struct inet6_ifaddr *ifa,