On Tue, 2022-08-30 at 20:50 -0400, Dave Wysochanski wrote: > Rename readpage_async_filler to nfs_pageio_add_page to > better reflect what this function does (add a page to > the nfs_pageio_descriptor), and simplify arguments to > this function by removing struct nfs_readdesc. > > Signed-off-by: Dave Wysochanski <dwysocha@xxxxxxxxxx> > --- > fs/nfs/read.c | 60 +++++++++++++++++++++++++-------------------------- > 1 file changed, 30 insertions(+), 30 deletions(-) > > diff --git a/fs/nfs/read.c b/fs/nfs/read.c > index 8ae2c8d1219d..525e82ea9a9e 100644 > --- a/fs/nfs/read.c > +++ b/fs/nfs/read.c > @@ -127,11 +127,6 @@ static void nfs_readpage_release(struct nfs_page *req, int error) > nfs_release_request(req); > } > > -struct nfs_readdesc { > - struct nfs_pageio_descriptor pgio; > - struct nfs_open_context *ctx; > -}; > - > static void nfs_page_group_set_uptodate(struct nfs_page *req) > { > if (nfs_page_group_sync_on_bit(req, PG_UPTODATE)) > @@ -153,7 +148,8 @@ static void nfs_read_completion(struct nfs_pgio_header *hdr) > > if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) { > /* note: regions of the page not covered by a > - * request are zeroed in readpage_async_filler */ > + * request are zeroed in nfs_pageio_add_page > + */ > if (bytes > hdr->good_bytes) { > /* nothing in this request was good, so zero > * the full extent of the request */ > @@ -281,8 +277,10 @@ static void nfs_readpage_result(struct rpc_task *task, > nfs_readpage_retry(task, hdr); > } > > -static int > -readpage_async_filler(struct nfs_readdesc *desc, struct page *page) > +int > +nfs_pageio_add_page(struct nfs_pageio_descriptor *pgio, > + struct nfs_open_context *ctx, > + struct page *page) > { > struct inode *inode = page_file_mapping(page)->host; > unsigned int rsize = NFS_SERVER(inode)->rsize; > @@ -302,15 +300,15 @@ readpage_async_filler(struct nfs_readdesc *desc, struct page *page) > goto out_unlock; > } > > - new = nfs_create_request(desc->ctx, page, 0, aligned_len); > + new = nfs_create_request(ctx, page, 0, aligned_len); > if (IS_ERR(new)) > goto out_error; > > if (len < PAGE_SIZE) > zero_user_segment(page, len, PAGE_SIZE); > - if (!nfs_pageio_add_request(&desc->pgio, new)) { > + if (!nfs_pageio_add_request(pgio, new)) { > nfs_list_remove_request(new); > - error = desc->pgio.pg_error; > + error = pgio->pg_error; > nfs_readpage_release(new, error); > goto out; > } > @@ -332,7 +330,8 @@ readpage_async_filler(struct nfs_readdesc *desc, struct page *page) > int nfs_read_folio(struct file *file, struct folio *folio) > { > struct page *page = &folio->page; > - struct nfs_readdesc desc; > + struct nfs_pageio_descriptor pgio; > + struct nfs_open_context *ctx; > struct inode *inode = page_file_mapping(page)->host; > int ret; > > @@ -358,29 +357,29 @@ int nfs_read_folio(struct file *file, struct folio *folio) > > if (file == NULL) { > ret = -EBADF; > - desc.ctx = nfs_find_open_context(inode, NULL, FMODE_READ); > - if (desc.ctx == NULL) > + ctx = nfs_find_open_context(inode, NULL, FMODE_READ); > + if (ctx == NULL) > goto out_unlock; > } else > - desc.ctx = get_nfs_open_context(nfs_file_open_context(file)); > + ctx = get_nfs_open_context(nfs_file_open_context(file)); > > - xchg(&desc.ctx->error, 0); > - nfs_pageio_init_read(&desc.pgio, inode, false, > + xchg(&ctx->error, 0); > + nfs_pageio_init_read(&pgio, inode, false, > &nfs_async_read_completion_ops); > > - ret = readpage_async_filler(&desc, page); > + ret = nfs_pageio_add_page(&pgio, ctx, page); > if (ret) > goto out; > > - nfs_pageio_complete_read(&desc.pgio); > - ret = desc.pgio.pg_error < 0 ? desc.pgio.pg_error : 0; > + nfs_pageio_complete_read(&pgio); > + ret = pgio.pg_error < 0 ? pgio.pg_error : 0; > if (!ret) { > ret = wait_on_page_locked_killable(page); > if (!PageUptodate(page) && !ret) > - ret = xchg(&desc.ctx->error, 0); > + ret = xchg(&ctx->error, 0); > } > out: > - put_nfs_open_context(desc.ctx); > + put_nfs_open_context(ctx); > trace_nfs_aop_readpage_done(inode, page, ret); > return ret; > out_unlock: > @@ -391,9 +390,10 @@ int nfs_read_folio(struct file *file, struct folio *folio) > > void nfs_readahead(struct readahead_control *ractl) > { > + struct nfs_pageio_descriptor pgio; > + struct nfs_open_context *ctx; > unsigned int nr_pages = readahead_count(ractl); > struct file *file = ractl->file; > - struct nfs_readdesc desc; > struct inode *inode = ractl->mapping->host; > struct page *page; > int ret; > @@ -407,25 +407,25 @@ void nfs_readahead(struct readahead_control *ractl) > > if (file == NULL) { > ret = -EBADF; > - desc.ctx = nfs_find_open_context(inode, NULL, FMODE_READ); > - if (desc.ctx == NULL) > + ctx = nfs_find_open_context(inode, NULL, FMODE_READ); > + if (ctx == NULL) > goto out; > } else > - desc.ctx = get_nfs_open_context(nfs_file_open_context(file)); > + ctx = get_nfs_open_context(nfs_file_open_context(file)); > > - nfs_pageio_init_read(&desc.pgio, inode, false, > + nfs_pageio_init_read(&pgio, inode, false, > &nfs_async_read_completion_ops); > > while ((page = readahead_page(ractl)) != NULL) { > - ret = readpage_async_filler(&desc, page); > + ret = nfs_pageio_add_page(&pgio, ctx, page); > put_page(page); > if (ret) > break; > } > > - nfs_pageio_complete_read(&desc.pgio); > + nfs_pageio_complete_read(&pgio); > > - put_nfs_open_context(desc.ctx); > + put_nfs_open_context(ctx); > out: > trace_nfs_aop_readahead_done(inode, nr_pages, ret); > } Yeah, a special args struct only with 2 fields in it does seem sort of pointless: Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>