Message-ID: <89770990a66eb25000248daaf2b0b8fcf105c8d7.camel@gmail.com>
Subject: Re: [PATCH v2 28/28] NFS: Remove redundant open context from
 nfs_page
From:   Anna Schumaker <schumaker.anna@gmail.com>
To:     Trond Myklebust <trondmy@hammerspace.com>,
        "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Date:   Mon, 08 Apr 2019 13:11:31 -0400
In-Reply-To: <9498d534b8e59f903bf750183a08d22a6ae7ba9a.camel@hammerspace.com>
References: <20190329215948.107328-1-trond.myklebust@hammerspace.com>
         <20190329215948.107328-2-trond.myklebust@hammerspace.com>
         <20190329215948.107328-3-trond.myklebust@hammerspace.com>
         <20190329215948.107328-4-trond.myklebust@hammerspace.com>
         <20190329215948.107328-5-trond.myklebust@hammerspace.com>
         <20190329215948.107328-6-trond.myklebust@hammerspace.com>
         <20190329215948.107328-7-trond.myklebust@hammerspace.com>
         <20190329215948.107328-8-trond.myklebust@hammerspace.com>
         <20190329215948.107328-9-trond.myklebust@hammerspace.com>
         <20190329215948.107328-10-trond.myklebust@hammerspace.com>
         <20190329215948.107328-11-trond.myklebust@hammerspace.com>
         <20190329215948.107328-12-trond.myklebust@hammerspace.com>
         <20190329215948.107328-13-trond.myklebust@hammerspace.com>
         <20190329215948.107328-14-trond.myklebust@hammerspace.com>
         <20190329215948.107328-15-trond.myklebust@hammerspace.com>
         <20190329215948.107328-16-trond.myklebust@hammerspace.com>
         <20190329215948.107328-17-trond.myklebust@hammerspace.com>
         <20190329215948.107328-18-trond.myklebust@hammerspace.com>
         <20190329215948.107328-19-trond.myklebust@hammerspace.com>
         <20190329215948.107328-20-trond.myklebust@hammerspace.com>
         <20190329215948.107328-21-trond.myklebust@hammerspace.com>
         <20190329215948.107328-22-trond.myklebust@hammerspace.com>
         <20190329215948.107328-23-trond.myklebust@hammerspace.com>
         <20190329215948.107328-24-trond.myklebust@hammerspace.com>
         <20190329215948.107328-25-trond.myklebust@hammerspace.com>
         <20190329215948.107328-26-trond.myklebust@hammerspace.com>
         <20190329215948.107328-27-trond.myklebust@hammerspace.com>
         <20190329215948.107328-28-trond.myklebust@hammerspace.com>
         <20190329215948.107328-29-trond.myklebust@hammerspace.com>
         <c92a3c3d629636a0ce88a04098565345ce5ed5d8.camel@gmail.com>
         <9498d534b8e59f903bf750183a08d22a6ae7ba9a.camel@hammerspace.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.32.0 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Sat, 2019-04-06 at 14:14 +0000, Trond Myklebust wrote:
