[PATCH 1/1] NFSv4.2: fix CLONE attrs in presence of delegated attributes

[PATCH 1/1] NFSv4.2: fix CLONE attrs in presence of delegated attributes

[Olga Kornievskaia]

xfstest generic/407 is failing in 2 ways. It detects that after
doing a clone the client does not update it's mtime and it's ctime.
CLONE always sends a GETATTR operation and then calls
nfs_post_op_update_inode() based on the returned attributes.
Because of the delegated attributes the client ignores updating
the mtime. Then also, when delegated attributes are present, for
the change_attr the server replies with the same values as what
the client cached before and thus the generic/407 would flag that.
Instead, make sure we invalidate the blocks attr.

Fixes: e12912d94137 ("NFSv4: Add support for delegated atime and mtime attributes")
Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 fs/nfs/nfs42proc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 7e5c1172fc11..f6a7cfa12225 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -1306,7 +1306,15 @@ static int _nfs42_proc_clone(struct rpc_message *msg, struct file *src_f,
 		if (count == 0 && res.dst_fattr->valid & NFS_ATTR_FATTR_SIZE)
 			count = nfs_size_to_loff_t(res.dst_fattr->size) - dst_offset;
 		nfs42_copy_dest_done(dst_f, dst_offset, count, oldsize_dst);
-		status = nfs_post_op_update_inode(dst_inode, res.dst_fattr);
+		nfs_update_delegated_mtime(dst_inode);
+		if (!nfs_have_delegated_attributes(dst_inode))
+			status = nfs_post_op_update_inode(dst_inode,
+							  res.dst_fattr);
+		else {
+			spin_lock(&dst_inode->i_lock);
+			nfs_set_cache_invalid(dst_inode, NFS_INO_INVALID_BLOCKS);
+			spin_unlock(&dst_inode->i_lock);
+		}
 	}
 
 	kfree(res.dst_fattr);
-- 
2.52.0

Re: [PATCH 1/1] NFSv4.2: fix CLONE attrs in presence of delegated attributes

[Olga Kornievskaia]

On Fri, Mar 27, 2026 at 11:17 AM Olga Kornievskaia <okorniev@redhat.com> wrote:
>
> xfstest generic/407 is failing in 2 ways. It detects that after
> doing a clone the client does not update it's mtime and it's ctime.
> CLONE always sends a GETATTR operation and then calls
> nfs_post_op_update_inode() based on the returned attributes.
> Because of the delegated attributes the client ignores updating
> the mtime. Then also, when delegated attributes are present, for
> the change_attr the server replies with the same values as what
> the client cached before and thus the generic/407 would flag that.
> Instead, make sure we invalidate the blocks attr.
>
> Fixes: e12912d94137 ("NFSv4: Add support for delegated atime and mtime attributes")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>

This patch isn't needed if the server updates mtime/ctime after doing a CLONE.

> ---
>  fs/nfs/nfs42proc.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 7e5c1172fc11..f6a7cfa12225 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -1306,7 +1306,15 @@ static int _nfs42_proc_clone(struct rpc_message *msg, struct file *src_f,
>                 if (count == 0 && res.dst_fattr->valid & NFS_ATTR_FATTR_SIZE)
>                         count = nfs_size_to_loff_t(res.dst_fattr->size) - dst_offset;
>                 nfs42_copy_dest_done(dst_f, dst_offset, count, oldsize_dst);
> -               status = nfs_post_op_update_inode(dst_inode, res.dst_fattr);
> +               nfs_update_delegated_mtime(dst_inode);
> +               if (!nfs_have_delegated_attributes(dst_inode))
> +                       status = nfs_post_op_update_inode(dst_inode,
> +                                                         res.dst_fattr);
> +               else {
> +                       spin_lock(&dst_inode->i_lock);
> +                       nfs_set_cache_invalid(dst_inode, NFS_INO_INVALID_BLOCKS);
> +                       spin_unlock(&dst_inode->i_lock);
> +               }
>         }
>
>         kfree(res.dst_fattr);
> --
> 2.52.0
>
>

Re: [PATCH 1/1] NFSv4.2: fix CLONE attrs in presence of delegated attributes

[Trond Myklebust]

On Fri, 2026-03-27 at 11:11 -0400, Olga Kornievskaia wrote:
> xfstest generic/407 is failing in 2 ways. It detects that after
> doing a clone the client does not update it's mtime and it's ctime.
> CLONE always sends a GETATTR operation and then calls
> nfs_post_op_update_inode() based on the returned attributes.
> Because of the delegated attributes the client ignores updating
> the mtime. Then also, when delegated attributes are present, for
> the change_attr the server replies with the same values as what
> the client cached before and thus the generic/407 would flag that.
> Instead, make sure we invalidate the blocks attr.
> 
> Fixes: e12912d94137 ("NFSv4: Add support for delegated atime and
> mtime attributes")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
>  fs/nfs/nfs42proc.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 7e5c1172fc11..f6a7cfa12225 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -1306,7 +1306,15 @@ static int _nfs42_proc_clone(struct
> rpc_message *msg, struct file *src_f,
>  		if (count == 0 && res.dst_fattr->valid &
> NFS_ATTR_FATTR_SIZE)
>  			count = nfs_size_to_loff_t(res.dst_fattr-
> >size) - dst_offset;
>  		nfs42_copy_dest_done(dst_f, dst_offset, count,
> oldsize_dst);
> -		status = nfs_post_op_update_inode(dst_inode,
> res.dst_fattr);
> +		nfs_update_delegated_mtime(dst_inode);
> +		if (!nfs_have_delegated_attributes(dst_inode))
> +			status = nfs_post_op_update_inode(dst_inode,
> +							 
> res.dst_fattr);
> +		else {
> +			spin_lock(&dst_inode->i_lock);
> +			nfs_set_cache_invalid(dst_inode,
> NFS_INO_INVALID_BLOCKS);
> +			spin_unlock(&dst_inode->i_lock);
> +		}

Why not call nfs_post_op_update_inode() instead of setting
NFS_INO_INVALID_BLOCKS? It should be safe to do so.

>  	}
>  
>  	kfree(res.dst_fattr);

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