From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hannes Frederic Sowa Subject: Re: [PATCH 2/3] ipvlan: grab rcu_read_lock on xmit path Date: Thu, 21 May 2015 13:47:53 +0200 Message-ID: <1432208873.2495287.274666097.526EE2F1@webmail.messagingengine.com> References: <20150514134657.14062.87579.stgit@buzz> <20150514135619.14062.97774.stgit@buzz> <555DAAB9.5090700@yandex-team.ru> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: "linux-netdev" , "David S. Miller" , Jiri Benc To: Konstantin Khlebnikov , Mahesh Bandewar Return-path: Received: from out5-smtp.messagingengine.com ([66.111.4.29]:34323 "EHLO out5-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753081AbbEULry (ORCPT ); Thu, 21 May 2015 07:47:54 -0400 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id EEA8920BCE for ; Thu, 21 May 2015 07:47:53 -0400 (EDT) In-Reply-To: <555DAAB9.5090700@yandex-team.ru> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, May 21, 2015, at 11:51, Konstantin Khlebnikov wrote: > On 20.05.2015 02:33, Mahesh Bandewar wrote: > > On Thu, May 14, 2015 at 6:56 AM, Konstantin Khlebnikov > > wrote: > >> > >> ipvlan_start_xmit() is called with rcu_read_lock_bh() while its internal > >> structures requre normal rcu_read_lock(). > >> > >> Signed-off-by: Konstantin Khlebnikov > >> > >> --- > >> > >> [ 802.945151] =============================== > >> [ 802.945160] [ INFO: suspicious RCU usage. ] > >> [ 802.945164] 4.1.0-rc3+ #71 Not tainted > >> [ 802.945165] ------------------------------- > >> [ 802.945167] drivers/net/ipvlan/ipvlan.h:103 suspicious rcu_dereference_check() usage! > >> [ 802.945168] > >> [ 802.945168] other info that might help us debug this: > >> [ 802.945168] > >> [ 802.945170] > >> [ 802.945170] rcu_scheduler_active = 1, debug_locks = 1 > >> [ 802.945173] 3 locks held by ping6/3813: > >> [ 802.945174] #0: (sk_lock-AF_INET6){+.+.+.}, at: [] rawv6_sendmsg+0x512/0xbb0 > >> [ 802.945197] #1: (rcu_read_lock_bh){......}, at: [] ip6_finish_output2+0x57/0x790 > >> [ 802.945205] #2: (rcu_read_lock_bh){......}, at: [] __dev_queue_xmit+0x4b/0x930 > >> [ 802.945218] > >> [ 802.945218] stack backtrace: > >> [ 802.945221] CPU: 2 PID: 3813 Comm: ping6 Not tainted 4.1.0-rc3+ #71 > >> [ 802.945222] Hardware name: OpenStack Foundation OpenStack Nova, BIOS Bochs 01/01/2011 > >> [ 802.945224] 0000000000000001 ffff8800db7b7888 ffffffff819de6f8 0000000000000007 > >> [ 802.945226] ffff8800db738000 ffff8800db7b78b8 ffffffff810a7f92 ffff880214fc8c00 > >> [ 802.945229] ffff88021595b000 ffff88021595a000 ffff880214adf800 ffff8800db7b7968 > >> [ 802.945232] Call Trace: > >> [ 802.945248] [] dump_stack+0x4c/0x65 > >> [ 802.945253] [] lockdep_rcu_suspicious+0xe2/0x130 > >> [ 802.945265] [] ipvlan_queue_xmit+0x17c/0x5a0 > >> [ 802.945268] [] ? __lock_is_held+0x54/0x70 > >> [ 802.945271] [] ipvlan_start_xmit+0x1c/0x50 > >> [ 802.945272] [] dev_hard_start_xmit+0x2f7/0x820 > >> [ 802.945274] [] ? netif_skb_features+0xf6/0x1d0 > >> [ 802.945276] [] ? validate_xmit_skb.isra.99.part.100+0x24/0x2c0 > >> [ 802.945278] [] __dev_queue_xmit+0x6c4/0x930 > >> [ 802.945280] [] ? __dev_queue_xmit+0x4b/0x930 > >> [ 802.945281] [] ? mark_held_locks+0x6a/0x90 > >> [ 802.945283] [] dev_queue_xmit_sk+0xe/0x10 > >> [ 802.945285] [] ip6_finish_output2+0x304/0x790 > >> [ 802.945287] [] ? ip6_finish_output+0x9e/0x1e0 > >> [ 802.945288] [] ip6_finish_output+0x9e/0x1e0 > >> [ 802.945290] [] ip6_output+0xbb/0x180 > >> [ 802.945302] [] ? ip6_find_1stfragopt+0x9a/0xa0 > >> [ 802.945304] [] ? ip6_fragment+0xe80/0xe80 > >> [ 802.945306] [] ip6_local_out_sk+0x2c/0x70 > >> [ 802.945308] [] ip6_local_out+0x10/0x20 > >> [ 802.945309] [] ip6_send_skb+0x31/0xd0 > >> [ 802.945311] [] ip6_push_pending_frames+0x34/0x40 > >> [ 802.945313] [] rawv6_sendmsg+0x908/0xbb0 > >> [ 802.945328] [] ? __lock_is_held+0x54/0x70 > >> [ 802.945340] [] inet_sendmsg+0x10e/0x1f0 > >> [ 802.945343] [] ? inet_recvmsg+0x200/0x200 > >> [ 802.945351] [] sock_sendmsg+0x45/0x50 > >> [ 802.945354] [] SYSC_sendto+0xd9/0x110 > >> [ 802.945357] [] SyS_sendto+0x9/0x10 > >> [ 802.945362] [] system_call_fastpath+0x12/0x76 > >> --- > >> drivers/net/ipvlan/ipvlan_main.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c > >> index 77b92a0fe557..0cafd3e6f02d 100644 > >> --- a/drivers/net/ipvlan/ipvlan_main.c > >> +++ b/drivers/net/ipvlan/ipvlan_main.c > >> @@ -173,6 +173,7 @@ static int ipvlan_stop(struct net_device *dev) > >> return 0; > >> } > >> > >> +/* called with rcu_read_lock_bh. */ > >> static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb, > >> struct net_device *dev) > >> { > >> @@ -180,6 +181,7 @@ static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb, > >> int skblen = skb->len; > >> int ret; > >> > >> + rcu_read_lock(); > > > > I don't believe this is correct. The correct way would be the way it > > is fixed in the following patch - > > https://patchwork.ozlabs.org/patch/471481/ > > I'm not sure, that might just plug the warning without fixing problem. > The rest code uses call_rcu()/synchronize_rcu(). Probably relation > between quiescent states of rcu and rcu_bh isn't that easy. > In theory both, rcu_read_lock and rcu_read_lock_bh must be held, otherwise call_rcu would be allowed to execute callback if no rcu_read_lock but only rcu_read_lock_bh is taken. *AFAIK* in normal kernels, grace period for call_rcu is just longer, call_rcu_bh does check transition from and to softirqs. Bye, Hannes