> Hi Anna
> 
> On Fri, 2019-04-05 at 15:42 -0400, Anna Schumaker wrote:
> > Hi Trond,
> > 
> > I'm having some trouble with xfstests generic/464 after applying this
> > patch.
> > Here is the oops I'm getting, I'm able to reproduce it fairly easily
> > against all
> > NFS versions: 
> > 
> > [   41.182015] BUG: unable to handle kernel NULL pointer dereference
> > at
> > 0000000000000048
> > [   41.182051] #PF error: [normal kernel read fault]
> > [   41.182071] PGD 0 P4D 0 
> > [   41.182086] Oops: 0000 [#1] PREEMPT SMP PTI
> > [   41.182104] CPU: 1 PID: 14515 Comm: kworker/1:6 Not tainted 5.1.0-
> > rc3-ANNA+
> > #5292
> > [   41.182131] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> > [   41.182174] Workqueue: nfsiod rpc_async_release [sunrpc]
> > [   41.182209] RIP: 0010:__put_nfs_open_context+0xd/0x110 [nfs]
> > [   41.182227] Code: f8 ba a8 00 00 00 be c0 0c 00 00 e8 5d c0 58 f7
> > 48 89 c3 48
> > 85 c0 74 c5 e9 15 ff ff ff 0f 1f 44 00 00 41 56 41 55 41 54 55 53
> > <48> 8b 47 48
> > 48 8b 68 30 4c 8b 60 68 f0 ff 0f 0f 88 14 53 01 00 74
> > [   41.182267] RSP: 0018:ffffae5f04b6be30 EFLAGS: 00010246
> > [   41.182279] RAX: 0000000000000000 RBX: ffffa22aae71a680 RCX:
> > 0000000000000000
> > [   41.182295] RDX: ffffffffb9205dc8 RSI: 0000000000000000 RDI:
> > 0000000000000000
> > [   41.182311] RBP: 0000000000000801 R08: ffffa22aa5ca0000 R09:
> > ffffa22aba267800
> > [   41.182327] R10: 0000000000000000 R11: 0000000000000000 R12:
> > ffffce5effd1c200
> > [   41.182351] R13: 0000000000000000 R14: ffffa22aad2aa6c0 R15:
> > 0ffffce5effd1c20
> > [   41.182378] FS:  0000000000000000(0000) GS:ffffa22abcb00000(0000)
> > knlGS:0000000000000000
> > [   41.182411] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   41.182435] CR2: 0000000000000048 CR3: 00000000af972005 CR4:
> > 0000000000160ee0
> > [   41.182468] Call Trace:
> > [   41.182495]  nfs_commitdata_release+0x15/0x30 [nfs]
> > [   41.182536]  rpc_free_task+0x39/0x70 [sunrpc]
> > [   41.182571]  rpc_async_release+0x29/0x40 [sunrpc]
> > [   41.182594]  process_one_work+0x1eb/0x410
> > [   41.182611]  worker_thread+0x2d/0x3d0
> > [   41.182621]  ? process_one_work+0x410/0x410
> > [   41.182632]  kthread+0x112/0x130
> > [   41.182641]  ? kthread_park+0x80/0x80
> > [   41.182652]  ret_from_fork+0x35/0x40
> > [   41.182662] Modules linked in: nfsv3 nfs fscache rpcrdma ib_isert
> > 
> 
> Does the following patch help to fix this?

This patch does fix the problem. Thanks!

Anna

> 
> Cheers
>   Trond
> 
> 8<---------------------------------------
> From d84c699bba78d66d3ccd3f76a6aae573bdfddc05 Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> Date: Sat, 6 Apr 2019 10:05:55 -0400
> Subject: [PATCH] NFS: Ensure that all nfs lock contexts have a valid open
>  context
> 
> Force the lock context to keep a reference to the parent open
> context so that we can guarantee the validity of the latter.
> 
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  fs/nfs/inode.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 414a90d48493..efc45f22c581 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -885,10 +885,14 @@ struct nfs_lock_context *nfs_get_lock_context(struct
> nfs_open_context *ctx)
>  		spin_lock(&inode->i_lock);
>  		res = __nfs_find_lock_context(ctx);
>  		if (res == NULL) {
> -			list_add_tail_rcu(&new->list, &ctx->lock_context.list);
> -			new->open_context = ctx;
> -			res = new;
> -			new = NULL;
> +			new->open_context = get_nfs_open_context(ctx);
> +			if (new->open_context) {
> +				list_add_tail_rcu(&new->list,
> +						&ctx->lock_context.list);
> +				res = new;
> +				new = NULL;
> +			} else
> +				res = ERR_PTR(-EBADF);
>  		}
>  		spin_unlock(&inode->i_lock);
>  		kfree(new);
> @@ -906,6 +910,7 @@ void nfs_put_lock_context(struct nfs_lock_context *l_ctx)
>  		return;
>  	list_del_rcu(&l_ctx->list);
>  	spin_unlock(&inode->i_lock);
> +	put_nfs_open_context(ctx);
>  	kfree_rcu(l_ctx, rcu_head);
>  }
>  EXPORT_SYMBOL_GPL(nfs_put_lock_context);
> -- 
> 2.20.1
> 
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
> 
>