[pnfs] [PATCH 0/2] A couple of fixes for the pNFS client

[Trond Myklebust]

On Tue, 2008-07-15 at 09:26 +0300, Benny Halevy wrote:
> On Jul. 15, 2008, 6:39 +0300, Trond Myklebust <Trond.Myklebust at netapp.com> wrote:
> > The following series fixes a couple of serious problems in the pNFS
> > GETLAYOUT client code.
> > 
> >   - Routines that may be run in the NFS writeback path may not allocate
> >     memory using GFP_KERNEL.
> >   - It is illegal for an asynchronous RPC callback function to make
> >     synchronous RPC calls.
> > 
> > Please note that this is by no means an exhaustive list of the above two
> > types of bug...
> > 
> 
> Hi Trond, thanks for the patches.  I'll apply them later today.
> I'll look for more similar cases as you noted.

Actually, I messed up. I meant to change that GFP_KERNEL into a GFP_NOFS
(see below) so that we don't deadlock the VM by triggering a new cascade
of NFS writebacks in a low memory situation.

Cheers
  Trond

----------------------------------------------------------------------------
From: Trond Myklebust <Trond.Myklebust at netapp.com>
Date: Mon, 14 Jul 2008 18:01:47 -0400
NFS: Fix an illegal allocation...

Routines that may be run in the NFS writeback path may not allocate memory
using GFP_KERNEL, since that may initiate more I/O in order to free up
resources.

Signed-off-by: Trond Myklebust <Trond.Myklebust at netapp.com>
---

 fs/nfs/nfs4proc.c |    9 +++++++++
 fs/nfs/nfs4xdr.c  |    3 +--
 fs/nfs/pnfs.c     |    3 ---
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7c3bc50..2f8594d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5140,6 +5140,8 @@ static void nfs4_pnfs_layoutget_release(void *calldata)
 
 	dprintk("--> %s\n", __func__);
 	pnfs_layout_release(lo);
+	if (lgp->res.layout.buf != NULL)
+		free_page((unsigned long) lgp->res.layout.buf);
 	kfree(calldata);
 	dprintk("<-- %s\n", __func__);
 }
@@ -5171,6 +5173,13 @@ static int nfs4_proc_pnfs_layoutget(struct nfs4_pnfs_layoutget *lgp)
 
 	dprintk("--> %s\n", __func__);
 
+	lgp->res.layout.buf = (void *)__get_free_page(GFP_NOFS);
+	if (lgp->res.layout.buf == NULL) {
+		nfs4_pnfs_layoutget_release(lgp);
+		status = -ENOMEM;
+		goto out;
+	}
+
 	task = rpc_run_task(&task_setup_data);
 	if (IS_ERR(task)) {
 		status = PTR_ERR(task);
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 884c42d..50478b5 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -5562,8 +5562,7 @@ static int decode_pnfs_layoutget(struct xdr_stream *xdr, struct rpc_rqst *req,
 		res->type,
 		res->layout.len);
 
-	res->layout.buf = kmalloc(res->layout.len, GFP_KERNEL);
-	if (!res->layout.buf)
+	if (res->layout.len > PAGE_SIZE)
 		return -ENOMEM;
 	READ_BUF(res->layout.len);
 	COPYMEM(res->layout.buf, res->layout.len);
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index afb3d71..79208e6 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1049,9 +1049,6 @@ get_out:
 	/* Done processing layoutget. Set the layout stateid */
 	pnfs_set_layout_stateid(lo, &res->stateid);
 
-	/* res->layout.buf kalloc'ed by the xdr decoder? */
-	kfree(res->layout.buf);
-
 	dprintk("%s end (err:%d) state 0x%lx lseg %p\n",
 		__func__, lgp->status, nfsi->pnfs_layout_state, lseg);
 	if (lgp->lsegpp)


-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust at netapp.com
www.netapp.com