pnfs: defer release of pages in layoutget
authorIdan Kedar <idank@tonian.com>
Thu, 2 Aug 2012 08:47:10 +0000 (11:47 +0300)
committerTrond Myklebust <Trond.Myklebust@netapp.com>
Thu, 2 Aug 2012 21:38:54 +0000 (17:38 -0400)
we have encountered a bug whereby reading a lot of files (copying
fedora's /bin) from a pNFS mount and hitting Ctrl+C in the middle caused
a general protection fault in xdr_shrink_bufhead. this function is
called when decoding the response from LAYOUTGET. the decoding is done
by a worker thread, and the caller of LAYOUTGET waits for the worker
thread to complete.

hitting Ctrl+C caused the synchronous wait to end and the next thing the
caller does is to free the pages, so when the worker thread calls
xdr_shrink_bufhead, the pages are gone. therefore, the cleanup of these
pages has been moved to nfs4_layoutget_release.

Signed-off-by: Idan Kedar <idank@tonian.com>
Signed-off-by: Benny Halevy <bhalevy@tonian.com>
Cc: stable@vger.kernel.org
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
fs/nfs/nfs4proc.c
fs/nfs/pnfs.c
fs/nfs/pnfs.h

index a99a8d9..6a78d49 100644 (file)
@@ -6223,11 +6223,58 @@ static void nfs4_layoutget_done(struct rpc_task *task, void *calldata)
        dprintk("<-- %s\n", __func__);
 }
 
