Re: [PATCH v9 1/5] NFS: Rename readpage_async_filler to nfs_pageio_add_page

[Trond Myklebust <trondmy@kernel.org>]

On Fri, 2022-10-28 at 06:32 -0400, David Wysochanski wrote:
> On Thu, Oct 27, 2022 at 2:07 PM Trond Myklebust <trondmy@kernel.org>
> wrote:
> > 
> > On Mon, 2022-10-17 at 06:52 -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@redhat.com>
> > > Reviewed-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  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)
> > 
> > If we're going to rename this function, then let's not give it a
> > name
> > that suggests it belongs in pagelist.c. It's not a generic helper
> > function, but is still very much specific to the pagecache read
> > functionality.
> > 
> 
> How about nfs_read_add_page()?
> 
> 

That's better. 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com