Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755415Ab0DUX02 (ORCPT ); Wed, 21 Apr 2010 19:26:28 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:45137 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754646Ab0DUX00 convert rfc822-to-8bit (ORCPT ); Wed, 21 Apr 2010 19:26:26 -0400 To: paulmck@linux.vnet.ibm.com Cc: Eric Dumazet , Miles Lane , Eric Paris , Lai Jiangshan , Ingo Molnar , Peter Zijlstra , LKML , vgoyal@redhat.com, nauman@google.com, netdev@vger.kernel.org Subject: Re: [PATCH] RCU: don't turn off lockdep when find suspicious rcu_dereference_check() usage References: <20100419230136.GA16856@linux.vnet.ibm.com> <1271726729.2972.13.camel@dhcp231-113.rdu.redhat.com> <20100420030452.GB2905@linux.vnet.ibm.com> <4BCD646B.1080206@cn.fujitsu.com> <1271766716.2972.16.camel@dhcp231-113.rdu.redhat.com> <20100420135227.GC2628@linux.vnet.ibm.com> <20100421213543.GO2563@linux.vnet.ibm.com> <1271887029.7895.3535.camel@edumazet-laptop> <20100421221426.GS2563@linux.vnet.ibm.com> From: ebiederm@xmission.com (Eric W. Biederman) Date: Wed, 21 Apr 2010 16:26:17 -0700 In-Reply-To: <20100421221426.GS2563@linux.vnet.ibm.com> (Paul E. McKenney's message of "Wed\, 21 Apr 2010 15\:14\:26 -0700") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT X-XM-SPF: eid=;;;mid=;;;hst=in01.mta.xmission.com;;;ip=76.21.114.89;;;frm=ebiederm@xmission.com;;;spf=neutral X-SA-Exim-Connect-IP: 76.21.114.89 X-SA-Exim-Rcpt-To: paulmck@linux.vnet.ibm.com, netdev@vger.kernel.org, nauman@google.com, vgoyal@redhat.com, linux-kernel@vger.kernel.org, peterz@infradead.org, mingo@elte.hu, laijs@cn.fujitsu.com, eparis@redhat.com, miles.lane@gmail.com, eric.dumazet@gmail.com X-SA-Exim-Mail-From: ebiederm@xmission.com X-SA-Exim-Scanned: No (on in01.mta.xmission.com); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6834 Lines: 137 "Paul E. McKenney" writes: > On Wed, Apr 21, 2010 at 11:57:09PM +0200, Eric Dumazet wrote: >> Le mercredi 21 avril 2010 à 14:35 -0700, Paul E. McKenney a écrit : >> >> > > [ 33.425087] [ INFO: suspicious rcu_dereference_check() usage. ] >> > > [ 33.425090] --------------------------------------------------- >> > > [ 33.425094] net/core/dev.c:1993 invoked rcu_dereference_check() >> > > without protection! >> > > [ 33.425098] >> > > [ 33.425098] other info that might help us debug this: >> > > [ 33.425100] >> > > [ 33.425103] >> > > [ 33.425104] rcu_scheduler_active = 1, debug_locks = 1 >> > > [ 33.425108] 2 locks held by canberra-gtk-pl/4208: >> > > [ 33.425111] #0: (sk_lock-AF_INET){+.+.+.}, at: >> > > [] inet_stream_connect+0x3a/0x24d >> > > [ 33.425125] #1: (rcu_read_lock_bh){.+....}, at: >> > > [] dev_queue_xmit+0x14e/0x4b8 >> > > [ 33.425137] >> > > [ 33.425138] stack backtrace: >> > > [ 33.425142] Pid: 4208, comm: canberra-gtk-pl Not tainted 2.6.34-rc5 #18 >> > > [ 33.425146] Call Trace: >> > > [ 33.425154] [] lockdep_rcu_dereference+0x9d/0xa5 >> > > [ 33.425161] [] dev_queue_xmit+0x259/0x4b8 >> > > [ 33.425167] [] ? dev_queue_xmit+0x14e/0x4b8 >> > > [ 33.425173] [] ? _local_bh_enable_ip+0xcd/0xda >> > > [ 33.425180] [] neigh_resolve_output+0x234/0x285 >> > > [ 33.425188] [] ip_finish_output2+0x257/0x28c >> > > [ 33.425193] [] ip_finish_output+0x68/0x6a >> > > [ 33.425198] [] T.866+0x52/0x59 >> > > [ 33.425203] [] ip_output+0xaa/0xb4 >> > > [ 33.425209] [] ip_local_out+0x20/0x24 >> > > [ 33.425215] [] ip_queue_xmit+0x309/0x368 >> > > [ 33.425223] [] ? __kmalloc_track_caller+0x111/0x155 >> > > [ 33.425230] [] ? tcp_connect+0x223/0x3d3 >> > > [ 33.425236] [] tcp_transmit_skb+0x707/0x745 >> > > [ 33.425243] [] tcp_connect+0x376/0x3d3 >> > > [ 33.425250] [] ? secure_tcp_sequence_number+0x55/0x6f >> > > [ 33.425256] [] tcp_v4_connect+0x3df/0x455 >> > > [ 33.425263] [] ? lock_sock_nested+0xf3/0x102 >> > > [ 33.425269] [] inet_stream_connect+0xa4/0x24d >> > > [ 33.425276] [] sys_connect+0x90/0xd0 >> > > [ 33.425283] [] ? sysret_check+0x27/0x62 >> > > [ 33.425289] [] ? trace_hardirqs_on_caller+0x114/0x13f >> > > [ 33.425296] [] ? trace_hardirqs_on_thunk+0x3a/0x3f >> > > [ 33.425303] [] system_call_fastpath+0x16/0x1b >> > >> > This looks like an rcu_dereference() needs to instead be >> > rcu_dereference_bh(), but the line numbering in my version of >> > net/core/dev.c does not match yours. CCing netdev, hopefully >> > someone there will know which rcu_dereference() is indicated. >> >> This is already sorted out in David trees > > Very good!!! ;-) > >> > > [ 85.939528] [ INFO: suspicious rcu_dereference_check() usage. ] >> > > [ 85.939531] --------------------------------------------------- >> > > [ 85.939535] include/net/inet_timewait_sock.h:227 invoked >> > > rcu_dereference_check() without protection! >> > > [ 85.939539] >> > > [ 85.939540] other info that might help us debug this: >> > > [ 85.939541] >> > > [ 85.939544] >> > > [ 85.939545] rcu_scheduler_active = 1, debug_locks = 1 >> > > [ 85.939549] 2 locks held by gwibber-service/4798: >> > > [ 85.939552] #0: (&p->lock){+.+.+.}, at: [] >> > > seq_read+0x37/0x381 >> > > [ 85.939566] #1: (&(&hashinfo->ehash_locks[i])->rlock){+.-...}, >> > > at: [] established_get_next+0xc4/0x132 >> > > [ 85.939579] >> > > [ 85.939580] stack backtrace: >> > > [ 85.939585] Pid: 4798, comm: gwibber-service Not tainted 2.6.34-rc5 #18 >> > > [ 85.939588] Call Trace: >> > > [ 85.939598] [] lockdep_rcu_dereference+0x9d/0xa5 >> > > [ 85.939604] [] twsk_net+0x4f/0x57 >> > > [ 85.939610] [] established_get_next+0x54/0x132 >> > > [ 85.939615] [] tcp_seq_next+0x5d/0x6a >> > > [ 85.939621] [] seq_read+0x286/0x381 >> > > [ 85.939627] [] ? seq_read+0x0/0x381 >> > > [ 85.939633] [] proc_reg_read+0x8d/0xac >> > > [ 85.939640] [] vfs_read+0xa6/0x103 >> > > [ 85.939645] [] sys_read+0x45/0x69 >> > > [ 85.939652] [] system_call_fastpath+0x16/0x1b >> > >> > This one appears to be a case of missing rcu_read_lock(), but it is >> > not clear to me at what level it needs to go. >> > >> > Eric, any enlightenment on this one and the next one? >> >> Coming from commit b099ce2602d806deb41caaa578731848995cdb2a >> >From Eric Biederman (CCed) >> >> Apparently he added rcu to twsk_net(), but Changelog doesnt mention it. > > Thank you for chasing this down, Eric Dumazet! > > Eric Biederman, any enlightment? That change to twsk_net probably should have come in 575f4cd5a5b639457747434dbe18d175fa767db4. The point was to make twsk_net usable in an rcu context, instead of requiring a lock. Should it become rcu_deference_raw now that we have lockdep support? commit 575f4cd5a5b639457747434dbe18d175fa767db4 Author: Eric W. Biederman Date: Thu Dec 3 02:29:08 2009 +0000 net: Use rcu lookups in inet_twsk_purge. While we are looking up entries to free there is no reason to take the lock in inet_twsk_purge. We have to drop locks and restart occassionally anyway so adding a few more in case we get on the wrong list because of a timewait move is no big deal. At the same time not taking the lock for long periods of time is much more polite to the rest of the users of the hash table. In my test configuration of killing 4k network namespaces this change causes 4k back to back runs of inet_twsk_purge on an empty hash table to go from roughly 20.7s to 3.3s, and the total time to destroy 4k network namespaces goes from roughly 44s to 3.3s. Signed-off-by: Eric W. Biederman Acked-by: Eric Dumazet Signed-off-by: David S. Miller Eric -- 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/