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 <dwyso...@redhat.com>
> ---
>  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 <jlay...@kernel.org>

--
Linux-cachefs mailing list
Linux-cachefs@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-cachefs

Reply via email to