2015-06-18 00:02:54

by Trond Myklebust

Subject: [PATCH 1/2] pNFS: Fix a memory leak when attempted pnfs fails

pnfs_do_write() expects the call to pnfs_write_through_mds() to free the
pgio header and to release the layout segment before exiting. The problem
is that nfs_pgio_data_destroy() doesn't actually do this; it only frees
the memory allocated by nfs_generic_pgio().

Ditto for pnfs_do_read()...

Fix in both cases is to add a call to hdr->release(hdr).

Cc: [email protected]
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/pnfs.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 230606243be6..219ee6a3f1b3 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1865,6 +1865,7 @@ pnfs_write_through_mds(struct nfs_pageio_descriptor *desc,
mirror->pg_recoalesce = 1;
}
nfs_pgio_data_destroy(hdr);
+ hdr->release(hdr);
}

static enum pnfs_try_status
@@ -1979,6 +1980,7 @@ pnfs_read_through_mds(struct nfs_pageio_descriptor *desc,
mirror->pg_recoalesce = 1;
}
nfs_pgio_data_destroy(hdr);
+ hdr->release(hdr);
}

/*
--
2.4.3

2015-06-18 00:02:56

by Trond Myklebust

Subject: [PATCH 2/2] NFS: Ensure we set NFS_CONTEXT_RESEND_WRITES when requeuing writes

If a write attempt fails, and the write is queued up for resending to
the server, as opposed to being dropped, then we need to set the
appropriate flag so that nfs_file_fsync() does the right thing.

Cc: [email protected]
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/pnfs.c | 1 +
fs/nfs/write.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 219ee6a3f1b3..d47c188682b1 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1821,6 +1821,7 @@ int pnfs_write_done_resend_to_mds(struct nfs_pgio_header *hdr)
/* Resend all requests through the MDS */
nfs_pageio_init_write(&pgio, hdr->inode, FLUSH_STABLE, true,
hdr->completion_ops);
+ set_bit(NFS_CONTEXT_RESEND_WRITES, &hdr->args.context->flags);
return nfs_pageio_resend(&pgio, hdr);
}
EXPORT_SYMBOL_GPL(pnfs_write_done_resend_to_mds);
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index dfc19f1575a1..daf355642845 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1289,6 +1289,7 @@ static void nfs_initiate_write(struct nfs_pgio_header *hdr,
static void nfs_redirty_request(struct nfs_page *req)
{
nfs_mark_request_dirty(req);
+ set_bit(NFS_CONTEXT_RESEND_WRITES, &req->wb_context->flags);
nfs_unlock_request(req);
nfs_end_page_writeback(req);
nfs_release_request(req);
--
2.4.3

2015-06-18 20:35:38

by Jeff Layton

Subject: Re: [PATCH 1/2] pNFS: Fix a memory leak when attempted pnfs fails

On Wed, 17 Jun 2015 20:02:50 -0400
Trond Myklebust <[email protected]> wrote:

> pnfs_do_write() expects the call to pnfs_write_through_mds() to free the
> pgio header and to release the layout segment before exiting. The problem
> is that nfs_pgio_data_destroy() doesn't actually do this; it only frees
> the memory allocated by nfs_generic_pgio().
>
> Ditto for pnfs_do_read()...
>
> Fix in both cases is to add a call to hdr->release(hdr).
>
> Cc: [email protected]
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/pnfs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 230606243be6..219ee6a3f1b3 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1865,6 +1865,7 @@ pnfs_write_through_mds(struct nfs_pageio_descriptor *desc,
> mirror->pg_recoalesce = 1;
> }
> nfs_pgio_data_destroy(hdr);
> + hdr->release(hdr);
> }
>
> static enum pnfs_try_status
> @@ -1979,6 +1980,7 @@ pnfs_read_through_mds(struct nfs_pageio_descriptor *desc,
> mirror->pg_recoalesce = 1;
> }
> nfs_pgio_data_destroy(hdr);
> + hdr->release(hdr);
> }
>
> /*

Reviewed-by: Jeff Layton <[email protected]>

2015-06-18 20:38:31

by Jeff Layton

Subject: Re: [PATCH 2/2] NFS: Ensure we set NFS_CONTEXT_RESEND_WRITES when requeuing writes

On Wed, 17 Jun 2015 20:02:51 -0400
Trond Myklebust <[email protected]> wrote:

> If a write attempt fails, and the write is queued up for resending to
> the server, as opposed to being dropped, then we need to set the
> appropriate flag so that nfs_file_fsync() does the right thing.
>
> Cc: [email protected]
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/pnfs.c | 1 +
> fs/nfs/write.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 219ee6a3f1b3..d47c188682b1 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1821,6 +1821,7 @@ int pnfs_write_done_resend_to_mds(struct nfs_pgio_header *hdr)
> /* Resend all requests through the MDS */
> nfs_pageio_init_write(&pgio, hdr->inode, FLUSH_STABLE, true,
> hdr->completion_ops);
> + set_bit(NFS_CONTEXT_RESEND_WRITES, &hdr->args.context->flags);
> return nfs_pageio_resend(&pgio, hdr);
> }
> EXPORT_SYMBOL_GPL(pnfs_write_done_resend_to_mds);
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index dfc19f1575a1..daf355642845 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -1289,6 +1289,7 @@ static void nfs_initiate_write(struct nfs_pgio_header *hdr,
> static void nfs_redirty_request(struct nfs_page *req)
> {
> nfs_mark_request_dirty(req);
> + set_bit(NFS_CONTEXT_RESEND_WRITES, &req->wb_context->flags);
> nfs_unlock_request(req);
> nfs_end_page_writeback(req);
> nfs_release_request(req);

Looks right.

Reviewed-by: Jeff Layton <[email protected]>