+static size_t max_response_pages(struct nfs_server *server)
+{
+       u32 max_resp_sz = server->nfs_client->cl_session->fc_attrs.max_resp_sz;
+       return nfs_page_array_len(0, max_resp_sz);
+}
+
+static void nfs4_free_pages(struct page **pages, size_t size)
+{
+       int i;
+
+       if (!pages)
+               return;
+
+       for (i = 0; i < size; i++) {
+               if (!pages[i])
+                       break;
+               __free_page(pages[i]);
+       }
+       kfree(pages);
+}
+
+static struct page **nfs4_alloc_pages(size_t size, gfp_t gfp_flags)
+{
+       struct page **pages;
+       int i;
+
+       pages = kcalloc(size, sizeof(struct page *), gfp_flags);
+       if (!pages) {
+               dprintk("%s: can't alloc array of %zu pages\n", __func__, size);
+               return NULL;
+       }
+
+       for (i = 0; i < size; i++) {
+               pages[i] = alloc_page(gfp_flags);
+               if (!pages[i]) {
+                       dprintk("%s: failed to allocate page\n", __func__);
+                       nfs4_free_pages(pages, size);
+                       return NULL;
+               }
+       }
+
+       return pages;
+}
+
 static void nfs4_layoutget_release(void *calldata)
 {
        struct nfs4_layoutget *lgp = calldata;
+       struct nfs_server *server = NFS_SERVER(lgp->args.inode);
+       size_t max_pages = max_response_pages(server);
 
        dprintk("--> %s\n", __func__);
+       nfs4_free_pages(lgp->args.layout.pages, max_pages);
        put_nfs_open_context(lgp->args.ctx);
        kfree(calldata);
        dprintk("<-- %s\n", __func__);
@@ -6239,9 +6286,10 @@ static const struct rpc_call_ops nfs4_layoutget_call_ops = {
        .rpc_release = nfs4_layoutget_release,
 };
 
-int nfs4_proc_layoutget(struct nfs4_layoutget *lgp)
+int nfs4_proc_layoutget(struct nfs4_layoutget *lgp, gfp_t gfp_flags)
 {
        struct nfs_server *server = NFS_SERVER(lgp->args.inode);
+       size_t max_pages = max_response_pages(server);
        struct rpc_task *task;
        struct rpc_message msg = {
                .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_LAYOUTGET],
@@ -6259,6 +6307,13 @@ int nfs4_proc_layoutget(struct nfs4_layoutget *lgp)
 
        dprintk("--> %s\n", __func__);
 
+       lgp->args.layout.pages = nfs4_alloc_pages(max_pages, gfp_flags);
+       if (!lgp->args.layout.pages) {
+               nfs4_layoutget_release(lgp);
+               return -ENOMEM;
+       }
+       lgp->args.layout.pglen = max_pages * PAGE_SIZE;
+
        lgp->res.layoutp = &lgp->args.layout;
        lgp->res.seq_res.sr_slot = NULL;
        nfs41_init_sequence(&lgp->args.seq_args, &lgp->res.seq_res, 0);
index 76875bf..2e00fea 100644 (file)
@@ -583,9 +583,6 @@ send_layoutget(struct pnfs_layout_hdr *lo,
        struct nfs_server *server = NFS_SERVER(ino);
        struct nfs4_layoutget *lgp;
        struct pnfs_layout_segment *lseg = NULL;
-       struct page **pages = NULL;
-       int i;
-       u32 max_resp_sz, max_pages;
 
        dprintk("--> %s\n", __func__);
 
@@ -594,20 +591,6 @@ send_layoutget(struct pnfs_layout_hdr *lo,
        if (lgp == NULL)
                return NULL;
 
-       /* allocate pages for xdr post processing */
-       max_resp_sz = server->nfs_client->cl_session->fc_attrs.max_resp_sz;
-       max_pages = nfs_page_array_len(0, max_resp_sz);
-
-       pages = kcalloc(max_pages, sizeof(struct page *), gfp_flags);
-       if (!pages)
-               goto out_err_free;
-
-       for (i = 0; i < max_pages; i++) {
-               pages[i] = alloc_page(gfp_flags);
-               if (!pages[i])
-                       goto out_err_free;
-       }
-
        lgp->args.minlength = PAGE_CACHE_SIZE;
        if (lgp->args.minlength > range->length)
                lgp->args.minlength = range->length;
@@ -616,39 +599,19 @@ send_layoutget(struct pnfs_layout_hdr *lo,
        lgp->args.type = server->pnfs_curr_ld->id;
        lgp->args.inode = ino;
        lgp->args.ctx = get_nfs_open_context(ctx);
-       lgp->args.layout.pages = pages;
-       lgp->args.layout.pglen = max_pages * PAGE_SIZE;
        lgp->lsegpp = &lseg;
        lgp->gfp_flags = gfp_flags;
 
        /* Synchronously retrieve layout information from server and
         * store in lseg.
         */
-       nfs4_proc_layoutget(lgp);
+       nfs4_proc_layoutget(lgp, gfp_flags);
        if (!lseg) {
                /* remember that LAYOUTGET failed and suspend trying */
                set_bit(lo_fail_bit(range->iomode), &lo->plh_flags);
        }
 
-       /* free xdr pages */
-       for (i = 0; i < max_pages; i++)
-               __free_page(pages[i]);
-       kfree(pages);
-
        return lseg;
-
-out_err_free:
-       /* free any allocated xdr pages, lgp as it's not used */
-       if (pages) {
-               for (i = 0; i < max_pages; i++) {
-                       if (!pages[i])
-                               break;
-                       __free_page(pages[i]);
-               }
-               kfree(pages);
-       }
-       kfree(lgp);
-       return NULL;
 }
 
 /*
index 2c6c805..5ea019e 100644 (file)
@@ -172,7 +172,7 @@ extern int nfs4_proc_getdevicelist(struct nfs_server *server,
                                   struct pnfs_devicelist *devlist);
 extern int nfs4_proc_getdeviceinfo(struct nfs_server *server,
                                   struct pnfs_device *dev);
-extern int nfs4_proc_layoutget(struct nfs4_layoutget *lgp);
+extern int nfs4_proc_layoutget(struct nfs4_layoutget *lgp, gfp_t gfp_flags);
 extern int nfs4_proc_layoutreturn(struct nfs4_layoutreturn *lrp);
 
 /* pnfs.c */