Re: [PATCH v2 1/1] NFS: Convert from readpages() to readahead()

[Trond Myklebust <trondmy@hammerspace.com>]

On Sat, 2021-10-09 at 08:36 -0400, Dave Wysochanski wrote:
> Convert to the new VM readahead() API which is the preferred API
> to read multiple pages, and rename the NFSIOS_* counters and the
> tracepoint as needed.
> 
> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> ---
>  fs/nfs/file.c              |  2 +-
>  fs/nfs/read.c              | 18 +++++++++++++-----
>  include/linux/nfs_fs.h     |  3 +--
>  include/linux/nfs_iostat.h |  6 +++---
>  4 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 209dac208477..cc76d17fa97f 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -519,7 +519,7 @@ static void nfs_swap_deactivate(struct file
> *file)
>  
>  const struct address_space_operations nfs_file_aops = {
>         .readpage = nfs_readpage,
> -       .readpages = nfs_readpages,
> +       .readahead = nfs_readahead,
>         .set_page_dirty = __set_page_dirty_nobuffers,
>         .writepage = nfs_writepage,
>         .writepages = nfs_writepages,
> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> index d06b91a101d2..296ea9a9b6ce 100644
> --- a/fs/nfs/read.c
> +++ b/fs/nfs/read.c
> @@ -397,15 +397,19 @@ int nfs_readpage(struct file *file, struct page
> *page)
>         return ret;
>  }
>  
> -int nfs_readpages(struct file *file, struct address_space *mapping,
> -               struct list_head *pages, unsigned nr_pages)
> +void nfs_readahead(struct readahead_control *ractl)
>  {
> +       struct file *file = ractl->file;
> +       struct address_space *mapping = ractl->mapping;
> +       struct page *page;
> +       unsigned int nr_pages = readahead_count(ractl);
> +
>         struct nfs_readdesc desc;
>         struct inode *inode = mapping->host;
>         int ret;
>  
>         trace_nfs_aop_readahead(inode, nr_pages);
> -       nfs_inc_stats(inode, NFSIOS_VFSREADPAGES);
> +       nfs_inc_stats(inode, NFSIOS_VFSREADAHEAD);
>  
>         ret = -ESTALE;
>         if (NFS_STALE(inode))
> @@ -422,14 +426,18 @@ int nfs_readpages(struct file *file, struct
> address_space *mapping,


This function fails to compile due to the call to
nfs_readpages_from_fscache() taking a 'pages' argument.

>         nfs_pageio_init_read(&desc.pgio, inode, false,
>                              &nfs_async_read_completion_ops);
>  
> -       ret = read_cache_pages(mapping, pages, readpage_async_filler,
> &desc);
> +       ret = 0;
> +       while (!ret && (page = readahead_page(ractl))) {
> +               prefetchw(&page->flags);
> +               ret = readpage_async_filler(&desc, page);
> +               put_page(page);
> +       }
>  
>         nfs_pageio_complete_read(&desc.pgio);
>  
>         put_nfs_open_context(desc.ctx);
>  out:
>         trace_nfs_aop_readahead_done(inode, nr_pages, ret);
> -       return ret;
>  }
>  
>  int __init nfs_init_readpagecache(void)
> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index 140187b57db8..a5aef2cbe4ee 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -586,8 +586,7 @@ extern int nfs_access_get_cached(struct inode
> *inode, const struct cred *cred, s
>   * linux/fs/nfs/read.c
>   */
>  extern int  nfs_readpage(struct file *, struct page *);
> -extern int  nfs_readpages(struct file *, struct address_space *,
> -               struct list_head *, unsigned);
> +extern void nfs_readahead(struct readahead_control *);
>  
>  /*
>   * inline functions
> diff --git a/include/linux/nfs_iostat.h b/include/linux/nfs_iostat.h
> index 027874c36c88..418145f23700 100644
> --- a/include/linux/nfs_iostat.h
> +++ b/include/linux/nfs_iostat.h
> @@ -22,7 +22,7 @@
>  #ifndef _LINUX_NFS_IOSTAT
>  #define _LINUX_NFS_IOSTAT
>  
> -#define NFS_IOSTAT_VERS                "1.1"
> +#define NFS_IOSTAT_VERS                "1.2"
>  
>  /*
>   * NFS byte counters
> @@ -53,7 +53,7 @@
>   * NFS page counters
>   *
>   * These count the number of pages read or written via
> nfs_readpage(),
> - * nfs_readpages(), or their write equivalents.
> + * nfs_readahead(), or their write equivalents.
>   *
>   * NB: When adding new byte counters, please include the measured
>   * units in the name of each byte counter to help users of this
> @@ -98,7 +98,7 @@ enum nfs_stat_eventcounters {
>         NFSIOS_VFSACCESS,
>         NFSIOS_VFSUPDATEPAGE,
>         NFSIOS_VFSREADPAGE,
> -       NFSIOS_VFSREADPAGES,
> +       NFSIOS_VFSREADAHEAD,
>         NFSIOS_VFSWRITEPAGE,
>         NFSIOS_VFSWRITEPAGES,
>         NFSIOS_VFSGETDENTS,

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