From patchwork Mon Aug 11 15:35:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 4707791 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2F375C0338 for ; Mon, 11 Aug 2014 15:36:04 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 16C1520127 for ; Mon, 11 Aug 2014 15:36:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C553B20115 for ; Mon, 11 Aug 2014 15:36:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753926AbaHKPgA (ORCPT ); Mon, 11 Aug 2014 11:36:00 -0400 Received: from mail-qa0-f51.google.com ([209.85.216.51]:39160 "EHLO mail-qa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753914AbaHKPgA (ORCPT ); Mon, 11 Aug 2014 11:36:00 -0400 Received: by mail-qa0-f51.google.com with SMTP id k15so7707071qaq.38 for ; Mon, 11 Aug 2014 08:35:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:date:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=nOuLTnb5DLngCGLwsFkXcUQ+8wtgk4sA2xKdSJsePmM=; b=FZivAS3gLhF/7FnLeSIKAkFHYpYzosHRx3kirPhJ2rs1SzcdKDwRY1EkyXBgAu1QjT kDKwkPMYZzKyfBiKV/XQLXsItBCjvcSSy2nHlDHiTNpPmZxs1hEesoYDMPcVlWaNruOk NXkRQd5b3P1pjjc4QjlSG/iyMuNAHNco6wI/9C+lzny1kAexR1LB97ALyQTJgZ3dAvnZ Qx+0ofxnrvPpto2StQaZgdl5oj3E0g3MzQ3t0cUEyfgYyEx+FWDkQCaYWAKnMLKmeupW C+s487Vi/BmYOVy47sYhK+UU8DE5KvHn5rgszYb/bpB6n14PzLFqpmSbaI3Qq60vkm+p kxKg== X-Gm-Message-State: ALoCoQmY+ajocQQ5rzWOmTvtShBN03zBoTTgNaXknwzVYAPU4kNjRuzBOJJ9M1At8IgnA64WViLR X-Received: by 10.140.20.17 with SMTP id 17mr48048951qgi.85.1407771359509; Mon, 11 Aug 2014 08:35:59 -0700 (PDT) Received: from tlielax.poochiereds.net ([2001:470:8:d63:3a60:77ff:fe93:a95d]) by mx.google.com with ESMTPSA id 95sm14329830qgg.25.2014.08.11.08.35.59 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Aug 2014 08:35:59 -0700 (PDT) From: Jeff Layton X-Google-Original-From: Jeff Layton Date: Mon, 11 Aug 2014 11:35:57 -0400 To: Christoph Hellwig Cc: trond.myklebust@primarydata.com, linux-nfs@vger.kernel.org Subject: Re: [PATCH v2 2/3] nfs4: queue free_lock_state job submission to nfsiod Message-ID: <20140811113557.692d408b@tlielax.poochiereds.net> In-Reply-To: <20140811150924.GA3683@infradead.org> References: <1398940127-31150-1-git-send-email-jlayton@poochiereds.net> <1398940127-31150-3-git-send-email-jlayton@poochiereds.net> <20140811104253.GA4747@infradead.org> <20140811075013.77e0a29e@tlielax.poochiereds.net> <20140811090417.0287b32b@tlielax.poochiereds.net> <20140811150924.GA3683@infradead.org> X-Mailer: Claws Mail 3.10.1 (GTK+ 2.24.22; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, 11 Aug 2014 08:09:24 -0700 Christoph Hellwig wrote: > > I managed to hit a similar but different issue with your patch applied (note > the slab poisoning pattern in rax): > > generic/089 409s ...[ 399.057379] general protection fault: 0000 [#1] > SMP > [ 399.059137] Modules linked in: > [ 399.060089] CPU: 2 PID: 4367 Comm: kworker/2:2 Not tainted 3.16.0-rc6+ #1153 > [ 399.060617] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 > [ 399.060617] Workqueue: nfsiod free_lock_state_work > [ 399.060617] task: ffff88007ab68810 ti: ffff88007c3b4000 task.ti: ffff88007c3b4000 > [ 399.060617] RIP: 0010:[] [] nfs41_free_lock_state+0x2b/0x70 > [ 399.060617] RSP: 0018:ffff88007c3b7d18 EFLAGS: 00010286 > [ 399.060617] RAX: 6b6b6b6b6b6b6b6b RBX: ffff88007cdd3800 RCX: 0000000000000000 > [ 399.060617] RDX: ffffffff81e04c60 RSI: ffff88007cdd39a0 RDI: ffff880079e5a000 > [ 399.060617] RBP: ffff88007c3b7d38 R08: ffffffff832df6d0 R09: 000001c90f100000 > [ 399.060617] R10: 0000000000000000 R11: 00000000000656f0 R12: ffff880079e5a000 > [ 399.060617] R13: ffff88007fd18b00 R14: ffff88007cdd39c0 R15: 0000000000000000 > [ 399.060617] FS: 0000000000000000(0000) GS:ffff88007fd00000(0000) knlGS:0000000000000000 > [ 399.060617] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > [ 399.060617] CR2: 00007f5ac2f56800 CR3: 000000007a95b000 CR4: 00000000000006e0 > [ 399.060617] Stack: > [ 399.060617] 000000007fd13240 ffff88007a8f7800 ffff88007fd13240 ffff88007fd18b00 > [ 399.060617] ffff88007c3b7d58 ffffffff813621ae ffff88007cdd39c0 ffff88007a4d0c40 > [ 399.060617] ffff88007c3b7dd8 ffffffff810cc877 ffffffff810cc80d ffff88007fd13258 > [ 399.060617] Call Trace: > [ 399.060617] [] free_lock_state_work+0x2e/0x40 > [ 399.060617] [] process_one_work+0x1c7/0x490 > [ 399.060617] [] ? process_one_work+0x15d/0x490 > [ 399.060617] [] worker_thread+0x119/0x4f0 > [ 399.060617] [] ? trace_hardirqs_on+0xd/0x10 > [ 399.060617] [] ? init_pwq+0x190/0x190 > [ 399.060617] [] kthread+0xdf/0x100 > [ 399.060617] [] ? __init_kthread_worker+0x70/0x70 > [ 399.060617] [] ret_from_fork+0x7c/0xb0 > [ 399.060617] [] ? __init_kthread_worker+0x70/0x70 > > (gdb) l *(nfs41_free_lock_state+0x2b) > 0xffffffff8134e5bb is in nfs41_free_lock_state (fs/nfs/nfs4proc.c:8313). > 8308 nfs41_free_lock_state(struct nfs_server *server, struct > nfs4_lock_state *lsp) > 8309 { > 8310 struct rpc_task *task; > 8311 struct rpc_cred *cred = lsp->ls_state->owner->so_cred; > 8312 > 8313 task = _nfs41_free_stateid(server, &lsp->ls_stateid, > cred, false); > 8314 nfs4_free_lock_state(server, lsp); > 8315 if (IS_ERR(task)) > 8316 return; > 8317 rpc_put_task(task); > Oof -- right. Same problem, just in a different spot. So there, we need the openowner. We don't have a pointer directly to that, so we're probably best off just holding a reference to the open stateid, and pinning the superblock too. Maybe something like this instead? I'm also running xfstests on a VM now to see if I can reproduce this and verify the fix: ---------------------[snip]----------------------- [PATCH] nfs: pin superblock and open state when running free_lock_state operations Christoph reported seeing this oops when running xfstests on a v4.1 client: generic/089 242s ...[ 2187.041239] general protection fault: 0000 [#1] SMP [ 2187.042899] Modules linked in: [ 2187.044000] CPU: 0 PID: 11913 Comm: kworker/0:1 Not tainted 3.16.0-rc6+ #1151 [ 2187.044287] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 2187.044287] Workqueue: nfsiod free_lock_state_work [ 2187.044287] task: ffff880072b50cd0 ti: ffff88007a4ec000 task.ti: ffff88007a4ec000 [ 2187.044287] RIP: 0010:[] [] free_lock_state_work+0x16/0x30 [ 2187.044287] RSP: 0018:ffff88007a4efd58 EFLAGS: 00010296 [ 2187.044287] RAX: 6b6b6b6b6b6b6b6b RBX: ffff88007a947ac0 RCX: 8000000000000000 [ 2187.044287] RDX: ffffffff826af9e0 RSI: ffff88007b093c00 RDI: ffff88007b093db8 [ 2187.044287] RBP: ffff88007a4efd58 R08: ffffffff832d3e10 R09: 000001c40efc0000 [ 2187.044287] R10: 0000000000000000 R11: 0000000000059e30 R12: ffff88007fc13240 [ 2187.044287] R13: ffff88007fc18b00 R14: ffff88007b093db8 R15: 0000000000000000 [ 2187.044287] FS: 0000000000000000(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000 [ 2187.044287] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b [ 2187.044287] CR2: 00007f93ec33fb80 CR3: 0000000079dc2000 CR4: 00000000000006f0 [ 2187.044287] Stack: [ 2187.044287] ffff88007a4efdd8 ffffffff810cc877 ffffffff810cc80d ffff88007fc13258 [ 2187.044287] 000000007a947af0 0000000000000000 ffffffff8353ccc8 ffffffff82b6f3d0 [ 2187.044287] 0000000000000000 ffffffff82267679 ffff88007a4efdd8 ffff88007fc13240 [ 2187.044287] Call Trace: [ 2187.044287] [] process_one_work+0x1c7/0x490 [ 2187.044287] [] ? process_one_work+0x15d/0x490 [ 2187.044287] [] worker_thread+0x119/0x4f0 [ 2187.044287] [] ? trace_hardirqs_on+0xd/0x10 [ 2187.044287] [] ? init_pwq+0x190/0x190 [ 2187.044287] [] kthread+0xdf/0x100 [ 2187.044287] [] ? __init_kthread_worker+0x70/0x70 [ 2187.044287] [] ret_from_fork+0x7c/0xb0 [ 2187.044287] [] ? __init_kthread_worker+0x70/0x70 [ 2187.044287] Code: 0f 1f 44 00 00 31 c0 5d c3 66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 8d b7 48 fe ff ff 48 8b 87 58 fe ff ff 48 89 e5 48 8b 40 30 <48> 8b 00 48 8b 10 48 89 c7 48 8b 92 90 03 00 00 ff 52 28 5d c3 [ 2187.044287] RIP [] free_lock_state_work+0x16/0x30 [ 2187.044287] RSP [ 2187.103626] ---[ end trace 0f11326d28e5d8fa ]--- It appears that the lock state outlived the open state with which it was associated. Fix this by pinning down the open stateid and the superblock prior to queueing the workqueue job, and then releasing putting those references once the RPC task has been queued. Reported-by: Christoph Hellwig Signed-off-by: Jeff Layton --- fs/nfs/nfs4state.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index a770c8e469a7..fb29c7cbe8e3 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -804,11 +804,14 @@ free_lock_state_work(struct work_struct *work) { struct nfs4_lock_state *lsp = container_of(work, struct nfs4_lock_state, ls_release); - struct nfs4_state *state = lsp->ls_state; - struct nfs_server *server = state->owner->so_server; + struct nfs4_state *osp = lsp->ls_state; + struct super_block *sb = osp->inode->i_sb; + struct nfs_server *server = NFS_SB(sb); struct nfs_client *clp = server->nfs_client; clp->cl_mvops->free_lock_state(server, lsp); + nfs4_put_open_state(osp); + nfs_sb_deactive(sb); } /* @@ -896,9 +899,11 @@ void nfs4_put_lock_state(struct nfs4_lock_state *lsp) if (list_empty(&state->lock_states)) clear_bit(LK_STATE_IN_USE, &state->flags); spin_unlock(&state->state_lock); - if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags)) + if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags)) { + atomic_inc(&lsp->ls_state->count); + nfs_sb_active(lsp->ls_state->inode->i_sb); queue_work(nfsiod_workqueue, &lsp->ls_release); - else { + } else { server = state->owner->so_server; nfs4_free_lock_state(server, lsp); }