diff mbox series

[11/11] NFS: Bring back nfs_dir_mapping_need_revalidate() in nfs_readdir()

Message ID 1604325011-29427-12-git-send-email-dwysocha@redhat.com (mailing list archive)
State New
Headers show
Series Add NFS readdir tracepoints and improve performance of reading directories | expand

Commit Message

David Wysochanski Nov. 2, 2020, 1:50 p.m. UTC
commit 79f687a3de9e ("NFS: Fix a performance regression in readdir")
removed nfs_dir_mapping_need_revalidate() in an attempt to fix a
performance regression, but had the effect that with NFSv3 the
pagecache would never expire while a process was reading a directory.
An earlier patch addressed this problem by allowing the pagecache
to expire but the current process to continue using the last cookie
returned by the server when searching the pagecache, rather than
always starting at 0.  Thus, bring back nfs_dir_mapping_need_revalidate()
so that nfsi->cache_validity & NFS_INO_INVALID_DATA will be seen
and nfs_revalidate_mapping() will be called with NFSv3 as well,
making it behave the same as NFSv4.

Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
 fs/nfs/dir.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Mkrtchyan, Tigran Nov. 2, 2020, 3:38 p.m. UTC | #1
Hi Dave,

----- Original Message -----
> From: "David Wysochanski" <dwysocha@redhat.com>
> To: "trondmy" <trondmy@hammerspace.com>, "Anna Schumaker" <anna.schumaker@netapp.com>
> Cc: "linux-nfs" <linux-nfs@vger.kernel.org>
> Sent: Monday, 2 November, 2020 14:50:11
> Subject: [PATCH 11/11] NFS: Bring back nfs_dir_mapping_need_revalidate() in nfs_readdir()

> commit 79f687a3de9e ("NFS: Fix a performance regression in readdir")
> removed nfs_dir_mapping_need_revalidate() in an attempt to fix a
> performance regression, but had the effect that with NFSv3 the
> pagecache would never expire while a process was reading a directory.
> An earlier patch addressed this problem by allowing the pagecache
> to expire but the current process to continue using the last cookie
> returned by the server when searching the pagecache, rather than
> always starting at 0.  Thus, bring back nfs_dir_mapping_need_revalidate()
> so that nfsi->cache_validity & NFS_INO_INVALID_DATA will be seen
> and nfs_revalidate_mapping() will be called with NFSv3 as well,
> making it behave the same as NFSv4.
> 
> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> ---
> fs/nfs/dir.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index cbd74cbdbb9f..aeb086fb3d0a 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -872,6 +872,17 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc)
> 	return status;
> }
> 
> +static bool nfs_dir_mapping_need_revalidate(struct inode *dir)
> +{
> +	struct nfs_inode *nfsi = NFS_I(dir);
> +
> +	if (nfs_attribute_cache_expired(dir))
> +		return true;
> +	if (nfsi->cache_validity & NFS_INO_INVALID_DATA)
> +		return true;
> +	return false;
> +}

Is this the same as:

static bool nfs_dir_mapping_need_revalidate(struct inode *dir)
{
        struct nfs_inode *nfsi = NFS_I(dir);

        return nfs_attribute_cache_expired(dir) || nfsi->cache_validity & NFS_INO_INVALID_DATA);
}

Tigran.

