From patchwork Sun Nov 30 16:36:56 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 11472 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.176.167]) by ozlabs.org (Postfix) with ESMTP id A167DDDDCA for ; Mon, 1 Dec 2008 03:37:11 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751472AbYK3QhG (ORCPT ); Sun, 30 Nov 2008 11:37:06 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751459AbYK3QhF (ORCPT ); Sun, 30 Nov 2008 11:37:05 -0500 Received: from gw1.cosmosbay.com ([86.65.150.130]:39694 "EHLO gw1.cosmosbay.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751431AbYK3QhD (ORCPT ); Sun, 30 Nov 2008 11:37:03 -0500 Received: from [127.0.0.1] (localhost [127.0.0.1]) by gw1.cosmosbay.com (8.13.7/8.13.7) with ESMTP id mAUGavYJ011734; Sun, 30 Nov 2008 17:36:57 +0100 Message-ID: <4932C128.6040401@cosmosbay.com> Date: Sun, 30 Nov 2008 17:36:56 +0100 From: Eric Dumazet User-Agent: Thunderbird 2.0.0.18 (Windows/20081105) MIME-Version: 1.0 To: Alexey Dobriyan CC: netdev@vger.kernel.org, "David S. Miller" Subject: Re: net-next: lockdep complains re percpu counters References: <20081130130726.GA4365@x200.localdomain> In-Reply-To: <20081130130726.GA4365@x200.localdomain> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-1.6 (gw1.cosmosbay.com [0.0.0.0]); Sun, 30 Nov 2008 17:36:58 +0100 (CET) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Alexey Dobriyan a écrit : > This is net-next as of c5419e6f054c877339f754e02c3b1dafd88cd96c > aka "cxgb3: Fix sparse warning and micro-optimize is_pure_response()". > > ================================= > [ INFO: inconsistent lock state ] > 2.6.28-rc6-netns #2 > --------------------------------- > inconsistent {softirq-on-W} -> {in-softirq-W} usage. > recvfrom01/7632 [HC0[0]:SC1[1]:HE1:SE0] takes: > (&fbc->lock){-+..}, at: [] __percpu_counter_add+0x63/0xc0 > {softirq-on-W} state was registered at: > [] __lock_acquire+0x4b9/0x9c0 > [] lock_acquire+0x56/0x80 > [] _spin_lock+0x36/0x50 > [] __percpu_counter_add+0x63/0xc0 > [] get_empty_filp+0x59/0x110 > [] path_lookup_open+0x30/0xc0 > [] do_filp_open+0xae/0x8a0 > [] do_sys_open+0x76/0xd0 > [] sys_open+0x1b/0x20 > [] system_call_fastpath+0x16/0x1b > [] 0xffffffffffffffff > irq event stamp: 5698 > hardirqs last enabled at (5698): [] debug_check_no_locks_freed+0xa1/0x150 > hardirqs last disabled at (5697): [] debug_check_no_locks_freed+0x40/0x150 > softirqs last enabled at (5656): [] release_sock+0xc1/0xf0 > softirqs last disabled at (5657): [] call_softirq+0x1c/0x30 > > other info that might help us debug this: > 2 locks held by recvfrom01/7632: > #0: (slock-AF_INET/1){-+..}, at: [] tcp_v4_rcv+0x50b/0x840 > #1: (slock-AF_INET){-+..}, at: [] sk_clone+0xe1/0x340 > > stack backtrace: > Pid: 7632, comm: recvfrom01 Not tainted 2.6.28-rc6-netns #2 > Call Trace: > [] print_usage_bug+0x18d/0x1e0 > [] mark_lock+0x887/0xb70 > [] __lock_acquire+0x442/0x9c0 > [] lock_acquire+0x56/0x80 > [] ? __percpu_counter_add+0x63/0xc0 > [] _spin_lock+0x36/0x50 > [] ? __percpu_counter_add+0x63/0xc0 > [] __percpu_counter_add+0x63/0xc0 > [] sk_clone+0x307/0x340 > [] inet_csk_clone+0x11/0xa0 > [] tcp_create_openreq_child+0x24/0x470 > [] tcp_v4_syn_recv_sock+0x53/0x210 > [] tcp_check_req+0x125/0x450 > [] ? local_bh_enable+0xa4/0x110 > [] tcp_v4_do_rcv+0x13d/0x230 > [] tcp_v4_rcv+0x745/0x840 > [] ip_local_deliver_finish+0x124/0x2b0 > [] ip_local_deliver+0xa0/0xb0 > [] ip_rcv_finish+0x10c/0x3c0 > [] ip_rcv+0x1c0/0x300 > [] netif_receive_skb+0x379/0x400 > [] ? process_backlog+0x8b/0x100 > [] process_backlog+0x97/0x100 > [] net_rx_action+0x14a/0x210 > [] __do_softirq+0x88/0x150 > [] ? release_sock+0xc1/0xf0 > [] call_softirq+0x1c/0x30 > [] do_softirq+0x77/0xd0 > [] ? release_sock+0xc1/0xf0 > [] local_bh_enable_ip+0xeb/0x110 > [] _spin_unlock_bh+0x39/0x40 > [] ? release_sock+0x8e/0xf0 > [] release_sock+0xc1/0xf0 > [] inet_stream_connect+0x198/0x2e0 > [] ? autoremove_wake_function+0x0/0x40 > [] ? autoremove_wake_function+0x0/0x40 > [] sys_connect+0x7d/0xc0 > [] ? sysret_check+0x27/0x62 > [] ? trace_hardirqs_on_caller+0xba/0x130 > [] ? trace_hardirqs_on_thunk+0x3a/0x3f > [] system_call_fastpath+0x16/0x1b > -- Hi Alexey, thanks for the report. I checked all per_cpu_counter_xxx() usages in network tree, and I think all call sites are BH enabled except one in inet_csk_listen_stop(), and we can change this. Could you try following patch please ? [PATCH] net: percpu_counter_inc() should not be called in BH-disabled section I checked all per_cpu_counter_xxx() usages in network tree, and I think all call sites are BH enabled except one in inet_csk_listen_stop(). commit dd24c00191d5e4a1ae896aafe33c6b8095ab4bd1 (net: Use a percpu_counter for orphan_count) replaced atomic_t orphan_count to a percpu_counter. atomic_inc()/atomic_dec() can be called from any context, while percpu_counter_xxx() should be called from a consistent state. For orphan_count, this context can be the BH-enabled one. Signed-off-by: Eric Dumazet diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index 1ccdbba..fe32255 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -632,6 +632,8 @@ void inet_csk_listen_stop(struct sock *sk) acc_req = req->dl_next; + percpu_counter_inc(sk->sk_prot->orphan_count); + local_bh_disable(); bh_lock_sock(child); WARN_ON(sock_owned_by_user(child)); @@ -641,8 +643,6 @@ void inet_csk_listen_stop(struct sock *sk) sock_orphan(child); - percpu_counter_inc(sk->sk_prot->orphan_count); - inet_csk_destroy_sock(child); bh_unlock_sock(child);