[PATCH v2 1/2] nfs: fix utimensat() for atime with delegated timestamps

Jeff Layton posted 2 patches 1 week, 4 days ago
[PATCH v2 1/2] nfs: fix utimensat() for atime with delegated timestamps
Posted by Jeff Layton 1 week, 4 days ago
xfstest generic/221 is failing with delegated timestamps enabled.  When
the client holds a WRITE_ATTRS_DELEG delegation, and a userland process
does a utimensat() for only the atime, the ctime is not properly
updated. The problem is that the client tries to cache the atime update,
but there is no mtime update, so the delegated attribute update never
updates the ctime.

Delegated timestamps don't have a mechanism to update the ctime in
accordance with atime-only changes due to utimensat() and the like.
Change the client to issue an RPC in this case, so that the ctime gets
properly updated alongside the atime.

Fixes: 40f45ab3814f ("NFS: Further fixes to attribute delegation a/mtime changes")
Reported-by: Olga Kornievskaia <aglo@umich.edu>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfs/inode.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 4786343eeee0f874aa1f31ace2f35fdcb83fc7a6..3a5bba7e3c92d4d4fcd65234cd2f10e56f78dee0 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -757,14 +757,7 @@ nfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	} else if (nfs_have_delegated_atime(inode) &&
 		   attr->ia_valid & ATTR_ATIME &&
 		   !(attr->ia_valid & ATTR_MTIME)) {
-		if (attr->ia_valid & ATTR_ATIME_SET) {
-			if (uid_eq(task_uid, owner_uid)) {
-				spin_lock(&inode->i_lock);
-				nfs_set_timestamps_to_ts(inode, attr);
-				spin_unlock(&inode->i_lock);
-				attr->ia_valid &= ~(ATTR_ATIME|ATTR_ATIME_SET);
-			}
-		} else {
+		if (!(attr->ia_valid & ATTR_ATIME_SET)) {
 			nfs_update_delegated_atime(inode);
 			attr->ia_valid &= ~ATTR_ATIME;
 		}

-- 
2.53.0
Re: [PATCH v2 1/2] nfs: fix utimensat() for atime with delegated timestamps
Posted by Jeff Layton 1 week, 4 days ago
On Tue, 2026-03-24 at 13:32 -0400, Jeff Layton wrote:
> xfstest generic/221 is failing with delegated timestamps enabled.  When
> the client holds a WRITE_ATTRS_DELEG delegation, and a userland process
> does a utimensat() for only the atime, the ctime is not properly
> updated. The problem is that the client tries to cache the atime update,
> but there is no mtime update, so the delegated attribute update never
> updates the ctime.
> 
> Delegated timestamps don't have a mechanism to update the ctime in
> accordance with atime-only changes due to utimensat() and the like.
> Change the client to issue an RPC in this case, so that the ctime gets
> properly updated alongside the atime.
> 
> Fixes: 40f45ab3814f ("NFS: Further fixes to attribute delegation a/mtime changes")
> Reported-by: Olga Kornievskaia <aglo@umich.edu>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfs/inode.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 4786343eeee0f874aa1f31ace2f35fdcb83fc7a6..3a5bba7e3c92d4d4fcd65234cd2f10e56f78dee0 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -757,14 +757,7 @@ nfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>  	} else if (nfs_have_delegated_atime(inode) &&
>  		   attr->ia_valid & ATTR_ATIME &&
>  		   !(attr->ia_valid & ATTR_MTIME)) {
> -		if (attr->ia_valid & ATTR_ATIME_SET) {
> -			if (uid_eq(task_uid, owner_uid)) {
> -				spin_lock(&inode->i_lock);
> -				nfs_set_timestamps_to_ts(inode, attr);
> -				spin_unlock(&inode->i_lock);
> -				attr->ia_valid &= ~(ATTR_ATIME|ATTR_ATIME_SET);
> -			}
> -		} else {

This probably deserves a comment. How about something like:

		/*
		 * An atime-only update via an explicit setattr (e.g.: utimensat() and
		 * the like) requires updating the ctime as well. Delegated timestamps
		 * don't have a mechanism for updating the ctime with a delegated
		 * atime-only update, so an RPC must be issued.
		 */

> +		if (!(attr->ia_valid & ATTR_ATIME_SET)) {
>  			nfs_update_delegated_atime(inode);
>  			attr->ia_valid &= ~ATTR_ATIME;
>  		}

-- 
Jeff Layton <jlayton@kernel.org>