* [PATCH v2 0/1] Convert nfs_readpages() to nfs_readahead()
@ 2021-10-09 12:36 Dave Wysochanski
2021-10-09 12:36 ` [PATCH v2 1/1] NFS: Convert from readpages() to readahead() Dave Wysochanski
0 siblings, 1 reply; 4+ messages in thread
From: Dave Wysochanski @ 2021-10-09 12:36 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: Matthew Wilcox, David Howells, Chuck Lever, linux-cachefs, linux-nfs
This patch converts nfs_readpages() to nfs_readahead(). It was
applied as follows:
0. Start with trond's testing branch at
0abb8895b065 NFS: Fix an Oops in pnfs_mark_request_commit()
1. Apply David Howells v3 of "fscache: Replace and remove old I/O API" [1]
2. Apply my fscache patches v2 of "Various NFS fscache cleanups" [2]
3. Apply Chucks v3 of "NFS: Replace dprintk callsites in nfs_readpage(s)"
plus one fixup (remove the "read_complete:" label in last hunk, which
conflicts with #2) [3]
4. Apply this patch
So far the existing BakeAThon tests have gone well with no oops or any
failure differences in xfstests (generic) between 5.15.0-rc4 and kernel
with #2 and #3 above. I will continue testing now with all patches as
described above (#1 - #5).
As far as I know this has been an outstanding item for the NFS client
for a while and the fscache fallback IO API clears the way for this
patch.
I also just posted a v2 of the nfs-utils patch to display a "VFS readahead"
count rather than a readpages count [4].
[1] https://marc.info/?l=linux-nfs&m=163363955619832&w=2
[2] https://marc.info/?l=linux-nfs&m=163364580324243&w=2
[3] https://marc.info/?l=linux-nfs&m=163370503223875&w=2
[4] https://marc.info/?l=linux-nfs&m=163378240328297&w=2
Dave Wysochanski (1):
NFS: Convert from readpages() to readahead()
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(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] NFS: Convert from readpages() to readahead()
2021-10-09 12:36 [PATCH v2 0/1] Convert nfs_readpages() to nfs_readahead() Dave Wysochanski
@ 2021-10-09 12:36 ` Dave Wysochanski
2021-10-20 19:26 ` Trond Myklebust
0 siblings, 1 reply; 4+ messages in thread
From: Dave Wysochanski @ 2021-10-09 12:36 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: Matthew Wilcox, David Howells, Chuck Lever, linux-cachefs, linux-nfs
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,
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,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] NFS: Convert from readpages() to readahead()
2021-10-09 12:36 ` [PATCH v2 1/1] NFS: Convert from readpages() to readahead() Dave Wysochanski
@ 2021-10-20 19:26 ` Trond Myklebust
2021-10-20 19:53 ` David Wysochanski
0 siblings, 1 reply; 4+ messages in thread
From: Trond Myklebust @ 2021-10-20 19:26 UTC (permalink / raw)
To: anna.schumaker, dwysocha
Cc: linux-cachefs, linux-nfs, willy, dhowells, chuck.lever
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] NFS: Convert from readpages() to readahead()
2021-10-20 19:26 ` Trond Myklebust
@ 2021-10-20 19:53 ` David Wysochanski
0 siblings, 0 replies; 4+ messages in thread
From: David Wysochanski @ 2021-10-20 19:53 UTC (permalink / raw)
To: Trond Myklebust
Cc: anna.schumaker, linux-cachefs, linux-nfs, willy, dhowells, chuck.lever
On Wed, Oct 20, 2021 at 3:27 PM Trond Myklebust <trondmy@hammerspace.com> wrote:
>
> 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.
>
Sorry about the confusion. See the "PATCH 0/1" description [1].
This patch as posted assumes dhowells "fallback API" series.
Are you ok with that series, or do you still have concerns?
I am not sure if I can redo this patch without that series but I can
try if you're opposed to the fallback API series or see problems
such as merging conflicts or want this patch only for some reason.
Let me know what you want and I'll try to make it happen.
[1] https://marc.info/?l=linux-nfs&m=163378294028491&w=2
> > 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
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-20 19:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-09 12:36 [PATCH v2 0/1] Convert nfs_readpages() to nfs_readahead() Dave Wysochanski
2021-10-09 12:36 ` [PATCH v2 1/1] NFS: Convert from readpages() to readahead() Dave Wysochanski
2021-10-20 19:26 ` Trond Myklebust
2021-10-20 19:53 ` David Wysochanski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.