Re: [PATCH 2/4] NFS: Ensure nfs_readpage returns promptly when internal error occurs
From: Trond Myklebust <hidden>
Date: 2021-06-28 19:17:49
On Mon, 2021-06-28 at 13:39 -0400, Dave Wysochanski wrote:
quoted hunk ↗ jump to hunk
A previous refactoring of nfs_readpage() might end up calling wait_on_page_locked_killable() even if readpage_async_filler() failed with an internal error and pg_error was non-zero (for example, if nfs_create_request() failed). In the case of an internal error, skip over wait_on_page_locked_killable() as this is only needed when the read is sent and an error occurs during completion handling. Signed-off-by: Dave Wysochanski <redacted> --- fs/nfs/read.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)diff --git a/fs/nfs/read.c b/fs/nfs/read.c index 684a730f6670..b0680351df23 100644 --- a/fs/nfs/read.c +++ b/fs/nfs/read.c@@ -74,7 +74,7 @@ void nfs_pageio_init_read(structnfs_pageio_descriptor *pgio, } EXPORT_SYMBOL_GPL(nfs_pageio_init_read); -static void nfs_pageio_complete_read(struct nfs_pageio_descriptor *pgio) +static int nfs_pageio_complete_read(struct nfs_pageio_descriptor *pgio) { struct nfs_pgio_mirror *pgm; unsigned long npages;@@ -88,6 +88,8 @@ static void nfs_pageio_complete_read(structnfs_pageio_descriptor *pgio) NFS_I(pgio->pg_inode)->read_io += pgm->pg_bytes_written; npages = (pgm->pg_bytes_written + PAGE_SIZE - 1) >> PAGE_SHIFT; nfs_add_stats(pgio->pg_inode, NFSIOS_READPAGES, npages); + + return pgio->pg_error < 0 ? pgio->pg_error : 0; }@@ -373,16 +375,17 @@ int nfs_readpage(struct file *file, struct page*page) &nfs_async_read_completion_ops); ret = readpage_async_filler(&desc, page); + if (ret) + goto out; - if (!ret)
Can't this patch basically be reduced to the above 2 changes? The rest appears just to be shifting code around. I'm seeing nothing in the remaining patches that actually depends on nfs_pageio_complete_read() returning a value.
- nfs_pageio_complete_read(&desc.pgio);
+ ret = nfs_pageio_complete_read(&desc.pgio);
+ if (ret)
+ goto out;
+
+ ret = wait_on_page_locked_killable(page);
+ if (!PageUptodate(page) && !ret)
+ ret = xchg(&desc.ctx->error, 0);
- ret = desc.pgio.pg_error < 0 ? desc.pgio.pg_error : 0;
- if (!ret) {
- ret = wait_on_page_locked_killable(page);
- if (!PageUptodate(page) && !ret)
- ret = xchg(&desc.ctx->error, 0);
- }
out:
put_nfs_open_context(desc.ctx);
return ret;-- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com