Re: [PATCH 2/3] NFSv42: Don't drop NFS_INO_INVALID_CHANGE if we hold a delegation

[Trond Myklebust <trondmy@hammerspace.com>]

On Tue, 2021-11-16 at 08:49 -0500, Benjamin Coddington wrote:
> The nfs_set_cache_invalid() helper drops NFS_INO_INVALID_CHANGE if we
> hold
> a delegation, but after a copy or clone the change attribute can be
> updated
> on the server.  After commit b6f80a2ebb97 "NFS: Fix open coded
> versions of
> nfs_set_cache_invalid() in NFSv4", the client stopped updating the
> change
> attribute after copy or clone while holding a read delegation.
> 
> We can use NFS_INO_REVAL_PAGECACHE to help nfs_set_cache_invalid()
> know
> when we really want to keep NFS_INO_INVALID_CHANGE, even if the
> client
> holds a delegation.

No, we really shouldn't need to care what the server thinks or does.
The client is authoritative for the change attribute while it holds a
delegation, not the server.

> 
> Fixes: b6f80a2ebb97 ("NFS: Fix open coded versions of
> nfs_set_cache_invalid() in NFSv4")
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
>  fs/nfs/inode.c     | 4 +++-
>  fs/nfs/nfs42proc.c | 3 ++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 853213b3a209..296ed8ea3273 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -202,7 +202,9 @@ void nfs_set_cache_invalid(struct inode *inode,
> unsigned long flags)
>                         flags &= ~(NFS_INO_INVALID_MODE |
>                                    NFS_INO_INVALID_OTHER |
>                                    NFS_INO_INVALID_XATTR);
> -               flags &= ~(NFS_INO_INVALID_CHANGE |
> NFS_INO_INVALID_SIZE);
> +               if (!(flags & NFS_INO_REVAL_PAGECACHE))
> +                       flags &= ~NFS_INO_INVALID_CHANGE;
> +               flags &= ~NFS_INO_INVALID_SIZE;
>         } else if (flags & NFS_INO_REVAL_PAGECACHE)
>                 flags |= NFS_INO_INVALID_CHANGE |
> NFS_INO_INVALID_SIZE;
>  
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index bbcd4c80c5a6..fc3c36e1f656 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -292,7 +292,8 @@ static void nfs42_copy_dest_done(struct inode
> *inode, loff_t pos, loff_t len)
>         spin_lock(&inode->i_lock);
>         if (newsize > i_size_read(inode))
>                 i_size_write(inode, newsize);
> -       nfs_set_cache_invalid(inode, NFS_INO_INVALID_CHANGE |
> +       nfs_set_cache_invalid(inode, NFS_INO_REVAL_PAGECACHE |
> +                                            NFS_INO_INVALID_CHANGE |
>                                              NFS_INO_INVALID_CTIME |
>                                              NFS_INO_INVALID_MTIME |
>                                              NFS_INO_INVALID_BLOCKS);

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com