[PATCH v2 3/5] nfsd: don't stamp over an applied delegated mtime at DELEGRETURN

[Jeff Layton]

nfsd4_finalize_deleg_timestamps() stamps a/m/ctime to "now" at
DELEGRETURN when the file was written and dl_setattr is clear. But
dl_setattr is only set when both the mtime and ctime vets pass:

- nfsd4_vet_deleg_attrs() applies the holder's mtime but leaves
  dl_setattr clear if that mtime doesn't pass dl_ctime. A client mtime a
  fraction of a millisecond behind the server's ctime at grant is
  enough.
- cb_getattr_update_times() applies the holder's mtime but never sets
  dl_setattr.

Either way, DELEGRETURN then overwrites the mtime the holder supplied
with the return time. Seen as git t1092 failures over NFSv4.2: an
untouched file's c/mtime jump ~25s after git recorded them.

- Set dl_setattr whenever the holder's mtime is applied.
- If that mtime doesn't pass dl_ctime, set ctime to the current time
  instead of leaving it at its grant value. DELEGRETURN no longer
  stamps it, and where the change attribute derives from ctime (no
  i_version), it must still move across the holder's writes. The client
  fix "NFS: don't let delegated c/mtime stamps go backward" makes this
  case rare. RFC 9754 specifies the change time only when the mtime
  exceeds it; RFC 8881 section 10.4.3 says the server SHOULD keep
  time_metadata and change from going backward.

Fixes: e5e9b24ab8fa ("nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation")
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
 fs/nfsd/nfs4state.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8306a647fe9b..f276c16b3ea4 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -10744,11 +10744,16 @@ void nfsd4_vet_deleg_attrs(struct iattr *iattr, struct nfs4_delegation *dp,
 
 	if (deleg_attrs & FATTR4_WORD2_TIME_DELEG_MODIFY) {
 		if (nfsd4_vet_deleg_time(&iattr->ia_mtime, &dp->dl_mtime, &now)) {
+			/* Don't let DELEGRETURN stamp over the holder's mtime */
+			dp->dl_setattr = true;
 			iattr->ia_ctime = iattr->ia_mtime;
-			if (nfsd4_vet_deleg_time(&iattr->ia_ctime, &dp->dl_ctime, &now))
-				dp->dl_setattr = true;
-			else
-				iattr->ia_valid &= ~(ATTR_CTIME | ATTR_CTIME_SET);
+			/*
+			 * dl_setattr keeps DELEGRETURN from stamping ctime,
+			 * and the change attribute may derive from it. Set
+			 * it to the current time instead.
+			 */
+			if (!nfsd4_vet_deleg_time(&iattr->ia_ctime, &dp->dl_ctime, &now))
+				iattr->ia_valid &= ~ATTR_CTIME_SET;
 		} else {
 			iattr->ia_valid &= ~(ATTR_CTIME | ATTR_CTIME_SET |
 					     ATTR_MTIME | ATTR_MTIME_SET);
@@ -10804,8 +10809,11 @@ static void cb_getattr_update_times(const struct path *path,
 		if (nfsd4_vet_deleg_time(&attrs.ia_mtime, &dp->dl_mtime, &now)) {
 			attrs.ia_valid |= ATTR_MTIME | ATTR_MTIME_SET;
 			attrs.ia_ctime = attrs.ia_mtime;
+			/* See nfsd4_vet_deleg_attrs() */
 			if (nfsd4_vet_deleg_time(&attrs.ia_ctime, &dp->dl_ctime, &now))
 				attrs.ia_valid |= ATTR_CTIME | ATTR_CTIME_SET;
+			else
+				attrs.ia_valid |= ATTR_CTIME;
 		}
 
 		/*
@@ -10845,6 +10853,8 @@ static void cb_getattr_update_times(const struct path *path,
 		 * them.
 		 */
 		WRITE_ONCE(ncf->ncf_file_modified, true);
+		/* Don't let DELEGRETURN stamp over the holder's mtime */
+		dp->dl_setattr = true;
 	}
 	if (deleg_attrs_deleg(dp->dl_type) && !nfsd4_srv_cinfo(path, &cinfo))
 		ncf->ncf_srv_cinfo = cinfo;

-- 
2.56.0