From: Jeff Layton <jlayton@kernel.org>
To: Chuck Lever <chuck.lever@oracle.com>, NeilBrown <neil@brown.name>,
Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
Jeff Layton <jlayton@kernel.org>
Subject: [PATCH] nfsd: don't set the ctime on delegated atime updates
Date: Tue, 15 Jul 2025 07:34:10 -0400 [thread overview]
Message-ID: <20250715-tsfix-v1-1-da21665d4626@kernel.org> (raw)
Clients will typically precede a DELEGRETURN for a delegation with
delegated timestamp with a SETATTR to set the timestamps on the server
to match what the client has.
knfsd implements this by using the nfsd_setattr() infrastructure, which
will set ATTR_CTIME on any update that goes to notify_change(). This is
problematic as it means that the client will get a spurious ctime
updates when updating the atime.
Fix this by pushing the handling of ATTR_CTIME down into the decoder
functions so that they are set earlier and more deliberately, and stop
nfsd_setattr() from implicitly adding it to every notify_change() call.
Fixes: 7e13f4f8d27d ("nfsd: handle delegated timestamps in SETATTR")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
I finally was able to spend some time on the gitr failures with
delegated timestamps and found this. With this patch I was able to run
the gitr suite in the "stress" configuration under kdevops and it
passed.
---
fs/nfsd/nfs3xdr.c | 3 +++
fs/nfsd/nfs4state.c | 2 +-
fs/nfsd/nfs4xdr.c | 18 +++++++++---------
fs/nfsd/nfsxdr.c | 3 +++
fs/nfsd/vfs.c | 1 -
5 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index ef4971d71ac4bfee5171537eda80dec6e367060d..3058c73fe2d204131c33e31e76f7ca15c2545d36 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -289,6 +289,9 @@ svcxdr_decode_sattr3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
return false;
}
+ if (iap->ia_valid)
+ iap->ia_valid |= ATTR_CTIME;
+
return true;
}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d5694987f86fadab985e55cce6261ad680e83b69..3aa430c8d941570840fc482efc750daa5df3ae84 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5690,7 +5690,7 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh,
struct nfsd4_open *open)
{
struct iattr iattr = {
- .ia_valid = ATTR_SIZE,
+ .ia_valid = ATTR_SIZE | ATTR_CTIME,
.ia_size = 0,
};
struct nfsd_attrs attrs = {
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 3afcdbed6e1465929ec7da67593243a8bf97b3f4..568b30619c79a857429113015b2ff7081557681f 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -409,7 +409,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
if (xdr_stream_decode_u64(argp->xdr, &size) < 0)
return nfserr_bad_xdr;
iattr->ia_size = size;
- iattr->ia_valid |= ATTR_SIZE;
+ iattr->ia_valid |= ATTR_SIZE | ATTR_CTIME;
}
if (bmval[0] & FATTR4_WORD0_ACL) {
status = nfsd4_decode_acl(argp, acl);
@@ -424,7 +424,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
return nfserr_bad_xdr;
iattr->ia_mode = mode;
iattr->ia_mode &= (S_IFMT | S_IALLUGO);
- iattr->ia_valid |= ATTR_MODE;
+ iattr->ia_valid |= ATTR_MODE | ATTR_CTIME;
}
if (bmval[1] & FATTR4_WORD1_OWNER) {
u32 length;
@@ -438,7 +438,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
&iattr->ia_uid);
if (status)
return status;
- iattr->ia_valid |= ATTR_UID;
+ iattr->ia_valid |= ATTR_UID | ATTR_CTIME;
}
if (bmval[1] & FATTR4_WORD1_OWNER_GROUP) {
u32 length;
@@ -452,7 +452,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
&iattr->ia_gid);
if (status)
return status;
- iattr->ia_valid |= ATTR_GID;
+ iattr->ia_valid |= ATTR_GID | ATTR_CTIME;
}
if (bmval[1] & FATTR4_WORD1_TIME_ACCESS_SET) {
u32 set_it;
@@ -464,10 +464,10 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
status = nfsd4_decode_nfstime4(argp, &iattr->ia_atime);
if (status)
return status;
- iattr->ia_valid |= (ATTR_ATIME | ATTR_ATIME_SET);
+ iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_CTIME;
break;
case NFS4_SET_TO_SERVER_TIME:
- iattr->ia_valid |= ATTR_ATIME;
+ iattr->ia_valid |= ATTR_ATIME | ATTR_CTIME;
break;
default:
return nfserr_bad_xdr;
@@ -492,10 +492,10 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
status = nfsd4_decode_nfstime4(argp, &iattr->ia_mtime);
if (status)
return status;
- iattr->ia_valid |= (ATTR_MTIME | ATTR_MTIME_SET);
+ iattr->ia_valid |= ATTR_MTIME | ATTR_MTIME_SET | ATTR_CTIME;
break;
case NFS4_SET_TO_SERVER_TIME:
- iattr->ia_valid |= ATTR_MTIME;
+ iattr->ia_valid |= ATTR_MTIME | ATTR_CTIME;
break;
default:
return nfserr_bad_xdr;
@@ -519,7 +519,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
if (xdr_stream_decode_u32(argp->xdr, &mask) < 0)
return nfserr_bad_xdr;
*umask = mask & S_IRWXUGO;
- iattr->ia_valid |= ATTR_MODE;
+ iattr->ia_valid |= ATTR_MODE | ATTR_CTIME;
}
if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
fattr4_time_deleg_access access;
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index fc262ceafca972df353a389eea2d7e99fd6bc1d2..8372a374623a5d98fff7c0227a73a9b562da407d 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -196,6 +196,9 @@ svcxdr_decode_sattr(struct svc_rqst *rqstp, struct xdr_stream *xdr,
iap->ia_valid &= ~(ATTR_ATIME_SET|ATTR_MTIME_SET);
}
+ if (iap->ia_valid)
+ iap->ia_valid |= ATTR_CTIME;
+
return true;
}
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index cd689df2ca5d7396cffb5ed9dc14f774a8f3881c..383a724e9aa20a4c6a71a563281fc8a618dd36d3 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -470,7 +470,6 @@ static int __nfsd_setattr(struct dentry *dentry, struct iattr *iap)
if (!iap->ia_valid)
return 0;
- iap->ia_valid |= ATTR_CTIME;
return notify_change(&nop_mnt_idmap, dentry, iap, NULL);
}
---
base-commit: 347e9f5043c89695b01e66b3ed111755afcf1911
change-id: 20250715-tsfix-780246904a01
Best regards,
--
Jeff Layton <jlayton@kernel.org>
next reply other threads:[~2025-07-15 11:34 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-15 11:34 Jeff Layton [this message]
2025-07-15 15:42 ` [PATCH] nfsd: don't set the ctime on delegated atime updates Chuck Lever
2025-07-15 16:46 ` Chuck Lever
2025-07-15 17:33 ` Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250715-tsfix-v1-1-da21665d4626@kernel.org \
--to=jlayton@kernel.org \
--cc=Dai.Ngo@oracle.com \
--cc=chuck.lever@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neil@brown.name \
--cc=okorniev@redhat.com \
--cc=tom@talpey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.