Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1604325011-29427-1-git-send-email-dwysocha@redhat.com>
 <1604325011-29427-12-git-send-email-dwysocha@redhat.com> <1976524196.5308081.1604331522179.JavaMail.zimbra@desy.de>
In-Reply-To: <1976524196.5308081.1604331522179.JavaMail.zimbra@desy.de>
From:   David Wysochanski <dwysocha@redhat.com>
Date:   Mon, 2 Nov 2020 11:16:59 -0500
Message-ID: <CALF+zOkEuGEj=4Rre_ofyxCZrtj-snFshYxZX1VfURdXOcALbA@mail.gmail.com>
Subject: Re: [PATCH 11/11] NFS: Bring back nfs_dir_mapping_need_revalidate()
 in nfs_readdir()
To:     "Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de>
Cc:     trondmy <trondmy@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        linux-nfs <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>