[PATCH 0/2] nfs: delegated attribute fixes

[PATCH 0/2] nfs: delegated attribute fixes

[Jeff Layton]

This patchset fixes a couple of test failures in xfstests when delegated
timestamps are enabled. Please consider for v7.1!

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Jeff Layton (2):
      nfs: fix utimensat() for atime with delegated timestamps
      nfs: update inode ctime after removexattr operation

 fs/nfs/inode.c          |  9 +--------
 fs/nfs/nfs42proc.c      | 18 ++++++++++++++++--
 fs/nfs/nfs42xdr.c       | 10 ++++++++--
 include/linux/nfs_xdr.h |  3 +++
 4 files changed, 28 insertions(+), 12 deletions(-)
---
base-commit: c107785c7e8dbabd1c18301a1c362544b5786282
change-id: 20260305-nfs-7-1-9f71bcde58c5

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

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

[Jeff Layton]

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.

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

[PATCH 2/2] nfs: update inode ctime after removexattr operation

[Jeff Layton]

xfstest generic/728 fails with delegated timestamps. The client does a
removexattr and then a stat to test the ctime, which doesn't change. The
stat() doesn't trigger a GETATTR because of the delegated timestamps, so
it relies on the cached ctime, which is wrong.

The setxattr compound has a trailing GETATTR, which ensures that its
ctime gets updated. Follow the same strategy with removexattr.

Reported-by: Olga Kornievskaia <aglo@umich.edu>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfs/nfs42proc.c      | 18 ++++++++++++++++--
 fs/nfs/nfs42xdr.c       | 10 ++++++++--
 include/linux/nfs_xdr.h |  3 +++
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 7b3ca68fb4bb3bee293f8621e5ed5fa596f5da3f..7e5c1172fc11c9d5a55b3621977ac83bb98f7c20 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -1372,11 +1372,15 @@ int nfs42_proc_clone(struct file *src_f, struct file *dst_f,
 static int _nfs42_proc_removexattr(struct inode *inode, const char *name)
 {
 	struct nfs_server *server = NFS_SERVER(inode);
+	__u32 bitmask[NFS_BITMASK_SZ];
 	struct nfs42_removexattrargs args = {
 		.fh = NFS_FH(inode),
+		.bitmask = bitmask,
 		.xattr_name = name,
 	};
-	struct nfs42_removexattrres res;
+	struct nfs42_removexattrres res = {
+		.server = server,
+	};
 	struct rpc_message msg = {
 		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_REMOVEXATTR],
 		.rpc_argp = &args,
@@ -1385,12 +1389,22 @@ static int _nfs42_proc_removexattr(struct inode *inode, const char *name)
 	int ret;
 	unsigned long timestamp = jiffies;
 
+	res.fattr = nfs_alloc_fattr();
+	if (!res.fattr)
+		return -ENOMEM;
+
+	nfs4_bitmask_set(bitmask, server->cache_consistency_bitmask,
+			 inode, NFS_INO_INVALID_CHANGE);
+
 	ret = nfs4_call_sync(server->client, server, &msg, &args.seq_args,
 	    &res.seq_res, 1);
 	trace_nfs4_removexattr(inode, name, ret);
-	if (!ret)
+	if (!ret) {
 		nfs4_update_changeattr(inode, &res.cinfo, timestamp, 0);
+		ret = nfs_post_op_update_inode(inode, res.fattr);
+	}
 
+	kfree(res.fattr);
 	return ret;
 }
 
diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index 5c7452ce6e8ac94bd24bc3a33d4479d458a29907..ec105c62f721cfe01bfc60f5981396958084d627 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -263,11 +263,13 @@
 #define NFS4_enc_removexattr_sz		(compound_encode_hdr_maxsz + \
 					 encode_sequence_maxsz + \
 					 encode_putfh_maxsz + \
-					 encode_removexattr_maxsz)
+					 encode_removexattr_maxsz + \
+					 encode_getattr_maxsz)
 #define NFS4_dec_removexattr_sz		(compound_decode_hdr_maxsz + \
 					 decode_sequence_maxsz + \
 					 decode_putfh_maxsz + \
-					 decode_removexattr_maxsz)
+					 decode_removexattr_maxsz + \
+					 decode_getattr_maxsz)
 
 /*
  * These values specify the maximum amount of data that is not
@@ -869,6 +871,7 @@ static void nfs4_xdr_enc_removexattr(struct rpc_rqst *req,
 	encode_sequence(xdr, &args->seq_args, &hdr);
 	encode_putfh(xdr, args->fh, &hdr);
 	encode_removexattr(xdr, args->xattr_name, &hdr);
+	encode_getfattr(xdr, args->bitmask, &hdr);
 	encode_nops(&hdr);
 }
 
@@ -1818,6 +1821,9 @@ static int nfs4_xdr_dec_removexattr(struct rpc_rqst *req,
 		goto out;
 
 	status = decode_removexattr(xdr, &res->cinfo);
+	if (status)
+		goto out;
+	status = decode_getfattr(xdr, res->fattr, res->server);
 out:
 	return status;
 }
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index ff1f12aa73d27b6fd874baf7019dd03195fc36e5..fcbd21b5685f46136a210c8e11c20a54d6ed9dad 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1611,12 +1611,15 @@ struct nfs42_listxattrsres {
 struct nfs42_removexattrargs {
 	struct nfs4_sequence_args	seq_args;
 	struct nfs_fh			*fh;
+	const u32			*bitmask;
 	const char			*xattr_name;
 };
 
 struct nfs42_removexattrres {
 	struct nfs4_sequence_res	seq_res;
 	struct nfs4_change_info		cinfo;
+	struct nfs_fattr		*fattr;
+	const struct nfs_server		*server;
 };
 
 #endif /* CONFIG_NFS_V4_2 */