> +
> /* The file offset position represents the dirent entry number.  A
>    last cookie cache takes care of the common case of reading the
>    whole directory.
> @@ -903,7 +914,7 @@ static int nfs_readdir(struct file *file, struct dir_context
> *ctx)
> 	 * to either find the entry with the appropriate number or
> 	 * revalidate the cookie.
> 	 */
> -	if (ctx->pos == 0 || nfs_attribute_cache_expired(inode))
> +	if (ctx->pos == 0 || nfs_dir_mapping_need_revalidate(inode))
> 		res = nfs_revalidate_mapping(inode, file->f_mapping);
> 	if (res < 0)
> 		goto out;
> --
> 1.8.3.1
David Wysochanski Nov. 2, 2020, 4:16 p.m. UTC | #2
On Mon, Nov 2, 2020 at 10:48 AM Mkrtchyan, Tigran
<tigran.mkrtchyan@desy.de> wrote:
>
> Hi Dave,
>
> ----- Original Message -----
> > From: "David Wysochanski" <dwysocha@redhat.com>
> > To: "trondmy" <trondmy@hammerspace.com>, "Anna Schumaker" <anna.schumaker@netapp.com>
> > Cc: "linux-nfs" <linux-nfs@vger.kernel.org>
> > Sent: Monday, 2 November, 2020 14:50:11
> > Subject: [PATCH 11/11] NFS: Bring back nfs_dir_mapping_need_revalidate() in nfs_readdir()
>
> > commit 79f687a3de9e ("NFS: Fix a performance regression in readdir")
> > removed nfs_dir_mapping_need_revalidate() in an attempt to fix a
> > performance regression, but had the effect that with NFSv3 the
> > pagecache would never expire while a process was reading a directory.
> > An earlier patch addressed this problem by allowing the pagecache
> > to expire but the current process to continue using the last cookie
> > returned by the server when searching the pagecache, rather than
> > always starting at 0.  Thus, bring back nfs_dir_mapping_need_revalidate()
> > so that nfsi->cache_validity & NFS_INO_INVALID_DATA will be seen
> > and nfs_revalidate_mapping() will be called with NFSv3 as well,
> > making it behave the same as NFSv4.
> >
> > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> > ---
> > fs/nfs/dir.c | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index cbd74cbdbb9f..aeb086fb3d0a 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -872,6 +872,17 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc)
> >       return status;
> > }
> >
> > +static bool nfs_dir_mapping_need_revalidate(struct inode *dir)
> > +{
> > +     struct nfs_inode *nfsi = NFS_I(dir);
> > +
> > +     if (nfs_attribute_cache_expired(dir))
> > +             return true;
> > +     if (nfsi->cache_validity & NFS_INO_INVALID_DATA)
> > +             return true;
> > +     return false;
> > +}
>
> Is this the same as:
>
> static bool nfs_dir_mapping_need_revalidate(struct inode *dir)
> {
>         struct nfs_inode *nfsi = NFS_I(dir);
>
>         return nfs_attribute_cache_expired(dir) || nfsi->cache_validity & NFS_INO_INVALID_DATA);
> }
>
Yes that's a nice simplification!

> Tigran.
>
> > +
> > /* The file offset position represents the dirent entry number.  A
> >    last cookie cache takes care of the common case of reading the
> >    whole directory.
> > @@ -903,7 +914,7 @@ static int nfs_readdir(struct file *file, struct dir_context
> > *ctx)
> >        * to either find the entry with the appropriate number or
> >        * revalidate the cookie.
> >        */
> > -     if (ctx->pos == 0 || nfs_attribute_cache_expired(inode))
> > +     if (ctx->pos == 0 || nfs_dir_mapping_need_revalidate(inode))
> >               res = nfs_revalidate_mapping(inode, file->f_mapping);
> >       if (res < 0)
> >               goto out;
> > --
> > 1.8.3.1
>
diff mbox series

Patch

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index cbd74cbdbb9f..aeb086fb3d0a 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -872,6 +872,17 @@  int uncached_readdir(nfs_readdir_descriptor_t *desc)
 	return status;
 }
 
+static bool nfs_dir_mapping_need_revalidate(struct inode *dir)
+{
+	struct nfs_inode *nfsi = NFS_I(dir);
+
+	if (nfs_attribute_cache_expired(dir))
+		return true;
+	if (nfsi->cache_validity & NFS_INO_INVALID_DATA)
+		return true;
+	return false;
+}
+
 /* The file offset position represents the dirent entry number.  A
    last cookie cache takes care of the common case of reading the
    whole directory.
@@ -903,7 +914,7 @@  static int nfs_readdir(struct file *file, struct dir_context *ctx)
 	 * to either find the entry with the appropriate number or
 	 * revalidate the cookie.
 	 */
-	if (ctx->pos == 0 || nfs_attribute_cache_expired(inode))
+	if (ctx->pos == 0 || nfs_dir_mapping_need_revalidate(inode))
 		res = nfs_revalidate_mapping(inode, file->f_mapping);
 	if (res < 0)
 		goto out;