From patchwork Mon Aug 16 20:22:10 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 61835 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 1D55AB6F11 for ; Tue, 17 Aug 2010 06:23:17 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756571Ab0HPUW2 (ORCPT ); Mon, 16 Aug 2010 16:22:28 -0400 Received: from mail-ew0-f46.google.com ([209.85.215.46]:35880 "EHLO mail-ew0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756521Ab0HPUW0 (ORCPT ); Mon, 16 Aug 2010 16:22:26 -0400 Received: by ewy23 with SMTP id 23so2644474ewy.19 for ; Mon, 16 Aug 2010 13:22:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:cc :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=BpEXi62NOvAc3dWifl/RA3yQK2NnKOfK26olvs9+D/Y=; b=unTBycs2aRkV0n2LY62zEApoN6BRZsoZ0ua7mlsCUd5s5m+FYFkgd39b/MpFxUDAKH 0aBcZqUSqcF383YxUJqAUuyE6NJBVwhC178/3xNAeEZh2w+MZQvIXTuamW7oEf2LsVYx rJfeVJr6vThnVkfnIreY9noXw9k4qXjd6Dy8M= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=Mo3DvqQHnYOIgARbM0xlEjux1xZoRvHqrP+pHlgc+mh3p74iOnorYVAauva8E6b1pS ruDSjDNzZ4D1oW1FFsArH85sTEeAxc+GEGGEwWQnOTgMhV3tng40t9P9KjBoSxRzBeZP NV+Q6DLvM860TsQtiE9J1FJLw5rTS5z2ggqg0= Received: by 10.216.180.200 with SMTP id j50mr2973060wem.36.1281990144771; Mon, 16 Aug 2010 13:22:24 -0700 (PDT) Received: from [127.0.0.1] ([85.17.35.125]) by mx.google.com with ESMTPS id n17sm3596050weq.6.2010.08.16.13.22.21 (version=SSLv3 cipher=RC4-MD5); Mon, 16 Aug 2010 13:22:23 -0700 (PDT) Subject: Re: [GIT] Networking From: Eric Dumazet To: David Miller Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kaber@trash.net, Peter Zijlstra In-Reply-To: <20100816.123607.57459160.davem@davemloft.net> References: <20100814.220945.232761341.davem@davemloft.net> <1281869722.2942.20.camel@edumazet-laptop> <1281883637.2942.42.camel@edumazet-laptop> <20100816.123607.57459160.davem@davemloft.net> Date: Mon, 16 Aug 2010 22:22:10 +0200 Message-ID: <1281990130.2487.70.camel@edumazet-laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Le lundi 16 août 2010 à 12:36 -0700, David Miller a écrit : > I'm hesistent to say that we should put this kind of patch in. > > It will shut up lockdep for this specific case, but it also means > that if we do any other kinds of locking in this sequence we will > not validate it. > > The valuable of this is open for debate I guess. > > But locking is hard so I would say that disabling lockdep to kill a > warning it generates should be an absolute last resort. > > I also don't think making the locking mechanics conditional upon > LOCKDEP is sane either, exactly because it means lockdep is testing > something other than what actually gets used in practice. :-) Hmm, maybe just disable BH, not for whole duration, but for each cpu. Its a bit late here and I prefer to close this problem before whole earth shout on me. Thanks [PATCH] netfilter: {ip,ip6,arp}_tables: avoid lockdep false positive After commit 24b36f019 (netfilter: {ip,ip6,arp}_tables: dont block bottom half more than necessary), lockdep can raise a warning because we attempt to lock a spinlock with BH enabled, while the same lock is usually locked by another cpu in a softirq context. Disable again BH to avoid these lockdep warnings. Reported-by: Linus Torvalds Diagnosed-by: David S. Miller Signed-off-by: Eric Dumazet CC: Patrick McHardy CC: Peter Zijlstra Acked-by: David S. Miller --- net/ipv4/netfilter/arp_tables.c | 2 ++ net/ipv4/netfilter/ip_tables.c | 2 ++ net/ipv6/netfilter/ip6_tables.c | 2 ++ 3 files changed, 6 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index 6bccba3..51d6c31 100644 --- a/net/ipv4/netfilter/arp_tables.c +++ b/net/ipv4/netfilter/arp_tables.c @@ -735,6 +735,7 @@ static void get_counters(const struct xt_table_info *t, if (cpu == curcpu) continue; i = 0; + local_bh_disable(); xt_info_wrlock(cpu); xt_entry_foreach(iter, t->entries[cpu], t->size) { ADD_COUNTER(counters[i], iter->counters.bcnt, @@ -742,6 +743,7 @@ static void get_counters(const struct xt_table_info *t, ++i; } xt_info_wrunlock(cpu); + local_bh_enable(); } put_cpu(); } diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index c439721..97b64b2 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -909,6 +909,7 @@ get_counters(const struct xt_table_info *t, if (cpu == curcpu) continue; i = 0; + local_bh_disable(); xt_info_wrlock(cpu); xt_entry_foreach(iter, t->entries[cpu], t->size) { ADD_COUNTER(counters[i], iter->counters.bcnt, @@ -916,6 +917,7 @@ get_counters(const struct xt_table_info *t, ++i; /* macro does multi eval of i */ } xt_info_wrunlock(cpu); + local_bh_enable(); } put_cpu(); } diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c index 5359ef4..29a7bca 100644 --- a/net/ipv6/netfilter/ip6_tables.c +++ b/net/ipv6/netfilter/ip6_tables.c @@ -922,6 +922,7 @@ get_counters(const struct xt_table_info *t, if (cpu == curcpu) continue; i = 0; + local_bh_disable(); xt_info_wrlock(cpu); xt_entry_foreach(iter, t->entries[cpu], t->size) { ADD_COUNTER(counters[i], iter->counters.bcnt, @@ -929,6 +930,7 @@ get_counters(const struct xt_table_info *t, ++i; } xt_info_wrunlock(cpu); + local_bh_enable(); } put_cpu(); }