On Wed, 2021-03-24 at 17:23 -0400, J. Bruce Fields wrote:
> On Wed, Mar 24, 2021 at 03:53:53PM -0400, trondmy@xxxxxxxxxx wrote:
> > From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> >
> > The change attribute is always set by all NFS client versions so
> > get rid
> > of the open-coded version.
>
> Thanks!
>
> I'm unclear whether there's a user-visible bug here or whether it's
> mainly cleanup?
>
It is mainly about ensuring that revalidation is done correctly. It is
a performance issue, but is also about ensuring that we all feed into
the same validity checking code.
> --b.
>
> > Fixes: 3cc55f4434b4 ("nfs: use change attribute for NFS re-
> > exports")
> > Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> > ---
> > fs/nfs/export.c | 15 ++++-----------
> > 1 file changed, 4 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/nfs/export.c b/fs/nfs/export.c
> > index f2b34cfe286c..b347e3ce0cc8 100644
> > --- a/fs/nfs/export.c
> > +++ b/fs/nfs/export.c
> > @@ -171,17 +171,10 @@ static u64 nfs_fetch_iversion(struct inode
> > *inode)
> > {
> > struct nfs_server *server = NFS_SERVER(inode);
> >
> > - /* Is this the right call?: */
> > - nfs_revalidate_inode(server, inode);
> > - /*
> > - * Also, note we're ignoring any returned error. That
> > seems to be
> > - * the practice for cache consistency information elsewhere
> > in
> > - * the server, but I'm not sure why.
> > - */
> > - if (server->nfs_client->rpc_ops->version >= 4)
> > - return inode_peek_iversion_raw(inode);
> > - else
> > - return time_to_chattr(&inode->i_ctime);
> > + if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_CHANGE |
> > +
> > NFS_INO_REVAL_PAGECACHE))
> > + __nfs_revalidate_inode(server, inode);
> > + return inode_peek_iversion_raw(inode);
> > }
> >
> > const struct export_operations nfs_export_ops = {
> > --
> > 2.30.2
> >
>
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
