* [PATCH 0/1] Convert nfs_readpages() to nfs_readahead() @ 2021-10-07 23:32 Dave Wysochanski 2021-10-07 23:32 ` [PATCH 1/1] NFS: Convert from readpages() to readahead() Dave Wysochanski 0 siblings, 1 reply; 7+ messages in thread From: Dave Wysochanski @ 2021-10-07 23:32 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 on top of the set I just posted [1] plus Chuck's v1 patch to convert dfprintks to tracepoints [2]. It is being tested on top of these patches at BakeAThon against various servers, cthon, xfstests, and my fscache unit tests. Note that Chuck may post a v3 of his patch and this would need rebased on that patch. Note that a nfs-utils patch will need to be done to properly account for the changed NFSIOS_* counters. I would consider this a "nice to have" for this cycle but I wanted to get it out there sooner than later. 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. [1] https://marc.info/?l=linux-nfs&m=163364580324243&w=2 [2] https://marc.info/?l=linux-nfs&m=163345402812426&w=2 Dave Wysochanski (1): NFS: Convert from readpages() to readahead() fs/nfs/file.c | 2 +- fs/nfs/nfstrace.h | 2 +- fs/nfs/read.c | 21 +++++++++++++++------ include/linux/nfs_fs.h | 3 +-- include/linux/nfs_iostat.h | 6 +++--- 5 files changed, 21 insertions(+), 13 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/1] NFS: Convert from readpages() to readahead() 2021-10-07 23:32 [PATCH 0/1] Convert nfs_readpages() to nfs_readahead() Dave Wysochanski @ 2021-10-07 23:32 ` Dave Wysochanski 2021-10-08 14:09 ` Chuck Lever III 0 siblings, 1 reply; 7+ messages in thread From: Dave Wysochanski @ 2021-10-07 23:32 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/nfstrace.h | 2 +- fs/nfs/read.c | 21 +++++++++++++++------ include/linux/nfs_fs.h | 3 +-- include/linux/nfs_iostat.h | 6 +++--- 5 files changed, 21 insertions(+), 13 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/nfstrace.h b/fs/nfs/nfstrace.h index 78b0f649dd09..d2b2080765a6 100644 --- a/fs/nfs/nfstrace.h +++ b/fs/nfs/nfstrace.h @@ -915,7 +915,7 @@ ) ); -TRACE_EVENT(nfs_aops_readpages, +TRACE_EVENT(nfs_aops_readahead, TP_PROTO( const struct inode *inode, unsigned int nr_pages diff --git a/fs/nfs/read.c b/fs/nfs/read.c index 927504605e0f..5c2aab47cf1d 100644 --- a/fs/nfs/read.c +++ b/fs/nfs/read.c @@ -395,15 +395,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_aops_readpages(inode, nr_pages); - nfs_inc_stats(inode, NFSIOS_VFSREADPAGES); + trace_nfs_aops_readahead(inode, nr_pages); + nfs_inc_stats(inode, NFSIOS_VFSREADAHEAD); ret = -ESTALE; if (NFS_STALE(inode)) @@ -420,13 +424,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: - return ret; + return; } int __init nfs_init_readpagecache(void) diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index b9a8b925db43..6cbe3f2c5669 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -580,8 +580,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] 7+ messages in thread
* Re: [PATCH 1/1] NFS: Convert from readpages() to readahead() 2021-10-07 23:32 ` [PATCH 1/1] NFS: Convert from readpages() to readahead() Dave Wysochanski @ 2021-10-08 14:09 ` Chuck Lever III 2021-10-09 12:35 ` David Wysochanski 2021-10-15 7:42 ` Trond Myklebust 0 siblings, 2 replies; 7+ messages in thread From: Chuck Lever III @ 2021-10-08 14:09 UTC (permalink / raw) To: Dave Wysochanski Cc: Trond Myklebust, Anna Schumaker, Matthew Wilcox, David Howells, linux-cachefs@redhat.com, Linux NFS Mailing List > On Oct 7, 2021, at 7:32 PM, Dave Wysochanski <dwysocha@redhat.com> 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/nfstrace.h | 2 +- > fs/nfs/read.c | 21 +++++++++++++++------ > include/linux/nfs_fs.h | 3 +-- > include/linux/nfs_iostat.h | 6 +++--- > 5 files changed, 21 insertions(+), 13 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/nfstrace.h b/fs/nfs/nfstrace.h > index 78b0f649dd09..d2b2080765a6 100644 > --- a/fs/nfs/nfstrace.h > +++ b/fs/nfs/nfstrace.h > @@ -915,7 +915,7 @@ > ) > ); > > -TRACE_EVENT(nfs_aops_readpages, > +TRACE_EVENT(nfs_aops_readahead, In v2 and v3 of my patch, this tracepoint has already been renamed to nfs_aop_readahead. > TP_PROTO( > const struct inode *inode, > unsigned int nr_pages > diff --git a/fs/nfs/read.c b/fs/nfs/read.c > index 927504605e0f..5c2aab47cf1d 100644 > --- a/fs/nfs/read.c > +++ b/fs/nfs/read.c > @@ -395,15 +395,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_aops_readpages(inode, nr_pages); > - nfs_inc_stats(inode, NFSIOS_VFSREADPAGES); > + trace_nfs_aops_readahead(inode, nr_pages); > + nfs_inc_stats(inode, NFSIOS_VFSREADAHEAD); > > ret = -ESTALE; > if (NFS_STALE(inode)) > @@ -420,13 +424,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: > - return ret; > + return; > } > > int __init nfs_init_readpagecache(void) > diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h > index b9a8b925db43..6cbe3f2c5669 100644 > --- a/include/linux/nfs_fs.h > +++ b/include/linux/nfs_fs.h > @@ -580,8 +580,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, I'm wondering if you should add NFSIOS_VFSREADAHEAD but leave NFSIOS_VFSREADPAGES? I don't remember exactly how the mountstats API versioning is supposed to work. > NFSIOS_VFSWRITEPAGE, > NFSIOS_VFSWRITEPAGES, > NFSIOS_VFSGETDENTS, > -- > 1.8.3.1 > -- Chuck Lever ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] NFS: Convert from readpages() to readahead() 2021-10-08 14:09 ` Chuck Lever III @ 2021-10-09 12:35 ` David Wysochanski 2021-10-09 13:42 ` David Wysochanski 2021-10-15 7:42 ` Trond Myklebust 1 sibling, 1 reply; 7+ messages in thread From: David Wysochanski @ 2021-10-09 12:35 UTC (permalink / raw) To: Chuck Lever III Cc: Trond Myklebust, Anna Schumaker, Matthew Wilcox, David Howells, linux-cachefs@redhat.com, Linux NFS Mailing List On Fri, Oct 8, 2021 at 10:10 AM Chuck Lever III <chuck.lever@oracle.com> wrote: > > > > > On Oct 7, 2021, at 7:32 PM, Dave Wysochanski <dwysocha@redhat.com> 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/nfstrace.h | 2 +- > > fs/nfs/read.c | 21 +++++++++++++++------ > > include/linux/nfs_fs.h | 3 +-- > > include/linux/nfs_iostat.h | 6 +++--- > > 5 files changed, 21 insertions(+), 13 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/nfstrace.h b/fs/nfs/nfstrace.h > > index 78b0f649dd09..d2b2080765a6 100644 > > --- a/fs/nfs/nfstrace.h > > +++ b/fs/nfs/nfstrace.h > > @@ -915,7 +915,7 @@ > > ) > > ); > > > > -TRACE_EVENT(nfs_aops_readpages, > > +TRACE_EVENT(nfs_aops_readahead, > > In v2 and v3 of my patch, this tracepoint has already been > renamed to nfs_aop_readahead. > Ack. > > > TP_PROTO( > > const struct inode *inode, > > unsigned int nr_pages > > diff --git a/fs/nfs/read.c b/fs/nfs/read.c > > index 927504605e0f..5c2aab47cf1d 100644 > > --- a/fs/nfs/read.c > > +++ b/fs/nfs/read.c > > @@ -395,15 +395,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_aops_readpages(inode, nr_pages); > > - nfs_inc_stats(inode, NFSIOS_VFSREADPAGES); > > + trace_nfs_aops_readahead(inode, nr_pages); > > + nfs_inc_stats(inode, NFSIOS_VFSREADAHEAD); > > > > ret = -ESTALE; > > if (NFS_STALE(inode)) > > @@ -420,13 +424,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: > > - return ret; > > + return; > > } > > > > int __init nfs_init_readpagecache(void) > > diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h > > index b9a8b925db43..6cbe3f2c5669 100644 > > --- a/include/linux/nfs_fs.h > > +++ b/include/linux/nfs_fs.h > > @@ -580,8 +580,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, > > I'm wondering if you should add NFSIOS_VFSREADAHEAD > but leave NFSIOS_VFSREADPAGES? I don't remember exactly > how the mountstats API versioning is supposed to work. > I don't think we need to keep this but correct me if I'm wrong. That would mean we would have a 0 count for later kernels and I'm not sure about that given the similarities. You can see my nfs-utils patch for the attempt to use the versioning in nfs-iostat. > > > NFSIOS_VFSWRITEPAGE, > > NFSIOS_VFSWRITEPAGES, > > NFSIOS_VFSGETDENTS, > > -- > > 1.8.3.1 > > > > -- > Chuck Lever > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] NFS: Convert from readpages() to readahead() 2021-10-09 12:35 ` David Wysochanski @ 2021-10-09 13:42 ` David Wysochanski 0 siblings, 0 replies; 7+ messages in thread From: David Wysochanski @ 2021-10-09 13:42 UTC (permalink / raw) To: Chuck Lever III Cc: Trond Myklebust, Anna Schumaker, Matthew Wilcox, David Howells, linux-cachefs@redhat.com, Linux NFS Mailing List On Sat, Oct 9, 2021 at 8:35 AM David Wysochanski <dwysocha@redhat.com> wrote: > > On Fri, Oct 8, 2021 at 10:10 AM Chuck Lever III <chuck.lever@oracle.com> wrote: > > > > > > > > > On Oct 7, 2021, at 7:32 PM, Dave Wysochanski <dwysocha@redhat.com> 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/nfstrace.h | 2 +- > > > fs/nfs/read.c | 21 +++++++++++++++------ > > > include/linux/nfs_fs.h | 3 +-- > > > include/linux/nfs_iostat.h | 6 +++--- > > > 5 files changed, 21 insertions(+), 13 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/nfstrace.h b/fs/nfs/nfstrace.h > > > index 78b0f649dd09..d2b2080765a6 100644 > > > --- a/fs/nfs/nfstrace.h > > > +++ b/fs/nfs/nfstrace.h > > > @@ -915,7 +915,7 @@ > > > ) > > > ); > > > > > > -TRACE_EVENT(nfs_aops_readpages, > > > +TRACE_EVENT(nfs_aops_readahead, > > > > In v2 and v3 of my patch, this tracepoint has already been > > renamed to nfs_aop_readahead. > > > > Ack. > > > > > > TP_PROTO( > > > const struct inode *inode, > > > unsigned int nr_pages > > > diff --git a/fs/nfs/read.c b/fs/nfs/read.c > > > index 927504605e0f..5c2aab47cf1d 100644 > > > --- a/fs/nfs/read.c > > > +++ b/fs/nfs/read.c > > > @@ -395,15 +395,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_aops_readpages(inode, nr_pages); > > > - nfs_inc_stats(inode, NFSIOS_VFSREADPAGES); > > > + trace_nfs_aops_readahead(inode, nr_pages); > > > + nfs_inc_stats(inode, NFSIOS_VFSREADAHEAD); > > > > > > ret = -ESTALE; > > > if (NFS_STALE(inode)) > > > @@ -420,13 +424,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: > > > - return ret; > > > + return; > > > } > > > > > > int __init nfs_init_readpagecache(void) > > > diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h > > > index b9a8b925db43..6cbe3f2c5669 100644 > > > --- a/include/linux/nfs_fs.h > > > +++ b/include/linux/nfs_fs.h > > > @@ -580,8 +580,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, > > > > I'm wondering if you should add NFSIOS_VFSREADAHEAD > > but leave NFSIOS_VFSREADPAGES? I don't remember exactly > > how the mountstats API versioning is supposed to work. > > > > I don't think we need to keep this but correct me if I'm wrong. > That would mean we would have a 0 count for later kernels and I'm not > sure about that given the similarities. It may matter in the case of "old nfs-utils" and newer kernel. Do we want nfs-utils to display 0 for VFSREADPAGES count in that case? Or would users be more confused by the fact this value is always 0 and there's no notion of "VFSREADAHEAD" counts in older nfs-utils? Would users be more surprised by 0 here or the fact there's a more subtle change in the interface? I would say the permanent 0 value would be most surprising to users so I'd shy away from it. > You can see my nfs-utils patch for the attempt to use the versioning > in nfs-iostat. > > > > > > NFSIOS_VFSWRITEPAGE, > > > NFSIOS_VFSWRITEPAGES, > > > NFSIOS_VFSGETDENTS, > > > -- > > > 1.8.3.1 > > > > > > > -- > > Chuck Lever > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] NFS: Convert from readpages() to readahead() 2021-10-08 14:09 ` Chuck Lever III 2021-10-09 12:35 ` David Wysochanski @ 2021-10-15 7:42 ` Trond Myklebust 2021-10-15 20:18 ` Chuck Lever III 1 sibling, 1 reply; 7+ messages in thread From: Trond Myklebust @ 2021-10-15 7:42 UTC (permalink / raw) To: chuck.lever@oracle.com, dwysocha@redhat.com Cc: linux-cachefs@redhat.com, linux-nfs@vger.kernel.org, willy@infradead.org, anna.schumaker@netapp.com, dhowells@redhat.com On Fri, 2021-10-08 at 14:09 +0000, Chuck Lever III wrote: > > > > On Oct 7, 2021, at 7:32 PM, Dave Wysochanski <dwysocha@redhat.com> > > 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/nfstrace.h | 2 +- > > fs/nfs/read.c | 21 +++++++++++++++------ > > include/linux/nfs_fs.h | 3 +-- > > include/linux/nfs_iostat.h | 6 +++--- > > 5 files changed, 21 insertions(+), 13 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/nfstrace.h b/fs/nfs/nfstrace.h > > index 78b0f649dd09..d2b2080765a6 100644 > > --- a/fs/nfs/nfstrace.h > > +++ b/fs/nfs/nfstrace.h > > @@ -915,7 +915,7 @@ > > ) > > ); > > > > -TRACE_EVENT(nfs_aops_readpages, > > +TRACE_EVENT(nfs_aops_readahead, > > In v2 and v3 of my patch, this tracepoint has already been > renamed to nfs_aop_readahead. Does that mean you're going to resend, Chuck? All I have is your v1.. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] NFS: Convert from readpages() to readahead() 2021-10-15 7:42 ` Trond Myklebust @ 2021-10-15 20:18 ` Chuck Lever III 0 siblings, 0 replies; 7+ messages in thread From: Chuck Lever III @ 2021-10-15 20:18 UTC (permalink / raw) To: Trond Myklebust Cc: dwysocha@redhat.com, linux-cachefs@redhat.com, linux-nfs@vger.kernel.org, willy@infradead.org, anna.schumaker@netapp.com, dhowells@redhat.com > On Oct 15, 2021, at 3:42 AM, Trond Myklebust <trondmy@hammerspace.com> wrote: > > On Fri, 2021-10-08 at 14:09 +0000, Chuck Lever III wrote: >> >> >>> On Oct 7, 2021, at 7:32 PM, Dave Wysochanski <dwysocha@redhat.com> >>> 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/nfstrace.h | 2 +- >>> fs/nfs/read.c | 21 +++++++++++++++------ >>> include/linux/nfs_fs.h | 3 +-- >>> include/linux/nfs_iostat.h | 6 +++--- >>> 5 files changed, 21 insertions(+), 13 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/nfstrace.h b/fs/nfs/nfstrace.h >>> index 78b0f649dd09..d2b2080765a6 100644 >>> --- a/fs/nfs/nfstrace.h >>> +++ b/fs/nfs/nfstrace.h >>> @@ -915,7 +915,7 @@ >>> ) >>> ); >>> >>> -TRACE_EVENT(nfs_aops_readpages, >>> +TRACE_EVENT(nfs_aops_readahead, >> >> In v2 and v3 of my patch, this tracepoint has already been >> renamed to nfs_aop_readahead. > > Does that mean you're going to resend, Chuck? I re-sent this one patch a few days ago, but I can re-send the whole series again to be sure everyone has the latest. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-15 20:19 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-10-07 23:32 [PATCH 0/1] Convert nfs_readpages() to nfs_readahead() Dave Wysochanski 2021-10-07 23:32 ` [PATCH 1/1] NFS: Convert from readpages() to readahead() Dave Wysochanski 2021-10-08 14:09 ` Chuck Lever III 2021-10-09 12:35 ` David Wysochanski 2021-10-09 13:42 ` David Wysochanski 2021-10-15 7:42 ` Trond Myklebust 2021-10-15 20:18 ` Chuck Lever III
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).