-- 
2.53.0

Re: [PATCH 0/2] nfs: delegated attribute fixes

[Olga Kornievskaia]

I can confirm that generic/221 and generic/728 no longer fail with
this patch series applied.

On Thu, Mar 5, 2026 at 1:53 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> This patchset fixes a couple of test failures in xfstests when delegated
> timestamps are enabled. Please consider for v7.1!
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Jeff Layton (2):
>       nfs: fix utimensat() for atime with delegated timestamps
>       nfs: update inode ctime after removexattr operation
>
>  fs/nfs/inode.c          |  9 +--------
>  fs/nfs/nfs42proc.c      | 18 ++++++++++++++++--
>  fs/nfs/nfs42xdr.c       | 10 ++++++++--
>  include/linux/nfs_xdr.h |  3 +++
>  4 files changed, 28 insertions(+), 12 deletions(-)
> ---
> base-commit: c107785c7e8dbabd1c18301a1c362544b5786282
> change-id: 20260305-nfs-7-1-9f71bcde58c5
>
> Best regards,
> --
> Jeff Layton <jlayton@kernel.org>
>

Re: [PATCH 0/2] nfs: delegated attribute fixes

[Anna Schumaker]

Hi Jeff,

On Thu, Mar 5, 2026, at 1:53 PM, Jeff Layton wrote:
> This patchset fixes a couple of test failures in xfstests when delegated

Can you add fixes tags to the patches?

Anna

> timestamps are enabled. Please consider for v7.1!
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Jeff Layton (2):
>       nfs: fix utimensat() for atime with delegated timestamps
>       nfs: update inode ctime after removexattr operation
>
>  fs/nfs/inode.c          |  9 +--------
>  fs/nfs/nfs42proc.c      | 18 ++++++++++++++++--
>  fs/nfs/nfs42xdr.c       | 10 ++++++++--
>  include/linux/nfs_xdr.h |  3 +++
>  4 files changed, 28 insertions(+), 12 deletions(-)
> ---
> base-commit: c107785c7e8dbabd1c18301a1c362544b5786282
> change-id: 20260305-nfs-7-1-9f71bcde58c5
>
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>

Re: [PATCH 0/2] nfs: delegated attribute fixes

[Anna Schumaker]

Hi Olga,

On Fri, Mar 13, 2026, at 11:58 AM, Olga Kornievskaia wrote:
> I can confirm that generic/221 and generic/728 no longer fail with
> this patch series applied.

Thanks for checking that. Can we add your "Tested-by" to the commits?

Anna

>
> On Thu, Mar 5, 2026 at 1:53 PM Jeff Layton <jlayton@kernel.org> wrote:
>>
>> This patchset fixes a couple of test failures in xfstests when delegated
>> timestamps are enabled. Please consider for v7.1!
>>
>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>> ---
>> Jeff Layton (2):
>>       nfs: fix utimensat() for atime with delegated timestamps
>>       nfs: update inode ctime after removexattr operation
>>
>>  fs/nfs/inode.c          |  9 +--------
>>  fs/nfs/nfs42proc.c      | 18 ++++++++++++++++--
>>  fs/nfs/nfs42xdr.c       | 10 ++++++++--
>>  include/linux/nfs_xdr.h |  3 +++
>>  4 files changed, 28 insertions(+), 12 deletions(-)
>> ---
>> base-commit: c107785c7e8dbabd1c18301a1c362544b5786282
>> change-id: 20260305-nfs-7-1-9f71bcde58c5
>>
>> Best regards,
>> --
>> Jeff Layton <jlayton@kernel.org>
>>

Re: [PATCH 0/2] nfs: delegated attribute fixes

[Jeff Layton]

On Fri, 2026-03-13 at 13:48 -0400, Anna Schumaker wrote:
> Hi Jeff,
> 
> On Thu, Mar 5, 2026, at 1:53 PM, Jeff Layton wrote:
> > This patchset fixes a couple of test failures in xfstests when delegated
> 
> Can you add fixes tags to the patches?
> 
> 

Sure.

> > timestamps are enabled. Please consider for v7.1!
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > Jeff Layton (2):
> >       nfs: fix utimensat() for atime with delegated timestamps

I think this one maybe?

    Fixes: 40f45ab3814f ("NFS: Further fixes to attribute delegation a/mtime changes")

> >       nfs: update inode ctime after removexattr operation

I'm a little less sure on this one.

Technically, this worked until delegated timestamps were introduced,
but ISTM that omitting the GETATTR in the first place was a mistake:

    Fixes: 3e1f02123fba ("NFSv4.2: add client side XDR handling for extended attributes")

> 
Can you tack those onto the patches or do you need me to resend?

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>