From patchwork Thu Oct 30 13:21:29 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 6497 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 C34FEDDDEE for ; Fri, 31 Oct 2008 00:21:51 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754108AbYJ3NVs (ORCPT ); Thu, 30 Oct 2008 09:21:48 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753800AbYJ3NVs (ORCPT ); Thu, 30 Oct 2008 09:21:48 -0400 Received: from gw1.cosmosbay.com ([86.65.150.130]:48023 "EHLO gw1.cosmosbay.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753427AbYJ3NVr (ORCPT ); Thu, 30 Oct 2008 09:21:47 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) by gw1.cosmosbay.com (8.13.7/8.13.7) with ESMTP id m9UDLax2000753; Thu, 30 Oct 2008 14:21:37 +0100 Message-ID: <4909B4D9.20807@cosmosbay.com> Date: Thu, 30 Oct 2008 14:21:29 +0100 From: Eric Dumazet User-Agent: Thunderbird 2.0.0.17 (Windows/20080914) MIME-Version: 1.0 To: Alexander Beregalov CC: mchan@broadcom.com, linux-next@vger.kernel.org, Netdev Subject: Re: UDP/bnx2: inconsistent lock state (Re: linux-next: Tree for October 30) References: In-Reply-To: X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-1.6 (gw1.cosmosbay.com [0.0.0.0]); Thu, 30 Oct 2008 14:21:37 +0100 (CET) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Alexander Beregalov a écrit : > bnx2 0000:03:00.0: irq 38 for MSI/MSI-X > bnx2: eth0: using MSI > bnx2: eth0 NIC Copper Link is Up, 1000 Mbps full duplex, receive & > transmit flow control ON > > [ INFO: inconsistent lock state ] > 2.6.28-rc2-next-20081030 #4 > --------------------------------- > inconsistent {softirq-on-W} -> {in-softirq-W} usage. > swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes: > (&table->hash[i].lock){-+..}, at: [] > __udp4_lib_rcv+0x2a5/0x640 > {softirq-on-W} state was registered at: > [] __lock_acquire+0x2fd/0xc72 > [] lock_acquire+0x85/0xa9 > [] _spin_lock+0x3c/0x70 > [] udp_lib_unhash+0x2b/0x90 > [] sk_common_release+0x2f/0x83 > [] udp_lib_close+0x9/0xb > [] inet_release+0x58/0x5f > [] sock_release+0x20/0xc1 > [] sock_close+0x34/0x3a > [] __fput+0xc5/0x164 > [] fput+0x15/0x17 > [] filp_close+0x67/0x72 > [] put_files_struct+0x74/0xc8 > [] exit_files+0x47/0x4f > [] do_exit+0x27a/0x8b7 > [] do_group_exit+0x7f/0xaf > [] sys_exit_group+0x12/0x14 > [] system_call_fastpath+0x16/0x1b > [] 0xffffffffffffffff > irq event stamp: 56806 > hardirqs last enabled at (56806): [] > _local_bh_enable_ip+0xba/0xe3 > hardirqs last disabled at (56805): [] > _local_bh_enable_ip+0x54/0xe3 > softirqs last enabled at (56786): [] __do_softirq+0xf8/0x104 > softirqs last disabled at (56791): [] call_softirq+0x1c/0x28 > > other info that might help us debug this: > no locks held by swapper/0. > > stack backtrace: > Pid: 0, comm: swapper Not tainted 2.6.28-rc2-next-20081030 #4 > Call Trace: > [] valid_state+0x179/0x18c > [] mark_lock+0x14d/0x37a > [] __lock_acquire+0x282/0xc72 > [] ? rt_intern_hash+0x442/0x45c > [] ? local_bh_enable_ip+0x9/0xb > [] lock_acquire+0x85/0xa9 > [] ? __udp4_lib_rcv+0x2a5/0x640 > [] _spin_lock+0x3c/0x70 > [] ? __udp4_lib_rcv+0x2a5/0x640 > [] __udp4_lib_rcv+0x2a5/0x640 > [] udp_rcv+0x15/0x17 > [] ip_local_deliver+0xad/0x13d > [] ip_rcv+0x4b7/0x4ef > [] netif_receive_skb+0x213/0x23a > [] bnx2_poll_work+0x92e/0xaa5 > [] ? getnstimeofday+0x3a/0x96 > [] ? do_settimeofday+0x7f/0x131 > [] ? update_wall_time+0x38e/0x40f > [] ? mark_lock+0x1c/0x37a > [] ? __rcu_read_lock+0xa0/0xaf > [] bnx2_poll+0x128/0x20d > [] net_rx_action+0xd2/0x1f4 > [] __do_softirq+0x70/0x104 > [] call_softirq+0x1c/0x28 > [] do_softirq+0x39/0x8a > [] irq_exit+0x45/0xa2 > [] do_IRQ+0x16a/0x19c > [] ret_from_intr+0x0/0xf > [] ? mwait_idle+0x3e/0x48 > [] ? mwait_idle+0x35/0x48 > [] ? cpu_idle+0x59/0xc2 > [] ? start_secondary+0x263/0x26b Thanks for the report Alexander We need to use spin_lock_bh()/spin_unlock_bh() in udp_lib_unhash() Previous code was doing a write_lock_bh()/write_unlock_bh(), I cannot believe I missed this obvious thing... [PATCH] udp: Should use spin_lock_bh()/spin_unlock_bh() in udp_lib_unhash() Spotted by Alexander Beregalov Signed-off-by: Eric Dumazet diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index c3ecec8..f760b86 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -970,12 +970,12 @@ void udp_lib_unhash(struct sock *sk) unsigned int hash = udp_hashfn(sock_net(sk), sk->sk_hash); struct udp_hslot *hslot = &udptable->hash[hash]; - spin_lock(&hslot->lock); + spin_lock_bh(&hslot->lock); if (sk_del_node_init_rcu(sk)) { inet_sk(sk)->num = 0; sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1); } - spin_unlock(&hslot->lock); + spin_unlock_bh(&hslot->lock); } EXPORT_SYMBOL(udp_lib_unhash);