From patchwork Fri Jan 3 03:18:29 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "fan.du" X-Patchwork-Id: 306405 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 9F2672C009B for ; Fri, 3 Jan 2014 14:19:07 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753658AbaACDSp (ORCPT ); Thu, 2 Jan 2014 22:18:45 -0500 Received: from mail.windriver.com ([147.11.1.11]:35172 "EHLO mail.windriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751521AbaACDSn (ORCPT ); Thu, 2 Jan 2014 22:18:43 -0500 Received: from ALA-HCB.corp.ad.wrs.com (ala-hcb.corp.ad.wrs.com [147.11.189.41]) by mail.windriver.com (8.14.5/8.14.5) with ESMTP id s033IeNe029496 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Thu, 2 Jan 2014 19:18:40 -0800 (PST) Received: from iamroot-OptiPlex-780.corp.ad.wrs.com (128.224.162.236) by ALA-HCB.corp.ad.wrs.com (147.11.189.41) with Microsoft SMTP Server id 14.2.347.0; Thu, 2 Jan 2014 19:18:40 -0800 From: Fan Du To: CC: , Subject: [PATCHv4 net-next 3/8] {pktgen, xfrm} Correct xfrm_state_lock usage in xfrm_stateonly_find Date: Fri, 3 Jan 2014 11:18:29 +0800 Message-ID: <1388719114-26244-4-git-send-email-fan.du@windriver.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1388719114-26244-1-git-send-email-fan.du@windriver.com> References: <1388719114-26244-1-git-send-email-fan.du@windriver.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Acquiring xfrm_state_lock in process context is expected to turn BH off, as this lock is also used in BH context, namely xfrm state timer handler. Otherwise it surprises LOCKDEP with below messages. [ 81.422781] pktgen: Packet Generator for packet performance testing. Version: 2.74 [ 81.725194] [ 81.725211] ========================================================= [ 81.725212] [ INFO: possible irq lock inversion dependency detected ] [ 81.725215] 3.13.0-rc2+ #92 Not tainted [ 81.725216] --------------------------------------------------------- [ 81.725218] kpktgend_0/2780 just changed the state of lock: [ 81.725220] (xfrm_state_lock){+.+...}, at: [] xfrm_stateonly_find+0x41/0x1f0 [ 81.725231] but this lock was taken by another, SOFTIRQ-safe lock in the past: [ 81.725232] (&(&x->lock)->rlock){+.-...} [ 81.725232] [ 81.725232] and interrupts could create inverse lock ordering between them. [ 81.725232] [ 81.725235] [ 81.725235] other info that might help us debug this: [ 81.725237] Possible interrupt unsafe locking scenario: [ 81.725237] [ 81.725238] CPU0 CPU1 [ 81.725240] ---- ---- [ 81.725241] lock(xfrm_state_lock); [ 81.725243] local_irq_disable(); [ 81.725244] lock(&(&x->lock)->rlock); [ 81.725246] lock(xfrm_state_lock); [ 81.725248] [ 81.725249] lock(&(&x->lock)->rlock); [ 81.725251] [ 81.725251] *** DEADLOCK *** [ 81.725251] [ 81.725254] no locks held by kpktgend_0/2780. [ 81.725255] [ 81.725255] the shortest dependencies between 2nd lock and 1st lock: [ 81.725269] -> (&(&x->lock)->rlock){+.-...} ops: 8 { [ 81.725274] HARDIRQ-ON-W at: [ 81.725276] [] __lock_acquire+0x65b/0x1d70 [ 81.725282] [] lock_acquire+0x97/0x130 [ 81.725284] [] _raw_spin_lock+0x36/0x70 [ 81.725289] [] xfrm_timer_handler+0x43/0x290 [ 81.725292] [] __tasklet_hrtimer_trampoline+0x17/0x40 [ 81.725300] [] tasklet_hi_action+0xd7/0xf0 [ 81.725303] [] __do_softirq+0xe6/0x2d0 [ 81.725305] [] irq_exit+0x96/0xc0 [ 81.725308] [] smp_apic_timer_interrupt+0x4a/0x60 [ 81.725313] [] apic_timer_interrupt+0x6f/0x80 [ 81.725316] [] arch_cpu_idle+0x26/0x30 [ 81.725329] [] cpu_startup_entry+0x88/0x2b0 [ 81.725333] [] start_secondary+0x190/0x1f0 [ 81.725338] IN-SOFTIRQ-W at: [ 81.725340] [] __lock_acquire+0x62d/0x1d70 [ 81.725342] [] lock_acquire+0x97/0x130 [ 81.725344] [] _raw_spin_lock+0x36/0x70 [ 81.725347] [] xfrm_timer_handler+0x43/0x290 [ 81.725349] [] __tasklet_hrtimer_trampoline+0x17/0x40 [ 81.725352] [] tasklet_hi_action+0xd7/0xf0 [ 81.725355] [] __do_softirq+0xe6/0x2d0 [ 81.725358] [] irq_exit+0x96/0xc0 [ 81.725360] [] smp_apic_timer_interrupt+0x4a/0x60 [ 81.725363] [] apic_timer_interrupt+0x6f/0x80 [ 81.725365] [] arch_cpu_idle+0x26/0x30 [ 81.725368] [] cpu_startup_entry+0x88/0x2b0 [ 81.725370] [] start_secondary+0x190/0x1f0 [ 81.725373] INITIAL USE at: [ 81.725375] [] __lock_acquire+0x32a/0x1d70 [ 81.725385] [] lock_acquire+0x97/0x130 [ 81.725388] [] _raw_spin_lock+0x36/0x70 [ 81.725390] [] xfrm_timer_handler+0x43/0x290 [ 81.725394] [] __tasklet_hrtimer_trampoline+0x17/0x40 [ 81.725398] [] tasklet_hi_action+0xd7/0xf0 [ 81.725401] [] __do_softirq+0xe6/0x2d0 [ 81.725404] [] irq_exit+0x96/0xc0 [ 81.725407] [] smp_apic_timer_interrupt+0x4a/0x60 [ 81.725409] [] apic_timer_interrupt+0x6f/0x80 [ 81.725412] [] arch_cpu_idle+0x26/0x30 [ 81.725415] [] cpu_startup_entry+0x88/0x2b0 [ 81.725417] [] start_secondary+0x190/0x1f0 [ 81.725420] } [ 81.725421] ... key at: [] __key.46349+0x0/0x8 [ 81.725445] ... acquired at: [ 81.725446] [] lock_acquire+0x97/0x130 [ 81.725449] [] _raw_spin_lock+0x36/0x70 [ 81.725452] [] __xfrm_state_delete+0x37/0x140 [ 81.725454] [] xfrm_state_delete+0x2c/0x50 [ 81.725456] [] xfrm_state_flush+0xc7/0x1b0 [ 81.725458] [] pfkey_flush+0x7c/0x100 [af_key] [ 81.725465] [] pfkey_process+0x1c7/0x1f0 [af_key] [ 81.725468] [] pfkey_sendmsg+0x159/0x260 [af_key] [ 81.725471] [] sock_sendmsg+0xaf/0xc0 [ 81.725476] [] SYSC_sendto+0xfc/0x130 [ 81.725479] [] SyS_sendto+0xe/0x10 [ 81.725482] [] system_call_fastpath+0x16/0x1b [ 81.725484] [ 81.725486] -> (xfrm_state_lock){+.+...} ops: 11 { [ 81.725490] HARDIRQ-ON-W at: [ 81.725493] [] __lock_acquire+0x65b/0x1d70 [ 81.725504] [] lock_acquire+0x97/0x130 [ 81.725507] [] _raw_spin_lock_bh+0x3b/0x70 [ 81.725510] [] xfrm_state_flush+0x2f/0x1b0 [ 81.725513] [] pfkey_flush+0x7c/0x100 [af_key] [ 81.725516] [] pfkey_process+0x1c7/0x1f0 [af_key] [ 81.725519] [] pfkey_sendmsg+0x159/0x260 [af_key] [ 81.725522] [] sock_sendmsg+0xaf/0xc0 [ 81.725525] [] SYSC_sendto+0xfc/0x130 [ 81.725527] [] SyS_sendto+0xe/0x10 [ 81.725530] [] system_call_fastpath+0x16/0x1b [ 81.725533] SOFTIRQ-ON-W at: [ 81.725534] [] __lock_acquire+0x68a/0x1d70 [ 81.725537] [] lock_acquire+0x97/0x130 [ 81.725539] [] _raw_spin_lock+0x36/0x70 [ 81.725541] [] xfrm_stateonly_find+0x41/0x1f0 [ 81.725544] [] mod_cur_headers+0x793/0x7f0 [pktgen] [ 81.725547] [] pktgen_thread_worker+0xd42/0x1880 [pktgen] [ 81.725550] [] kthread+0xe4/0x100 [ 81.725555] [] ret_from_fork+0x7c/0xb0 [ 81.725565] INITIAL USE at: [ 81.725567] [] __lock_acquire+0x32a/0x1d70 [ 81.725569] [] lock_acquire+0x97/0x130 [ 81.725572] [] _raw_spin_lock_bh+0x3b/0x70 [ 81.725574] [] xfrm_state_flush+0x2f/0x1b0 [ 81.725576] [] pfkey_flush+0x7c/0x100 [af_key] [ 81.725580] [] pfkey_process+0x1c7/0x1f0 [af_key] [ 81.725583] [] pfkey_sendmsg+0x159/0x260 [af_key] [ 81.725586] [] sock_sendmsg+0xaf/0xc0 [ 81.725589] [] SYSC_sendto+0xfc/0x130 [ 81.725594] [] SyS_sendto+0xe/0x10 [ 81.725597] [] system_call_fastpath+0x16/0x1b [ 81.725599] } [ 81.725600] ... key at: [] xfrm_state_lock+0x18/0x50 [ 81.725606] ... acquired at: [ 81.725607] [] check_usage_backwards+0x110/0x150 [ 81.725609] [] mark_lock+0x196/0x2f0 [ 81.725611] [] __lock_acquire+0x68a/0x1d70 [ 81.725614] [] lock_acquire+0x97/0x130 [ 81.725616] [] _raw_spin_lock+0x36/0x70 [ 81.725627] [] xfrm_stateonly_find+0x41/0x1f0 [ 81.725629] [] mod_cur_headers+0x793/0x7f0 [pktgen] [ 81.725632] [] pktgen_thread_worker+0xd42/0x1880 [pktgen] [ 81.725635] [] kthread+0xe4/0x100 [ 81.725637] [] ret_from_fork+0x7c/0xb0 [ 81.725640] [ 81.725641] [ 81.725641] stack backtrace: [ 81.725645] CPU: 0 PID: 2780 Comm: kpktgend_0 Not tainted 3.13.0-rc2+ #92 [ 81.725647] Hardware name: innotek GmbH VirtualBox, BIOS VirtualBox 12/01/2006 [ 81.725649] ffffffff82537b80 ffff880018199988 ffffffff8176af37 0000000000000007 [ 81.725652] ffff8800181999f0 ffff8800181999d8 ffffffff81099358 ffffffff82537b80 [ 81.725655] ffffffff81a32def ffff8800181999f4 0000000000000000 ffff880002cbeaa8 [ 81.725659] Call Trace: [ 81.725664] [] dump_stack+0x46/0x58 [ 81.725667] [] print_irq_inversion_bug.part.42+0x1e8/0x1f0 [ 81.725670] [] check_usage_backwards+0x110/0x150 [ 81.725672] [] mark_lock+0x196/0x2f0 [ 81.725675] [] ? check_usage_forwards+0x150/0x150 [ 81.725685] [] __lock_acquire+0x68a/0x1d70 [ 81.725691] [] ? sched_clock_local+0x25/0x90 [ 81.725694] [] ? sched_clock_cpu+0xa8/0x120 [ 81.725697] [] ? __lock_acquire+0x32a/0x1d70 [ 81.725699] [] ? xfrm_stateonly_find+0x41/0x1f0 [ 81.725702] [] lock_acquire+0x97/0x130 [ 81.725704] [] ? xfrm_stateonly_find+0x41/0x1f0 [ 81.725707] [] ? sched_clock_local+0x25/0x90 [ 81.725710] [] _raw_spin_lock+0x36/0x70 [ 81.725712] [] ? xfrm_stateonly_find+0x41/0x1f0 [ 81.725715] [] ? lock_release_holdtime.part.26+0x1c/0x1a0 [ 81.725717] [] xfrm_stateonly_find+0x41/0x1f0 [ 81.725721] [] mod_cur_headers+0x793/0x7f0 [pktgen] [ 81.725724] [] pktgen_thread_worker+0xd42/0x1880 [pktgen] [ 81.725727] [] ? pktgen_thread_worker+0xb11/0x1880 [pktgen] [ 81.725729] [] ? trace_hardirqs_on+0xd/0x10 [ 81.725733] [] ? _raw_spin_unlock_irq+0x30/0x40 [ 81.725745] [] ? e1000_clean+0x9d0/0x9d0 [ 81.725751] [] ? __init_waitqueue_head+0x60/0x60 [ 81.725753] [] ? __init_waitqueue_head+0x60/0x60 [ 81.725757] [] ? mod_cur_headers+0x7f0/0x7f0 [pktgen] [ 81.725759] [] kthread+0xe4/0x100 [ 81.725762] [] ? flush_kthread_worker+0x170/0x170 [ 81.725765] [] ret_from_fork+0x7c/0xb0 [ 81.725768] [] ? flush_kthread_worker+0x170/0x170 Signed-off-by: Fan Du --- net/xfrm/xfrm_state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 68c2f35..f7cb4a3 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -900,7 +900,7 @@ xfrm_stateonly_find(struct net *net, u32 mark, unsigned int h; struct xfrm_state *rx = NULL, *x = NULL; - spin_lock(&net->xfrm.xfrm_state_lock); + spin_lock_bh(&net->xfrm.xfrm_state_lock); h = xfrm_dst_hash(net, daddr, saddr, reqid, family); hlist_for_each_entry(x, net->xfrm.state_bydst+h, bydst) { if (x->props.family == family && @@ -918,7 +918,7 @@ xfrm_stateonly_find(struct net *net, u32 mark, if (rx) xfrm_state_hold(rx); - spin_unlock(&net->xfrm.xfrm_state_lock); + spin_unlock_bh(&net->xfrm.xfrm_state_lock); return rx;