[PATCH v4 00/11] nfsd: implement the "delstid" draft

[PATCH v4 00/11] nfsd: implement the "delstid" draft

[Jeff Layton]

Sorry this has taken me a bit to re-post. I've been working on some
pynfs testcases for CB_GETATTR, and have found more bugs in our
implementation.

This repost is based on top of Chuck's nfsd-next branch. The first two
patches fix a couple of different bugs in how we handle the change attr.

I also dropped one of the patches from the last series that tried to
correct how we were handling the change attr.  After going over RFC8881
section 10.4.3 a few times, I think the existing code in this respect is
actually correct. We advance the change attr in the inode on every
CB_GETATTR, which follows the guidance in the spec.

The rest of the series is more or less the same as the last one.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v4:
- get attrs from correct inode when issuing write deleg
- dropped some change attr handling patches that were incorrect
- only request FATTR4_CHANGE in CB_GETATTR if the file isn't modified
- Link to v3: https://lore.kernel.org/r/20240829-delstid-v3-0-271c60806c5d@kernel.org

Changes in v3:
- fix includes in nfs4xdr_gen.c
- drop ATTR_CTIME_DLG flag (just use ATTR_DELEG instead)
- proper handling for SETATTR (maybe? Outstanding q about stateid here)
- incorporate Neil's patches for handling non-delegation leases
- always return times from CB_GETATTR instead of from local vfs_getattr
- fix potential races vs. mgtimes by moving ctime handling into VFS layer
- Link to v2: https://lore.kernel.org/r/20240826-delstid-v2-0-e8ab5c0e39cc@kernel.org

Changes in v2:
- rebase onto Chuck's lkxdrgen branch, and reworked how autogenerated
  code is included
- declare nfsd_open_arguments as a global, so it doesn't have to be
  set up on the stack each time
- delegated timestamp support has been added
- Link to v1: https://lore.kernel.org/r/20240816-delstid-v1-0-c221c3dc14cd@kernel.org

---
Jeff Layton (11):
      nfsd: fix initial getattr on write delegation
      nfsd: drop the ncf_cb_bmap field
      nfsd: don't request change attr in CB_GETATTR once file is modified
      nfsd: drop the nfsd4_fattr_args "size" field
      nfsd: have nfsd4_deleg_getattr_conflict pass back write deleg pointer
      nfs_common: make include/linux/nfs4.h include generated nfs4.h
      nfsd: add support for FATTR4_OPEN_ARGUMENTS
      nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION
      fs: handle delegated timestamps in setattr_copy_mgtime
      nfsd: add support for delegated timestamps
      nfsd: handle delegated timestamps in SETATTR

 fs/attr.c                 |  28 +++++---
 fs/inode.c                |  74 ++++++++++++++++++++++
 fs/nfsd/Makefile          |   2 +-
 fs/nfsd/nfs4callback.c    |  50 +++++++++++++--
 fs/nfsd/nfs4proc.c        |  29 ++++++++-
 fs/nfsd/nfs4state.c       | 158 ++++++++++++++++++++++++++++++++++++++--------
 fs/nfsd/nfs4xdr.c         | 108 +++++++++++++++++++++++++++----
 fs/nfsd/nfsd.h            |   5 +-
 fs/nfsd/state.h           |   6 +-
 fs/nfsd/xdr4cb.h          |  10 ++-
 include/linux/fs.h        |   2 +
 include/linux/nfs4.h      |   7 +-
 include/linux/nfs_xdr.h   |   5 --
 include/linux/time64.h    |   5 ++
 include/uapi/linux/nfs4.h |   7 +-
 15 files changed, 416 insertions(+), 80 deletions(-)
---
base-commit: 5cd183a621de1d76daeb58379d7a2c2f8dc1143f
change-id: 20240815-delstid-93290691ad11

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

[PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Jeff Layton]

At this point in compound processing, currentfh refers to the parent of
the file, not the file itself. Get the correct dentry from the delegation
stateid instead.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index df69dc6af467..db90677fc016 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
 	}
 }
 
+static bool
+nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
+		     struct kstat *stat)
+{
+	struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);
+	struct path path;
+
+	if (!nf)
+		return false;
+
+	path.mnt = currentfh->fh_export->ex_path.mnt;
+	path.dentry = file_dentry(nf->nf_file);
+
+	if (vfs_getattr(&path, stat,
+			(STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
+			AT_STATX_SYNC_AS_STAT))
+		return false;
+	return true;
+}
+
 /*
  * The Linux NFS server does not offer write delegations to NFSv4.0
  * clients in order to avoid conflicts between write delegations and
@@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	int cb_up;
 	int status = 0;
 	struct kstat stat;
-	struct path path;
 
 	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
 	open->op_recall = false;
@@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid));
 
 	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
-		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
-		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
-		path.mnt = currentfh->fh_export->ex_path.mnt;
-		path.dentry = currentfh->fh_dentry;
-		if (vfs_getattr(&path, &stat,
-				(STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
-				AT_STATX_SYNC_AS_STAT)) {
+		if (!nfs4_delegation_stat(dp, currentfh, &stat)) {
 			nfs4_put_stid(&dp->dl_stid);
 			destroy_delegation(dp);
 			goto out_no_deleg;
 		}
+		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
 		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
 		dp->dl_cb_fattr.ncf_initial_cinfo =
 			nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
+		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
 	} else {
 		open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
 		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);

-- 
2.46.0

[PATCH v4 02/11] nfsd: drop the ncf_cb_bmap field

[Jeff Layton]

This is always the same value, and in a later patch we're going to need
to set bits in WORD2. We can simplify this code and save a little space
in the delegation too. Just hardcode the bitmap in the callback encode
function.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4callback.c | 5 ++++-
 fs/nfsd/nfs4state.c    | 1 -
 fs/nfsd/state.h        | 1 -
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index b5b3ab9d719a..0c49e31d4350 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -364,10 +364,13 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
 	struct nfs4_delegation *dp =
 		container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
 	struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
+	u32 bmap[1];
+
+	bmap[0] = FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE;
 
 	encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
 	encode_nfs_fh4(xdr, fh);
-	encode_bitmap4(xdr, fattr->ncf_cb_bmap, ARRAY_SIZE(fattr->ncf_cb_bmap));
+	encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));
 	hdr->nops++;
 }
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index db90677fc016..63b7d271cc4a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1183,7 +1183,6 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_file *fp,
 	nfsd4_init_cb(&dp->dl_cb_fattr.ncf_getattr, dp->dl_stid.sc_client,
 			&nfsd4_cb_getattr_ops, NFSPROC4_CLNT_CB_GETATTR);
 	dp->dl_cb_fattr.ncf_file_modified = false;
-	dp->dl_cb_fattr.ncf_cb_bmap[0] = FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE;
 	get_nfs4_file(fp);
 	dp->dl_stid.sc_file = fp;
 	return dp;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 79c743c01a47..ac3a29224806 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -138,7 +138,6 @@ struct nfs4_cpntf_state {
 struct nfs4_cb_fattr {
 	struct nfsd4_callback ncf_getattr;
 	u32 ncf_cb_status;
-	u32 ncf_cb_bmap[1];
 
 	/* from CB_GETATTR reply */
 	u64 ncf_cb_change;

-- 
2.46.0

[PATCH v4 03/11] nfsd: don't request change attr in CB_GETATTR once file is modified

[Jeff Layton]

RFC8881, section 10.4.3 explains that states that once the server
recognizes that the file has been modified on the client, it needn't
request the change attribute.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4callback.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 0c49e31d4350..f88d4cfe9b38 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -361,13 +361,14 @@ static void
 encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
 			struct nfs4_cb_fattr *fattr)
 {
-	struct nfs4_delegation *dp =
-		container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
+	struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
 	struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
+	struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
 	u32 bmap[1];
 
-	bmap[0] = FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE;
-
+	bmap[0] = FATTR4_WORD0_SIZE;
+	if (!ncf->ncf_file_modified)
+		bmap[0] |= FATTR4_WORD0_CHANGE;
 	encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
 	encode_nfs_fh4(xdr, fh);
 	encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));

-- 
2.46.0

[PATCH v4 04/11] nfsd: drop the nfsd4_fattr_args "size" field

[Jeff Layton]

We already have a slot for this in the kstat structure. Just overwrite
that instead of keeping a copy.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4xdr.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f118921250c3..d028daf77549 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2928,7 +2928,6 @@ struct nfsd4_fattr_args {
 	struct kstat		stat;
 	struct kstatfs		statfs;
 	struct nfs4_acl		*acl;
-	u64			size;
 #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
 	void			*context;
 	int			contextlen;
@@ -3047,7 +3046,7 @@ static __be32 nfsd4_encode_fattr4_change(struct xdr_stream *xdr,
 static __be32 nfsd4_encode_fattr4_size(struct xdr_stream *xdr,
 				       const struct nfsd4_fattr_args *args)
 {
-	return nfsd4_encode_uint64_t(xdr, args->size);
+	return nfsd4_encode_uint64_t(xdr, args->stat.size);
 }
 
 static __be32 nfsd4_encode_fattr4_fsid(struct xdr_stream *xdr,
@@ -3555,7 +3554,6 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 		if (status)
 			goto out;
 	}
-	args.size = 0;
 	if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
 		status = nfsd4_deleg_getattr_conflict(rqstp, dentry,
 					&file_modified, &size);
@@ -3569,9 +3567,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 	if (err)
 		goto out_nfserr;
 	if (file_modified)
-		args.size = size;
-	else
-		args.size = args.stat.size;
+		args.stat.size = size;
 
 	if (!(args.stat.result_mask & STATX_BTIME))
 		/* underlying FS does not offer btime so we can't share it */

-- 
2.46.0

[PATCH v4 05/11] nfsd: have nfsd4_deleg_getattr_conflict pass back write deleg pointer

[Jeff Layton]

Currently we pass back the size and whether it has been modified, but
those just mirror values tracked inside the delegation. In a later
patch, we'll need to get at the timestamps in the delegation too, so
just pass back a reference to the write delegation, and use that to
properly override values in the iattr.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 17 ++++++++---------
 fs/nfsd/nfs4xdr.c   | 16 ++++++++++------
 fs/nfsd/state.h     |  2 +-
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 63b7d271cc4a..8851fc69f5c6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -8828,8 +8828,7 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
  * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
  * @rqstp: RPC transaction context
  * @dentry: dentry of inode to be checked for a conflict
- * @modified: return true if file was modified
- * @size: new size of file if modified is true
+ * @pdp: returned WRITE delegation, if one was found
  *
  * This function is called when there is a conflict between a write
  * delegation and a change/size GETATTR from another client. The server
@@ -8839,11 +8838,12 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
  * 18.7.4.
  *
  * Returns 0 if there is no conflict; otherwise an nfs_stat
- * code is returned.
+ * code is returned. If @pdp is set to a non-NULL value, then the
+ * caller must put the reference.
  */
 __be32
 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
-				bool *modified, u64 *size)
+			     struct nfs4_delegation **pdp)
 {
 	__be32 status;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
@@ -8854,10 +8854,9 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 	struct nfs4_cb_fattr *ncf;
 	struct inode *inode = d_inode(dentry);
 
-	*modified = false;
 	ctx = locks_inode_context(inode);
 	if (!ctx)
-		return 0;
+		return nfs_ok;
 
 #define NON_NFSD_LEASE ((void *)1)
 
@@ -8923,10 +8922,10 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 			goto out_status;
 		}
 		ncf->ncf_cur_fsize = ncf->ncf_cb_fsize;
-		*size = ncf->ncf_cur_fsize;
-		*modified = true;
+		*pdp = dp;
+		return nfs_ok;
 	}
-	status = 0;
+	status = nfs_ok;
 out_status:
 	nfs4_put_stid(&dp->dl_stid);
 	return status;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index d028daf77549..ccaee73de72b 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3511,6 +3511,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 		    int ignore_crossmnt)
 {
 	DECLARE_BITMAP(attr_bitmap, ARRAY_SIZE(nfsd4_enc_fattr4_encode_ops));
+	struct nfs4_delegation *dp = NULL;
 	struct nfsd4_fattr_args args;
 	struct svc_fh *tempfh = NULL;
 	int starting_len = xdr->buf->len;
@@ -3525,8 +3526,6 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 		.dentry	= dentry,
 	};
 	unsigned long bit;
-	bool file_modified = false;
-	u64 size = 0;
 
 	WARN_ON_ONCE(bmval[1] & NFSD_WRITEONLY_ATTRS_WORD1);
 	WARN_ON_ONCE(!nfsd_attrs_supported(minorversion, bmval));
@@ -3555,8 +3554,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 			goto out;
 	}
 	if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
-		status = nfsd4_deleg_getattr_conflict(rqstp, dentry,
-					&file_modified, &size);
+		status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
 		if (status)
 			goto out;
 	}
@@ -3564,10 +3562,16 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 	err = vfs_getattr(&path, &args.stat,
 			  STATX_BASIC_STATS | STATX_BTIME | STATX_CHANGE_COOKIE,
 			  AT_STATX_SYNC_AS_STAT);
+	if (dp) {
+		struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
+
+		if (ncf->ncf_file_modified)
+			args.stat.size = ncf->ncf_cur_fsize;
+
+		nfs4_put_stid(&dp->dl_stid);
+	}
 	if (err)
 		goto out_nfserr;
-	if (file_modified)
-		args.stat.size = size;
 
 	if (!(args.stat.result_mask & STATX_BTIME))
 		/* underlying FS does not offer btime so we can't share it */
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index ac3a29224806..c7c7ec21e510 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -781,5 +781,5 @@ static inline bool try_to_expire_client(struct nfs4_client *clp)
 }
 
 extern __be32 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp,
-		struct dentry *dentry, bool *file_modified, u64 *size);
+		struct dentry *dentry, struct nfs4_delegation **pdp);
 #endif   /* NFSD4_STATE_H */

-- 
2.46.0

[PATCH v4 06/11] nfs_common: make include/linux/nfs4.h include generated nfs4.h

[Jeff Layton]

Long term, we'd like to move to autogenerating a lot of our XDR code.
Both the client and server include include/linux/nfs4.h. That file is
hand-rolled, and some of the symbols in it conflict with the
autogenerated symbols from xdrgen.

Have include/linux/nfs4.h include the generated
include/linux/sunrpc/xdrgen/nfs4.h and remove the conflicting
definitions from it and nfs_xdr.h.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 include/linux/nfs4.h    | 7 +------
 include/linux/nfs_xdr.h | 5 -----
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 8d7430d9f218..87454d5d3365 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -17,6 +17,7 @@
 #include <linux/uidgid.h>
 #include <uapi/linux/nfs4.h>
 #include <linux/sunrpc/msg_prot.h>
+#include <linux/sunrpc/xdrgen/nfs4.h>
 
 enum nfs4_acl_whotype {
 	NFS4_ACL_WHO_NAMED = 0,
@@ -512,12 +513,6 @@ enum {
 	FATTR4_XATTR_SUPPORT		= 82,
 };
 
-enum {
-	FATTR4_TIME_DELEG_ACCESS	= 84,
-	FATTR4_TIME_DELEG_MODIFY	= 85,
-	FATTR4_OPEN_ARGUMENTS		= 86,
-};
-
 /*
  * The following internal definitions enable processing the above
  * attribute bits within 32-bit word boundaries.
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 45623af3e7b8..d3fe47baf110 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1315,11 +1315,6 @@ struct nfs4_fsid_present_res {
 
 #endif /* CONFIG_NFS_V4 */
 
-struct nfstime4 {
-	u64	seconds;
-	u32	nseconds;
-};
-
 #ifdef CONFIG_NFS_V4_1
 
 struct pnfs_commit_bucket {

-- 
2.46.0

[PATCH v4 07/11] nfsd: add support for FATTR4_OPEN_ARGUMENTS

[Jeff Layton]

Add support for FATTR4_OPEN_ARGUMENTS. This a new mechanism for the
client to discover what OPEN features the server supports.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/Makefile  |  2 +-
 fs/nfsd/nfs4xdr.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfsd.h    |  3 ++-
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
index b8736a82e57c..d6ab3ae7d0a0 100644
--- a/fs/nfsd/Makefile
+++ b/fs/nfsd/Makefile
@@ -18,7 +18,7 @@ nfsd-$(CONFIG_NFSD_V2) += nfsproc.o nfsxdr.o
 nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
 nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
 nfsd-$(CONFIG_NFSD_V4)	+= nfs4proc.o nfs4xdr.o nfs4state.o nfs4idmap.o \
-			   nfs4acl.o nfs4callback.o nfs4recover.o
+			   nfs4acl.o nfs4callback.o nfs4recover.o nfs4xdr_gen.o
 nfsd-$(CONFIG_NFSD_PNFS) += nfs4layouts.o
 nfsd-$(CONFIG_NFSD_BLOCKLAYOUT) += blocklayout.o blocklayoutxdr.o
 nfsd-$(CONFIG_NFSD_SCSILAYOUT) += blocklayout.o blocklayoutxdr.o
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index ccaee73de72b..4cedcb252aa1 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -55,6 +55,7 @@
 #include "netns.h"
 #include "pnfs.h"
 #include "filecache.h"
+#include "nfs4xdr_gen.h"
 
 #include "trace.h"
 
@@ -3398,6 +3399,54 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
 	return nfsd4_encode_bool(xdr, err == 0);
 }
 
+#define NFSD_OA_SHARE_ACCESS	(BIT(OPEN_ARGS_SHARE_ACCESS_READ)	| \
+				 BIT(OPEN_ARGS_SHARE_ACCESS_WRITE)	| \
+				 BIT(OPEN_ARGS_SHARE_ACCESS_BOTH))
+
+#define NFSD_OA_SHARE_DENY	(BIT(OPEN_ARGS_SHARE_DENY_NONE)		| \
+				 BIT(OPEN_ARGS_SHARE_DENY_READ)		| \
+				 BIT(OPEN_ARGS_SHARE_DENY_WRITE)	| \
+				 BIT(OPEN_ARGS_SHARE_DENY_BOTH))
+
+#define NFSD_OA_SHARE_ACCESS_WANT	(BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG)		| \
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG)		| \
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL))
+
+#define NFSD_OA_OPEN_CLAIM	(BIT(OPEN_ARGS_OPEN_CLAIM_NULL)		| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_PREVIOUS)	| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_DELEGATE_CUR)	| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_DELEGATE_PREV)| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_FH)		| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_DELEG_CUR_FH)	| \
+				 BIT(OPEN_ARGS_OPEN_CLAIM_DELEG_PREV_FH))
+
+#define NFSD_OA_CREATE_MODE	(BIT(OPEN_ARGS_CREATEMODE_UNCHECKED4)	| \
+				 BIT(OPEN_ARGS_CREATE_MODE_GUARDED)	| \
+				 BIT(OPEN_ARGS_CREATEMODE_EXCLUSIVE4)	| \
+				 BIT(OPEN_ARGS_CREATE_MODE_EXCLUSIVE4_1))
+
+static uint32_t oa_share_access = NFSD_OA_SHARE_ACCESS;
+static uint32_t oa_share_deny = NFSD_OA_SHARE_DENY;
+static uint32_t oa_share_access_want = NFSD_OA_SHARE_ACCESS_WANT;
+static uint32_t oa_open_claim = NFSD_OA_OPEN_CLAIM;
+static uint32_t oa_create_mode = NFSD_OA_CREATE_MODE;
+
+static const struct open_arguments4 nfsd_open_arguments = {
+	.oa_share_access = { .count = 1, .element = &oa_share_access },
+	.oa_share_deny = { .count = 1, .element = &oa_share_deny },
+	.oa_share_access_want = { .count = 1, .element = &oa_share_access_want },
+	.oa_open_claim = { .count = 1, .element = &oa_open_claim },
+	.oa_create_mode = { .count = 1, .element = &oa_create_mode },
+};
+
+static __be32 nfsd4_encode_fattr4_open_arguments(struct xdr_stream *xdr,
+						 const struct nfsd4_fattr_args *args)
+{
+	if (!xdrgen_encode_fattr4_open_arguments(xdr, &nfsd_open_arguments))
+		return nfserr_resource;
+	return nfs_ok;
+}
+
 static const nfsd4_enc_attr nfsd4_enc_fattr4_encode_ops[] = {
 	[FATTR4_SUPPORTED_ATTRS]	= nfsd4_encode_fattr4_supported_attrs,
 	[FATTR4_TYPE]			= nfsd4_encode_fattr4_type,
@@ -3498,6 +3547,7 @@ static const nfsd4_enc_attr nfsd4_enc_fattr4_encode_ops[] = {
 
 	[FATTR4_MODE_UMASK]		= nfsd4_encode_fattr4__noop,
 	[FATTR4_XATTR_SUPPORT]		= nfsd4_encode_fattr4_xattr_support,
+	[FATTR4_OPEN_ARGUMENTS]		= nfsd4_encode_fattr4_open_arguments,
 };
 
 /*
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 4ccbf014a2c7..c98fb104ba7d 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -454,7 +454,8 @@ enum {
 	(NFSD4_1_SUPPORTED_ATTRS_WORD2 | \
 	FATTR4_WORD2_MODE_UMASK | \
 	NFSD4_2_SECURITY_ATTRS | \
-	FATTR4_WORD2_XATTR_SUPPORT)
+	FATTR4_WORD2_XATTR_SUPPORT | \
+	FATTR4_WORD2_OPEN_ARGUMENTS)
 
 extern const u32 nfsd_suppattrs[3][3];
 

-- 
2.46.0

[PATCH v4 08/11] nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION

[Jeff Layton]

Allow clients to request getting a delegation xor an open stateid if a
delegation isn't available. This allows the client to avoid sending a
final CLOSE for the (useless) open stateid, when it is granted a
delegation.

This is done by moving the increment of the open stateid and unlocking
of the st_mutex until after we acquire a delegation. If we get a
delegation, we zero out the op_stateid field and set the NO_OPEN_STATEID
flag. If the open stateid was brand new, then unhash it too in this case
since it won't be needed.

If we can't get a delegation or the new flag wasn't requested, then just
increment and copy the open stateid as usual.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c       | 28 ++++++++++++++++++++++++----
 fs/nfsd/nfs4xdr.c         |  5 +++--
 include/uapi/linux/nfs4.h |  7 +++++--
 3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8851fc69f5c6..40e0c9b0ef71 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6051,6 +6051,17 @@ static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
 	 */
 }
 
+/* Are we only returning a delegation stateid? */
+static bool open_xor_delegation(struct nfsd4_open *open)
+{
+	if (!(open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
+		return false;
+	if (open->op_delegate_type != NFS4_OPEN_DELEGATE_READ &&
+	    open->op_delegate_type != NFS4_OPEN_DELEGATE_WRITE)
+		return false;
+	return true;
+}
+
 /**
  * nfsd4_process_open2 - finish open processing
  * @rqstp: the RPC transaction being executed
@@ -6073,6 +6084,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 	struct nfs4_delegation *dp = NULL;
 	__be32 status;
 	bool new_stp = false;
+	bool deleg_only = false;
 
 	/*
 	 * Lookup file; if found, lookup stateid and check open request,
@@ -6127,9 +6139,6 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 			open->op_odstate = NULL;
 	}
 
-	nfs4_inc_and_copy_stateid(&open->op_stateid, &stp->st_stid);
-	mutex_unlock(&stp->st_mutex);
-
 	if (nfsd4_has_session(&resp->cstate)) {
 		if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
 			open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
@@ -6143,7 +6152,18 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 	* OPEN succeeds even if we fail.
 	*/
 	nfs4_open_delegation(open, stp, &resp->cstate.current_fh);
+	deleg_only = open_xor_delegation(open);
 nodeleg:
+	if (deleg_only) {
+		memcpy(&open->op_stateid, &zero_stateid, sizeof(open->op_stateid));
+		open->op_rflags |= OPEN4_RESULT_NO_OPEN_STATEID;
+		if (new_stp)
+			release_open_stateid(stp);
+	} else {
+		nfs4_inc_and_copy_stateid(&open->op_stateid, &stp->st_stid);
+	}
+	mutex_unlock(&stp->st_mutex);
+
 	status = nfs_ok;
 	trace_nfsd_open(&stp->st_stid.sc_stateid);
 out:
@@ -6159,7 +6179,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 	/*
 	* To finish the open response, we just need to set the rflags.
 	*/
-	open->op_rflags = NFS4_OPEN_RESULT_LOCKTYPE_POSIX;
+	open->op_rflags |= NFS4_OPEN_RESULT_LOCKTYPE_POSIX;
 	if (nfsd4_has_session(&resp->cstate))
 		open->op_rflags |= NFS4_OPEN_RESULT_MAY_NOTIFY_LOCK;
 	else if (!(open->op_openowner->oo_flags & NFS4_OO_CONFIRMED))
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 4cedcb252aa1..08d9421cd90e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1067,7 +1067,7 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *sh
 		return nfs_ok;
 	if (!argp->minorversion)
 		return nfserr_bad_xdr;
-	switch (w & NFS4_SHARE_WANT_MASK) {
+	switch (w & NFS4_SHARE_WANT_TYPE_MASK) {
 	case NFS4_SHARE_WANT_NO_PREFERENCE:
 	case NFS4_SHARE_WANT_READ_DELEG:
 	case NFS4_SHARE_WANT_WRITE_DELEG:
@@ -3410,7 +3410,8 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
 
 #define NFSD_OA_SHARE_ACCESS_WANT	(BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG)		| \
 					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG)		| \
-					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL))
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL)		| \
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
 
 #define NFSD_OA_OPEN_CLAIM	(BIT(OPEN_ARGS_OPEN_CLAIM_NULL)		| \
 				 BIT(OPEN_ARGS_OPEN_CLAIM_PREVIOUS)	| \
diff --git a/include/uapi/linux/nfs4.h b/include/uapi/linux/nfs4.h
index caf4db2fcbb9..4273e0249fcb 100644
--- a/include/uapi/linux/nfs4.h
+++ b/include/uapi/linux/nfs4.h
@@ -58,7 +58,7 @@
 #define NFS4_SHARE_DENY_BOTH	0x0003
 
 /* nfs41 */
-#define NFS4_SHARE_WANT_MASK		0xFF00
+#define NFS4_SHARE_WANT_TYPE_MASK	0xFF00
 #define NFS4_SHARE_WANT_NO_PREFERENCE	0x0000
 #define NFS4_SHARE_WANT_READ_DELEG	0x0100
 #define NFS4_SHARE_WANT_WRITE_DELEG	0x0200
@@ -66,13 +66,16 @@
 #define NFS4_SHARE_WANT_NO_DELEG	0x0400
 #define NFS4_SHARE_WANT_CANCEL		0x0500
 
-#define NFS4_SHARE_WHEN_MASK		0xF0000
+#define NFS4_SHARE_WHEN_MASK				0xF0000
 #define NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL	0x10000
 #define NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED		0x20000
 
+#define NFS4_SHARE_WANT_MOD_MASK			0xF00000
 #define NFS4_SHARE_WANT_DELEG_TIMESTAMPS		0x100000
 #define NFS4_SHARE_WANT_OPEN_XOR_DELEGATION		0x200000
 
+#define NFS4_SHARE_WANT_MASK	(NFS4_SHARE_WANT_TYPE_MASK | NFS4_SHARE_WANT_MOD_MASK)
+
 #define NFS4_CDFC4_FORE	0x1
 #define NFS4_CDFC4_BACK 0x2
 #define NFS4_CDFC4_BOTH 0x3

-- 
2.46.0

[PATCH v4 09/11] fs: handle delegated timestamps in setattr_copy_mgtime

[Jeff Layton]

When updating the ctime on an inode for a SETATTR with a multigrain
filesystem, we usually want to take the latest time we can get for the
ctime. The exception to this rule is when there is a nfsd write
delegation and the server is proxying timestamps from the client.

When nfsd gets a CB_GETATTR response, we want to update the timestamp
value in the inode to the values that the client is tracking. The client
doesn't send a ctime value (since that's always determined by the
exported filesystem), but it can send a mtime value. In the case where
it does, then we may need to update the ctime to a value commensurate
with that instead of the current time.

If ATTR_DELEG is set, then use ia_ctime value instead of setting the
timestamp to the current time.

With the addition of delegated timestamps we can also receive a request
to update only the atime, but we may not need to set the ctime. Trust
the ATTR_CTIME flag in the update and only update the ctime when it's
set.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/attr.c          | 28 +++++++++++++--------
 fs/inode.c         | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |  2 ++
 3 files changed, 94 insertions(+), 10 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index 3bcbc45708a3..392eb62aa609 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -286,16 +286,20 @@ static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr)
 	unsigned int ia_valid = attr->ia_valid;
 	struct timespec64 now;
 
-	/*
-	 * If the ctime isn't being updated then nothing else should be
-	 * either.
-	 */
-	if (!(ia_valid & ATTR_CTIME)) {
-		WARN_ON_ONCE(ia_valid & (ATTR_ATIME|ATTR_MTIME));
-		return;
+	if (ia_valid & ATTR_CTIME) {
+		/*
+		 * In the case of an update for a write delegation, we must respect
+		 * the value in ia_ctime and not use the current time.
+		 */
+		if (ia_valid & ATTR_DELEG)
+			now = inode_set_ctime_deleg(inode, attr->ia_ctime);
+		else
+			now = inode_set_ctime_current(inode);
+	} else {
+		/* If ATTR_CTIME isn't set, then ATTR_MTIME shouldn't be either. */
+		WARN_ON_ONCE(ia_valid & ATTR_MTIME);
 	}
 
-	now = inode_set_ctime_current(inode);
 	if (ia_valid & ATTR_ATIME_SET)
 		inode_set_atime_to_ts(inode, attr->ia_atime);
 	else if (ia_valid & ATTR_ATIME)
@@ -354,8 +358,12 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode,
 		inode_set_atime_to_ts(inode, attr->ia_atime);
 	if (ia_valid & ATTR_MTIME)
 		inode_set_mtime_to_ts(inode, attr->ia_mtime);
-	if (ia_valid & ATTR_CTIME)
-		inode_set_ctime_to_ts(inode, attr->ia_ctime);
+	if (ia_valid & ATTR_CTIME) {
+		if (ia_valid & ATTR_DELEG)
+			inode_set_ctime_deleg(inode, attr->ia_ctime);
+		else
+			inode_set_ctime_to_ts(inode, attr->ia_ctime);
+	}
 }
 EXPORT_SYMBOL(setattr_copy);
 
diff --git a/fs/inode.c b/fs/inode.c
index 01f7df1973bd..f0fbfd470d8e 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2835,6 +2835,80 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
 }
 EXPORT_SYMBOL(inode_set_ctime_current);
 
+/**
+ * inode_set_ctime_deleg - try to update the ctime on a delegated inode
+ * @inode: inode to update
+ * @update: timespec64 to set the ctime
+ *
+ * Attempt to atomically update the ctime on behalf of a delegation holder.
+ *
+ * The nfs server can call back the holder of a delegation to get updated
+ * inode attributes, including the mtime. When updating the mtime we may
+ * need to update the ctime to a value at least equal to that.
+ *
+ * This can race with concurrent updates to the inode, in which
+ * case we just don't do the update.
+ *
+ * Note that this works even when multigrain timestamps are not enabled,
+ * so use it in either case.
+ */
+struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 update)
+{
+	ktime_t now, floor = atomic64_read(&ctime_floor);
+	struct timespec64 now_ts, cur_ts;
+	u32 cur, old;
+
+	/* pairs with try_cmpxchg below */
+	cur = smp_load_acquire(&inode->i_ctime_nsec);
+	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
+	cur_ts.tv_sec = inode->i_ctime_sec;
+
+	/* If the update is older than the existing value, skip it. */
+	if (timespec64_compare(&update, &cur_ts) <= 0)
+		return cur_ts;
+
+	now = coarse_ctime(floor);
+	now_ts = ktime_to_timespec64(now);
+
+	/* Clamp the update to "now" if it's in the future */
+	if (timespec64_compare(&update, &now_ts) > 0)
+		update = now_ts;
+
+	update = timestamp_truncate(update, inode);
+
+	/* No need to update if the values are already the same */
+	if (timespec64_equal(&update, &cur_ts))
+		return cur_ts;
+
+	/*
+	 * Try to swap the nsec value into place. If it fails, that means
+	 * we raced with an update due to a write or similar activity. That
+	 * stamp takes precedence, so just skip the update.
+	 */
+retry:
+	old = cur;
+	if (try_cmpxchg(&inode->i_ctime_nsec, &cur, update.tv_nsec)) {
+		inode->i_ctime_sec = update.tv_sec;
+		mgtime_counter_inc(mg_ctime_swaps);
+		return update;
+	}
+
+	/*
+	 * Was the change due to someone marking the old ctime QUERIED?
+	 * If so then retry the swap. This can only happen once since
+	 * the only way to clear I_CTIME_QUERIED is to stamp the inode
+	 * with a new ctime.
+	 */
+	if (!(old & I_CTIME_QUERIED) && (cur == (old | I_CTIME_QUERIED)))
+		goto retry;
+
+	/* Otherwise, it was a new timestamp. */
+	cur_ts.tv_sec = inode->i_ctime_sec;
+	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
+	return cur_ts;
+}
+EXPORT_SYMBOL(inode_set_ctime_deleg);
+
 /**
  * in_group_or_capable - check whether caller is CAP_FSETID privileged
  * @idmap:	idmap of the mount @inode was found from
diff --git a/include/linux/fs.h b/include/linux/fs.h
index eff688e75f2f..ea7ed437d2b1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1544,6 +1544,8 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
 
 struct timespec64 current_time(struct inode *inode);
 struct timespec64 inode_set_ctime_current(struct inode *inode);
+struct timespec64 inode_set_ctime_deleg(struct inode *inode,
+					struct timespec64 update);
 
 static inline time64_t inode_get_atime_sec(const struct inode *inode)
 {

-- 
2.46.0

[PATCH v4 10/11] nfsd: add support for delegated timestamps

[Jeff Layton]

Add support for the delegated timestamps on write delegations. This
allows the server to proxy timestamps from the delegation holder to
other clients that are doing GETATTRs vs. the same inode.

Add a new flag to nfs4_delegation for indicating that the client can
provide timestamps in the CB_GETATTR response. Set that when the client
sets the appropriate flag in the open request.

Add timespec64 fields to nfs4_cb_fattr and decode the timestamps into
those. Vet those timestamps according to the delstid spec and update
the inode attrs if necessary.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4callback.c | 42 +++++++++++++++++++++++---
 fs/nfsd/nfs4state.c    | 81 ++++++++++++++++++++++++++++++++++++++++++++++----
 fs/nfsd/nfs4xdr.c      | 15 ++++++++--
 fs/nfsd/nfsd.h         |  2 ++
 fs/nfsd/state.h        |  3 ++
 fs/nfsd/xdr4cb.h       | 10 +++++--
 include/linux/time64.h |  5 ++++
 7 files changed, 143 insertions(+), 15 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index f88d4cfe9b38..43b8320c8255 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -42,6 +42,7 @@
 #include "trace.h"
 #include "xdr4cb.h"
 #include "xdr4.h"
+#include "nfs4xdr_gen.h"
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
@@ -93,12 +94,35 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
 {
 	fattr->ncf_cb_change = 0;
 	fattr->ncf_cb_fsize = 0;
+	fattr->ncf_cb_atime.tv_sec = 0;
+	fattr->ncf_cb_atime.tv_nsec = 0;
+	fattr->ncf_cb_mtime.tv_sec = 0;
+	fattr->ncf_cb_mtime.tv_nsec = 0;
+
 	if (bitmap[0] & FATTR4_WORD0_CHANGE)
 		if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
 			return -NFSERR_BAD_XDR;
 	if (bitmap[0] & FATTR4_WORD0_SIZE)
 		if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
 			return -NFSERR_BAD_XDR;
+	if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
+		fattr4_time_deleg_access access;
+
+		if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
+			return -NFSERR_BAD_XDR;
+		fattr->ncf_cb_atime.tv_sec = access.seconds;
+		fattr->ncf_cb_atime.tv_nsec = access.nseconds;
+
+	}
+	if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
+		fattr4_time_deleg_modify modify;
+
+		if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
+			return -NFSERR_BAD_XDR;
+		fattr->ncf_cb_mtime.tv_sec = modify.seconds;
+		fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
+
+	}
 	return 0;
 }
 
@@ -364,14 +388,20 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
 	struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
 	struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
 	struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
-	u32 bmap[1];
+	u32 bmap_size = 1;
+	u32 bmap[3];
 
 	bmap[0] = FATTR4_WORD0_SIZE;
 	if (!ncf->ncf_file_modified)
 		bmap[0] |= FATTR4_WORD0_CHANGE;
+	if (dp->dl_deleg_ts) {
+		bmap[1] = 0;
+		bmap[2] = FATTR4_WORD2_TIME_DELEG_ACCESS | FATTR4_WORD2_TIME_DELEG_MODIFY;
+		bmap_size = 3;
+	}
 	encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
 	encode_nfs_fh4(xdr, fh);
-	encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));
+	encode_bitmap4(xdr, bmap, bmap_size);
 	hdr->nops++;
 }
 
@@ -596,7 +626,7 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
 	struct nfs4_cb_compound_hdr hdr;
 	int status;
 	u32 bitmap[3] = {0};
-	u32 attrlen;
+	u32 attrlen, maxlen;
 	struct nfs4_cb_fattr *ncf =
 		container_of(cb, struct nfs4_cb_fattr, ncf_getattr);
 
@@ -615,7 +645,11 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
 		return -NFSERR_BAD_XDR;
 	if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
 		return -NFSERR_BAD_XDR;
-	if (attrlen > (sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize)))
+	maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
+	if (bitmap[2] != 0)
+		maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
+			   sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
+	if (attrlen > maxlen)
 		return -NFSERR_BAD_XDR;
 	status = decode_cb_fattr4(xdr, bitmap, ncf);
 	return status;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 40e0c9b0ef71..5e1d25624961 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6002,6 +6002,8 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 
 	memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid));
 
+	if (open->op_deleg_want & NFS4_SHARE_WANT_DELEG_TIMESTAMPS)
+		dp->dl_deleg_ts = true;
 	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
 		if (!nfs4_delegation_stat(dp, currentfh, &stat)) {
 			nfs4_put_stid(&dp->dl_stid);
@@ -8844,6 +8846,78 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
 	get_stateid(cstate, &u->write.wr_stateid);
 }
 
+/**
+ * set_cb_time - vet and set the timespec for a cb_getattr update
+ * @cb: timestamp from the CB_GETATTR response
+ * @orig: original timestamp in the inode
+ * @now: current time
+ *
+ * Given a timestamp in a CB_GETATTR response, check it against the
+ * current timestamp in the inode and the current time. Returns true
+ * if the inode's timestamp needs to be updated, and false otherwise.
+ * @cb may also be changed if the timestamp needs to be clamped.
+ */
+static bool set_cb_time(struct timespec64 *cb, const struct timespec64 *orig,
+			const struct timespec64 *now)
+{
+
+	/*
+	 * "When the time presented is before the original time, then the
+	 *  update is ignored." Also no need to update if there is no change.
+	 */
+	if (timespec64_compare(cb, orig) <= 0)
+		return false;
+
+	/*
+	 * "When the time presented is in the future, the server can either
+	 *  clamp the new time to the current time, or it may
+	 *  return NFS4ERR_DELAY to the client, allowing it to retry."
+	 */
+	if (timespec64_compare(cb, now) > 0) {
+		/* clamp it */
+		*cb = *now;
+	}
+
+	return true;
+}
+
+static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation *dp)
+{
+	struct inode *inode = d_inode(dentry);
+	struct timespec64 now = current_time(inode);
+	struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
+	struct iattr attrs = { };
+	int ret;
+
+	if (dp->dl_deleg_ts) {
+		struct timespec64 atime = inode_get_atime(inode);
+		struct timespec64 mtime = inode_get_mtime(inode);
+
+		attrs.ia_atime = ncf->ncf_cb_atime;
+		attrs.ia_mtime = ncf->ncf_cb_mtime;
+
+		if (set_cb_time(&attrs.ia_atime, &atime, &now))
+			attrs.ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
+
+		if (set_cb_time(&attrs.ia_mtime, &mtime, &now)) {
+			attrs.ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET;
+			attrs.ia_ctime = attrs.ia_mtime;
+		}
+	} else {
+		attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
+		attrs.ia_mtime = attrs.ia_ctime = now;
+	}
+
+	if (!attrs.ia_valid)
+		return 0;
+
+	attrs.ia_valid |= ATTR_DELEG;
+	inode_lock(inode);
+	ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
+	inode_unlock(inode);
+	return ret;
+}
+
 /**
  * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
  * @rqstp: RPC transaction context
@@ -8870,7 +8944,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 	struct file_lock_context *ctx;
 	struct nfs4_delegation *dp = NULL;
 	struct file_lease *fl;
-	struct iattr attrs;
 	struct nfs4_cb_fattr *ncf;
 	struct inode *inode = d_inode(dentry);
 
@@ -8932,11 +9005,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 		 * not update the file's metadata with the client's
 		 * modified size
 		 */
-		attrs.ia_mtime = attrs.ia_ctime = current_time(inode);
-		attrs.ia_valid = ATTR_MTIME | ATTR_CTIME | ATTR_DELEG;
-		inode_lock(inode);
-		err = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
-		inode_unlock(inode);
+		err = cb_getattr_update_times(dentry, dp);
 		if (err) {
 			status = nfserrno(err);
 			goto out_status;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 08d9421cd90e..b11d75f483de 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3411,6 +3411,7 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
 #define NFSD_OA_SHARE_ACCESS_WANT	(BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG)		| \
 					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG)		| \
 					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL)		| \
+					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS)	| \
 					 BIT(OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
 
 #define NFSD_OA_OPEN_CLAIM	(BIT(OPEN_ARGS_OPEN_CLAIM_NULL)		| \
@@ -3604,7 +3605,11 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 		if (status)
 			goto out;
 	}
-	if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
+	if ((attrmask[0] & (FATTR4_WORD0_CHANGE |
+			    FATTR4_WORD0_SIZE)) ||
+	    (attrmask[1] & (FATTR4_WORD1_TIME_ACCESS |
+			    FATTR4_WORD1_TIME_MODIFY |
+			    FATTR4_WORD1_TIME_METADATA))) {
 		status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
 		if (status)
 			goto out;
@@ -3616,8 +3621,14 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 	if (dp) {
 		struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
 
-		if (ncf->ncf_file_modified)
+		if (ncf->ncf_file_modified) {
 			args.stat.size = ncf->ncf_cur_fsize;
+			if (!timespec64_is_epoch(&ncf->ncf_cb_mtime))
+				args.stat.mtime = ncf->ncf_cb_mtime;
+		}
+
+		if (!timespec64_is_epoch(&ncf->ncf_cb_atime))
+			args.stat.atime = ncf->ncf_cb_atime;
 
 		nfs4_put_stid(&dp->dl_stid);
 	}
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index c98fb104ba7d..18b8d383f73a 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -455,6 +455,8 @@ enum {
 	FATTR4_WORD2_MODE_UMASK | \
 	NFSD4_2_SECURITY_ATTRS | \
 	FATTR4_WORD2_XATTR_SUPPORT | \
+	FATTR4_WORD2_TIME_DELEG_ACCESS | \
+	FATTR4_WORD2_TIME_DELEG_MODIFY | \
 	FATTR4_WORD2_OPEN_ARGUMENTS)
 
 extern const u32 nfsd_suppattrs[3][3];
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index c7c7ec21e510..874fcab2b183 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -142,6 +142,8 @@ struct nfs4_cb_fattr {
 	/* from CB_GETATTR reply */
 	u64 ncf_cb_change;
 	u64 ncf_cb_fsize;
+	struct timespec64 ncf_cb_mtime;
+	struct timespec64 ncf_cb_atime;
 
 	unsigned long ncf_cb_flags;
 	bool ncf_file_modified;
@@ -185,6 +187,7 @@ struct nfs4_delegation {
 	int			dl_retries;
 	struct nfsd4_callback	dl_recall;
 	bool			dl_recalled;
+	bool			dl_deleg_ts;
 
 	/* for CB_GETATTR */
 	struct nfs4_cb_fattr    dl_cb_fattr;
diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
index e8b00309c449..f1a315cd31b7 100644
--- a/fs/nfsd/xdr4cb.h
+++ b/fs/nfsd/xdr4cb.h
@@ -59,16 +59,20 @@
  * 1: CB_GETATTR opcode (32-bit)
  * N: file_handle
  * 1: number of entry in attribute array (32-bit)
- * 1: entry 0 in attribute array (32-bit)
+ * 3: entry 0-2 in attribute array (32-bit * 3)
  */
 #define NFS4_enc_cb_getattr_sz		(cb_compound_enc_hdr_sz +       \
 					cb_sequence_enc_sz +            \
-					1 + enc_nfs4_fh_sz + 1 + 1)
+					1 + enc_nfs4_fh_sz + 1 + 3)
 /*
  * 4: fattr_bitmap_maxsz
  * 1: attribute array len
  * 2: change attr (64-bit)
  * 2: size (64-bit)
+ * 2: atime.seconds (64-bit)
+ * 1: atime.nanoseconds (32-bit)
+ * 2: mtime.seconds (64-bit)
+ * 1: mtime.nanoseconds (32-bit)
  */
 #define NFS4_dec_cb_getattr_sz		(cb_compound_dec_hdr_sz  +      \
-			cb_sequence_dec_sz + 4 + 1 + 2 + 2 + op_dec_sz)
+			cb_sequence_dec_sz + 4 + 1 + 2 + 2 + 2 + 1 + 2 + 1 + op_dec_sz)
diff --git a/include/linux/time64.h b/include/linux/time64.h
index f1bcea8c124a..9934331c7b86 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -49,6 +49,11 @@ static inline int timespec64_equal(const struct timespec64 *a,
 	return (a->tv_sec == b->tv_sec) && (a->tv_nsec == b->tv_nsec);
 }
 
+static inline bool timespec64_is_epoch(const struct timespec64 *ts)
+{
+	return ts->tv_sec == 0 && ts->tv_nsec == 0;
+}
+
 /*
  * lhs < rhs:  return <0
  * lhs == rhs: return 0

-- 
2.46.0

[PATCH v4 11/11] nfsd: handle delegated timestamps in SETATTR

[Jeff Layton]

Allow SETATTR to handle delegated timestamps. This patch assumes that
only the delegation holder has the ability to set the timestamps in this
way, so we only allow this if the SETATTR stateid refers to the
delegation.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c | 29 ++++++++++++++++++++++++++---
 fs/nfsd/nfs4xdr.c  | 20 ++++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b5a6bf4f459f..7f874943583c 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1133,18 +1133,41 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		.na_iattr	= &setattr->sa_iattr,
 		.na_seclabel	= &setattr->sa_label,
 	};
+	struct nfs4_stid *st = NULL;
 	struct inode *inode;
 	__be32 status = nfs_ok;
-	bool save_no_wcc;
+	bool save_no_wcc, deleg_attrs;
 	int err;
 
-	if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
+	deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
+					      FATTR4_WORD2_TIME_DELEG_MODIFY);
+
+	if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
 		status = nfs4_preprocess_stateid_op(rqstp, cstate,
 				&cstate->current_fh, &setattr->sa_stateid,
-				WR_STATE, NULL, NULL);
+				WR_STATE, NULL, &st);
 		if (status)
 			return status;
 	}
+
+	/*
+	 * If client is trying to set delegated timestamps, ensure that the
+	 * stateid refers to a write delegation.
+	 */
+	if (deleg_attrs) {
+		status = nfserr_bad_stateid;
+		if (st->sc_type & SC_TYPE_DELEG) {
+			struct nfs4_delegation *dp = delegstateid(st);
+
+			if (dp->dl_type == NFS4_OPEN_DELEGATE_WRITE)
+				status = nfs_ok;
+		}
+	}
+	if (st)
+		nfs4_put_stid(st);
+	if (status)
+		return status;
+
 	err = fh_want_write(&cstate->current_fh);
 	if (err)
 		return nfserrno(err);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index b11d75f483de..0dea4ee8b19e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 		*umask = mask & S_IRWXUGO;
 		iattr->ia_valid |= ATTR_MODE;
 	}
+	if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
+		fattr4_time_deleg_access access;
+
+		if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
+			return nfserr_bad_xdr;
+		iattr->ia_atime.tv_sec = access.seconds;
+		iattr->ia_atime.tv_nsec = access.nseconds;
+		iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
+	}
+	if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
+		fattr4_time_deleg_modify modify;
+
+		if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
+			return nfserr_bad_xdr;
+		iattr->ia_mtime.tv_sec = modify.seconds;
+		iattr->ia_mtime.tv_nsec = modify.nseconds;
+		iattr->ia_ctime.tv_sec = modify.seconds;
+		iattr->ia_ctime.tv_nsec = modify.seconds;
+		iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
+	}
 
 	/* request sanity: did attrlist4 contain the expected number of words? */
 	if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)

-- 
2.46.0

Re: [PATCH v4 09/11] fs: handle delegated timestamps in setattr_copy_mgtime

[Chuck Lever]

On Thu, Sep 05, 2024 at 08:41:53AM -0400, Jeff Layton wrote:
> When updating the ctime on an inode for a SETATTR with a multigrain
> filesystem, we usually want to take the latest time we can get for the
> ctime. The exception to this rule is when there is a nfsd write
> delegation and the server is proxying timestamps from the client.
> 
> When nfsd gets a CB_GETATTR response, we want to update the timestamp
> value in the inode to the values that the client is tracking. The client
> doesn't send a ctime value (since that's always determined by the
> exported filesystem), but it can send a mtime value. In the case where
> it does, then we may need to update the ctime to a value commensurate
> with that instead of the current time.
> 
> If ATTR_DELEG is set, then use ia_ctime value instead of setting the
> timestamp to the current time.
> 
> With the addition of delegated timestamps we can also receive a request
> to update only the atime, but we may not need to set the ctime. Trust
> the ATTR_CTIME flag in the update and only update the ctime when it's
> set.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/attr.c          | 28 +++++++++++++--------
>  fs/inode.c         | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fs.h |  2 ++
>  3 files changed, 94 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/attr.c b/fs/attr.c
> index 3bcbc45708a3..392eb62aa609 100644
> --- a/fs/attr.c
> +++ b/fs/attr.c
> @@ -286,16 +286,20 @@ static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr)
>  	unsigned int ia_valid = attr->ia_valid;
>  	struct timespec64 now;
>  
> -	/*
> -	 * If the ctime isn't being updated then nothing else should be
> -	 * either.
> -	 */
> -	if (!(ia_valid & ATTR_CTIME)) {
> -		WARN_ON_ONCE(ia_valid & (ATTR_ATIME|ATTR_MTIME));
> -		return;
> +	if (ia_valid & ATTR_CTIME) {
> +		/*
> +		 * In the case of an update for a write delegation, we must respect
> +		 * the value in ia_ctime and not use the current time.
> +		 */
> +		if (ia_valid & ATTR_DELEG)
> +			now = inode_set_ctime_deleg(inode, attr->ia_ctime);
> +		else
> +			now = inode_set_ctime_current(inode);
> +	} else {
> +		/* If ATTR_CTIME isn't set, then ATTR_MTIME shouldn't be either. */
> +		WARN_ON_ONCE(ia_valid & ATTR_MTIME);
>  	}
>  
> -	now = inode_set_ctime_current(inode);
>  	if (ia_valid & ATTR_ATIME_SET)
>  		inode_set_atime_to_ts(inode, attr->ia_atime);
>  	else if (ia_valid & ATTR_ATIME)
> @@ -354,8 +358,12 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode,
>  		inode_set_atime_to_ts(inode, attr->ia_atime);
>  	if (ia_valid & ATTR_MTIME)
>  		inode_set_mtime_to_ts(inode, attr->ia_mtime);
> -	if (ia_valid & ATTR_CTIME)
> -		inode_set_ctime_to_ts(inode, attr->ia_ctime);
> +	if (ia_valid & ATTR_CTIME) {
> +		if (ia_valid & ATTR_DELEG)
> +			inode_set_ctime_deleg(inode, attr->ia_ctime);
> +		else
> +			inode_set_ctime_to_ts(inode, attr->ia_ctime);
> +	}
>  }
>  EXPORT_SYMBOL(setattr_copy);
>  

This patch fails to apply cleanly to my copy of nfsd-next:

  error: `git apply --index`: error: patch failed: fs/attr.c:286
  error: fs/attr.c: patch does not apply

Before I try jiggling this to get it to apply, is there anything
I should know? I worry about a potential merge conflict here,
hopefully it will be no more complicated than that.


> diff --git a/fs/inode.c b/fs/inode.c
> index 01f7df1973bd..f0fbfd470d8e 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2835,6 +2835,80 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
>  }
>  EXPORT_SYMBOL(inode_set_ctime_current);
>  
> +/**
> + * inode_set_ctime_deleg - try to update the ctime on a delegated inode
> + * @inode: inode to update
> + * @update: timespec64 to set the ctime
> + *
> + * Attempt to atomically update the ctime on behalf of a delegation holder.
> + *
> + * The nfs server can call back the holder of a delegation to get updated
> + * inode attributes, including the mtime. When updating the mtime we may
> + * need to update the ctime to a value at least equal to that.
> + *
> + * This can race with concurrent updates to the inode, in which
> + * case we just don't do the update.
> + *
> + * Note that this works even when multigrain timestamps are not enabled,
> + * so use it in either case.
> + */
> +struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 update)
> +{
> +	ktime_t now, floor = atomic64_read(&ctime_floor);
> +	struct timespec64 now_ts, cur_ts;
> +	u32 cur, old;
> +
> +	/* pairs with try_cmpxchg below */
> +	cur = smp_load_acquire(&inode->i_ctime_nsec);
> +	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
> +	cur_ts.tv_sec = inode->i_ctime_sec;
> +
> +	/* If the update is older than the existing value, skip it. */
> +	if (timespec64_compare(&update, &cur_ts) <= 0)
> +		return cur_ts;
> +
> +	now = coarse_ctime(floor);
> +	now_ts = ktime_to_timespec64(now);
> +
> +	/* Clamp the update to "now" if it's in the future */
> +	if (timespec64_compare(&update, &now_ts) > 0)
> +		update = now_ts;
> +
> +	update = timestamp_truncate(update, inode);
> +
> +	/* No need to update if the values are already the same */
> +	if (timespec64_equal(&update, &cur_ts))
> +		return cur_ts;
> +
> +	/*
> +	 * Try to swap the nsec value into place. If it fails, that means
> +	 * we raced with an update due to a write or similar activity. That
> +	 * stamp takes precedence, so just skip the update.
> +	 */
> +retry:
> +	old = cur;
> +	if (try_cmpxchg(&inode->i_ctime_nsec, &cur, update.tv_nsec)) {
> +		inode->i_ctime_sec = update.tv_sec;
> +		mgtime_counter_inc(mg_ctime_swaps);
> +		return update;
> +	}
> +
> +	/*
> +	 * Was the change due to someone marking the old ctime QUERIED?
> +	 * If so then retry the swap. This can only happen once since
> +	 * the only way to clear I_CTIME_QUERIED is to stamp the inode
> +	 * with a new ctime.
> +	 */
> +	if (!(old & I_CTIME_QUERIED) && (cur == (old | I_CTIME_QUERIED)))
> +		goto retry;
> +
> +	/* Otherwise, it was a new timestamp. */
> +	cur_ts.tv_sec = inode->i_ctime_sec;
> +	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
> +	return cur_ts;
> +}
> +EXPORT_SYMBOL(inode_set_ctime_deleg);
> +
>  /**
>   * in_group_or_capable - check whether caller is CAP_FSETID privileged
>   * @idmap:	idmap of the mount @inode was found from
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index eff688e75f2f..ea7ed437d2b1 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1544,6 +1544,8 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
>  
>  struct timespec64 current_time(struct inode *inode);
>  struct timespec64 inode_set_ctime_current(struct inode *inode);
> +struct timespec64 inode_set_ctime_deleg(struct inode *inode,
> +					struct timespec64 update);
>  
>  static inline time64_t inode_get_atime_sec(const struct inode *inode)
>  {
> 
> -- 
> 2.46.0
> 

-- 
Chuck Lever

Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Chuck Lever]

On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote:
> At this point in compound processing, currentfh refers to the parent of
> the file, not the file itself. Get the correct dentry from the delegation
> stateid instead.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Subject says "fixes ..." so IMO a Fixes: tag is warranted.
Suggestions welcome.


> ---
>  fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index df69dc6af467..db90677fc016 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
>  	}
>  }
>  
> +static bool
> +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> +		     struct kstat *stat)
> +{
> +	struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);
> +	struct path path;
> +
> +	if (!nf)
> +		return false;
> +
> +	path.mnt = currentfh->fh_export->ex_path.mnt;
> +	path.dentry = file_dentry(nf->nf_file);
> +
> +	if (vfs_getattr(&path, stat,
> +			(STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> +			AT_STATX_SYNC_AS_STAT))
> +		return false;
> +	return true;
> +}
> +
>  /*
>   * The Linux NFS server does not offer write delegations to NFSv4.0
>   * clients in order to avoid conflicts between write delegations and
> @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  	int cb_up;
>  	int status = 0;
>  	struct kstat stat;
> -	struct path path;
>  
>  	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
>  	open->op_recall = false;
> @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  	memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid));
>  
>  	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
> -		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> -		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> -		path.mnt = currentfh->fh_export->ex_path.mnt;
> -		path.dentry = currentfh->fh_dentry;
> -		if (vfs_getattr(&path, &stat,
> -				(STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> -				AT_STATX_SYNC_AS_STAT)) {
> +		if (!nfs4_delegation_stat(dp, currentfh, &stat)) {
>  			nfs4_put_stid(&dp->dl_stid);
>  			destroy_delegation(dp);
>  			goto out_no_deleg;
>  		}
> +		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
>  		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>  		dp->dl_cb_fattr.ncf_initial_cinfo =
>  			nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
> +		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
>  	} else {
>  		open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
>  		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> 
> -- 
> 2.46.0
> 

-- 
Chuck Lever

Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Jeff Layton]

On Thu, 2024-09-05 at 11:46 -0400, Chuck Lever wrote:
> On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote:
> > At this point in compound processing, currentfh refers to the parent of
> > the file, not the file itself. Get the correct dentry from the delegation
> > stateid instead.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> 
> Subject says "fixes ..." so IMO a Fixes: tag is warranted.
> Suggestions welcome.
> 
> 

Yes, sorry. I think we want:

Fixes: c5967721e106 ("NFSD: handle GETATTR conflict with write delegation")

> > ---
> >  fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
> >  1 file changed, 23 insertions(+), 8 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index df69dc6af467..db90677fc016 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
> >  	}
> >  }
> >  
> > +static bool
> > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> > +		     struct kstat *stat)
> > +{
> > +	struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);
> > +	struct path path;
> > +
> > +	if (!nf)
> > +		return false;
> > +
> > +	path.mnt = currentfh->fh_export->ex_path.mnt;
> > +	path.dentry = file_dentry(nf->nf_file);
> > +
> > +	if (vfs_getattr(&path, stat,
> > +			(STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> > +			AT_STATX_SYNC_AS_STAT))
> > +		return false;
> > +	return true;
> > +}
> > +
> >  /*
> >   * The Linux NFS server does not offer write delegations to NFSv4.0
> >   * clients in order to avoid conflicts between write delegations and
> > @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> >  	int cb_up;
> >  	int status = 0;
> >  	struct kstat stat;
> > -	struct path path;
> >  
> >  	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> >  	open->op_recall = false;
> > @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> >  	memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid));
> >  
> >  	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
> > -		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> > -		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > -		path.mnt = currentfh->fh_export->ex_path.mnt;
> > -		path.dentry = currentfh->fh_dentry;
> > -		if (vfs_getattr(&path, &stat,
> > -				(STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> > -				AT_STATX_SYNC_AS_STAT)) {
> > +		if (!nfs4_delegation_stat(dp, currentfh, &stat)) {
> >  			nfs4_put_stid(&dp->dl_stid);
> >  			destroy_delegation(dp);
> >  			goto out_no_deleg;
> >  		}
> > +		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> >  		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> >  		dp->dl_cb_fattr.ncf_initial_cinfo =
> >  			nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
> > +		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> >  	} else {
> >  		open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
> >  		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> > 
> > -- 
> > 2.46.0
> > 
> 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v4 09/11] fs: handle delegated timestamps in setattr_copy_mgtime

[Jeff Layton]

On Thu, 2024-09-05 at 11:44 -0400, Chuck Lever wrote:
> On Thu, Sep 05, 2024 at 08:41:53AM -0400, Jeff Layton wrote:
> > When updating the ctime on an inode for a SETATTR with a multigrain
> > filesystem, we usually want to take the latest time we can get for the
> > ctime. The exception to this rule is when there is a nfsd write
> > delegation and the server is proxying timestamps from the client.
> > 
> > When nfsd gets a CB_GETATTR response, we want to update the timestamp
> > value in the inode to the values that the client is tracking. The client
> > doesn't send a ctime value (since that's always determined by the
> > exported filesystem), but it can send a mtime value. In the case where
> > it does, then we may need to update the ctime to a value commensurate
> > with that instead of the current time.
> > 
> > If ATTR_DELEG is set, then use ia_ctime value instead of setting the
> > timestamp to the current time.
> > 
> > With the addition of delegated timestamps we can also receive a request
> > to update only the atime, but we may not need to set the ctime. Trust
> > the ATTR_CTIME flag in the update and only update the ctime when it's
> > set.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/attr.c          | 28 +++++++++++++--------
> >  fs/inode.c         | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/fs.h |  2 ++
> >  3 files changed, 94 insertions(+), 10 deletions(-)
> > 
> > diff --git a/fs/attr.c b/fs/attr.c
> > index 3bcbc45708a3..392eb62aa609 100644
> > --- a/fs/attr.c
> > +++ b/fs/attr.c
> > @@ -286,16 +286,20 @@ static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr)
> >  	unsigned int ia_valid = attr->ia_valid;
> >  	struct timespec64 now;
> >  
> > -	/*
> > -	 * If the ctime isn't being updated then nothing else should be
> > -	 * either.
> > -	 */
> > -	if (!(ia_valid & ATTR_CTIME)) {
> > -		WARN_ON_ONCE(ia_valid & (ATTR_ATIME|ATTR_MTIME));
> > -		return;
> > +	if (ia_valid & ATTR_CTIME) {
> > +		/*
> > +		 * In the case of an update for a write delegation, we must respect
> > +		 * the value in ia_ctime and not use the current time.
> > +		 */
> > +		if (ia_valid & ATTR_DELEG)
> > +			now = inode_set_ctime_deleg(inode, attr->ia_ctime);
> > +		else
> > +			now = inode_set_ctime_current(inode);
> > +	} else {
> > +		/* If ATTR_CTIME isn't set, then ATTR_MTIME shouldn't be either. */
> > +		WARN_ON_ONCE(ia_valid & ATTR_MTIME);
> >  	}
> >  
> > -	now = inode_set_ctime_current(inode);
> >  	if (ia_valid & ATTR_ATIME_SET)
> >  		inode_set_atime_to_ts(inode, attr->ia_atime);
> >  	else if (ia_valid & ATTR_ATIME)
> > @@ -354,8 +358,12 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode,
> >  		inode_set_atime_to_ts(inode, attr->ia_atime);
> >  	if (ia_valid & ATTR_MTIME)
> >  		inode_set_mtime_to_ts(inode, attr->ia_mtime);
> > -	if (ia_valid & ATTR_CTIME)
> > -		inode_set_ctime_to_ts(inode, attr->ia_ctime);
> > +	if (ia_valid & ATTR_CTIME) {
> > +		if (ia_valid & ATTR_DELEG)
> > +			inode_set_ctime_deleg(inode, attr->ia_ctime);
> > +		else
> > +			inode_set_ctime_to_ts(inode, attr->ia_ctime);
> > +	}
> >  }
> >  EXPORT_SYMBOL(setattr_copy);
> >  
> 
> This patch fails to apply cleanly to my copy of nfsd-next:
> 
>   error: `git apply --index`: error: patch failed: fs/attr.c:286
>   error: fs/attr.c: patch does not apply
> 
> Before I try jiggling this to get it to apply, is there anything
> I should know? I worry about a potential merge conflict here,
> hopefully it will be no more complicated than that.
> 
> 

This is based on a combo of your nfsd-next branch and Christian's
vfs.mgtime branch. This patch in particular depends on the multigrain
patches in his tree.

I think we can just drop this patch from the series in your tree, and
I'll get Christian to pick it up in his tree. The ctime setting will be
a bit racy without it but everything should still work.

Sound ok? Christian, are you OK with picking this up in vfs.mgtime?

> > diff --git a/fs/inode.c b/fs/inode.c
> > index 01f7df1973bd..f0fbfd470d8e 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -2835,6 +2835,80 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
> >  }
> >  EXPORT_SYMBOL(inode_set_ctime_current);
> >  
> > +/**
> > + * inode_set_ctime_deleg - try to update the ctime on a delegated inode
> > + * @inode: inode to update
> > + * @update: timespec64 to set the ctime
> > + *
> > + * Attempt to atomically update the ctime on behalf of a delegation holder.
> > + *
> > + * The nfs server can call back the holder of a delegation to get updated
> > + * inode attributes, including the mtime. When updating the mtime we may
> > + * need to update the ctime to a value at least equal to that.
> > + *
> > + * This can race with concurrent updates to the inode, in which
> > + * case we just don't do the update.
> > + *
> > + * Note that this works even when multigrain timestamps are not enabled,
> > + * so use it in either case.
> > + */
> > +struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 update)
> > +{
> > +	ktime_t now, floor = atomic64_read(&ctime_floor);
> > +	struct timespec64 now_ts, cur_ts;
> > +	u32 cur, old;
> > +
> > +	/* pairs with try_cmpxchg below */
> > +	cur = smp_load_acquire(&inode->i_ctime_nsec);
> > +	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
> > +	cur_ts.tv_sec = inode->i_ctime_sec;
> > +
> > +	/* If the update is older than the existing value, skip it. */
> > +	if (timespec64_compare(&update, &cur_ts) <= 0)
> > +		return cur_ts;
> > +
> > +	now = coarse_ctime(floor);
> > +	now_ts = ktime_to_timespec64(now);
> > +
> > +	/* Clamp the update to "now" if it's in the future */
> > +	if (timespec64_compare(&update, &now_ts) > 0)
> > +		update = now_ts;
> > +
> > +	update = timestamp_truncate(update, inode);
> > +
> > +	/* No need to update if the values are already the same */
> > +	if (timespec64_equal(&update, &cur_ts))
> > +		return cur_ts;
> > +
> > +	/*
> > +	 * Try to swap the nsec value into place. If it fails, that means
> > +	 * we raced with an update due to a write or similar activity. That
> > +	 * stamp takes precedence, so just skip the update.
> > +	 */
> > +retry:
> > +	old = cur;
> > +	if (try_cmpxchg(&inode->i_ctime_nsec, &cur, update.tv_nsec)) {
> > +		inode->i_ctime_sec = update.tv_sec;
> > +		mgtime_counter_inc(mg_ctime_swaps);
> > +		return update;
> > +	}
> > +
> > +	/*
> > +	 * Was the change due to someone marking the old ctime QUERIED?
> > +	 * If so then retry the swap. This can only happen once since
> > +	 * the only way to clear I_CTIME_QUERIED is to stamp the inode
> > +	 * with a new ctime.
> > +	 */
> > +	if (!(old & I_CTIME_QUERIED) && (cur == (old | I_CTIME_QUERIED)))
> > +		goto retry;
> > +
> > +	/* Otherwise, it was a new timestamp. */
> > +	cur_ts.tv_sec = inode->i_ctime_sec;
> > +	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
> > +	return cur_ts;
> > +}
> > +EXPORT_SYMBOL(inode_set_ctime_deleg);
> > +
> >  /**
> >   * in_group_or_capable - check whether caller is CAP_FSETID privileged
> >   * @idmap:	idmap of the mount @inode was found from
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index eff688e75f2f..ea7ed437d2b1 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1544,6 +1544,8 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
> >  
> >  struct timespec64 current_time(struct inode *inode);
> >  struct timespec64 inode_set_ctime_current(struct inode *inode);
> > +struct timespec64 inode_set_ctime_deleg(struct inode *inode,
> > +					struct timespec64 update);
> >  
> >  static inline time64_t inode_get_atime_sec(const struct inode *inode)
> >  {
> > 
> > -- 
> > 2.46.0
> > 
> 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v4 00/11] nfsd: implement the "delstid" draft

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

On Thu, 05 Sep 2024 08:41:44 -0400, Jeff Layton wrote:                                              
> Sorry this has taken me a bit to re-post. I've been working on some
> pynfs testcases for CB_GETATTR, and have found more bugs in our
> implementation.
> 
> This repost is based on top of Chuck's nfsd-next branch. The first two
> patches fix a couple of different bugs in how we handle the change attr.
> 
> [...]                                                                        

Dropped 9/11, applied the rest to nfsd-next for v6.12, thanks!                                                                

[01/11] nfsd: fix initial getattr on write delegation
        commit: 51158790589f1ed3a98f2d590b071ea76bd0b19d
[02/11] nfsd: drop the ncf_cb_bmap field
        commit: 95cc4b27389630394dd42a5d7e12dfc92e82e49d
[03/11] nfsd: don't request change attr in CB_GETATTR once file is modified
        commit: 4374695a6b0ffdce0c9dce1d7833f87d5cc622b4
[04/11] nfsd: drop the nfsd4_fattr_args "size" field
        commit: 6cb6bcdbdf416ad24dcf430f8b88d7c38810efc2
[05/11] nfsd: have nfsd4_deleg_getattr_conflict pass back write deleg pointer
        commit: eecc86f5e5390c64e5f85241b8872f081dd7c520
[06/11] nfs_common: make include/linux/nfs4.h include generated nfs4.h
        commit: 4443c4f509e74bb91e059123eb8b42ad6fd666fd
[07/11] nfsd: add support for FATTR4_OPEN_ARGUMENTS
        commit: 9d0b846784f084479c5beeb61458f0d38ac16f1e
[08/11] nfsd: implement OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION
        commit: 91556251507771f943c2f04b2ffbf517908bbb48
[09/11] fs: handle delegated timestamps in setattr_copy_mgtime
        (no commit info)
[10/11] nfsd: add support for delegated timestamps
        commit: 921347aabef19b4df68d25d4327091b4a0554bac
[11/11] nfsd: handle delegated timestamps in SETATTR
        commit: 8dd7cf087ba772634cba60ab922febae0b756271                                                                      

--                                                                              
Chuck Lever

Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Jeff Layton]

On Thu, 2024-09-05 at 08:41 -0400, Jeff Layton wrote:
> At this point in compound processing, currentfh refers to the parent of
> the file, not the file itself. Get the correct dentry from the delegation
> stateid instead.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index df69dc6af467..db90677fc016 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
>  	}
>  }
>  
> +static bool
> +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> +		     struct kstat *stat)
> +{
> +	struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);
> +	struct path path;
> +
> +	if (!nf)
> +		return false;
> +
> +	path.mnt = currentfh->fh_export->ex_path.mnt;
> +	path.dentry = file_dentry(nf->nf_file);
> +
> +	if (vfs_getattr(&path, stat,
> +			(STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),

Minor oversight here.

I added STATX_INO when I was debugging, but we don't need it here. We
should probably drop that flag (though it's mostly harmless). Chuck,
would you be ok with fixing that up?

> +			AT_STATX_SYNC_AS_STAT))
> +		return false;
> +	return true;
> +}
> +
>  /*
>   * The Linux NFS server does not offer write delegations to NFSv4.0
>   * clients in order to avoid conflicts between write delegations and
> @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  	int cb_up;
>  	int status = 0;
>  	struct kstat stat;
> -	struct path path;
>  
>  	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
>  	open->op_recall = false;
> @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  	memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid));
>  
>  	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
> -		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> -		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> -		path.mnt = currentfh->fh_export->ex_path.mnt;
> -		path.dentry = currentfh->fh_dentry;
> -		if (vfs_getattr(&path, &stat,
> -				(STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> -				AT_STATX_SYNC_AS_STAT)) {
> +		if (!nfs4_delegation_stat(dp, currentfh, &stat)) {
>  			nfs4_put_stid(&dp->dl_stid);
>  			destroy_delegation(dp);
>  			goto out_no_deleg;
>  		}
> +		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
>  		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>  		dp->dl_cb_fattr.ncf_initial_cinfo =
>  			nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
> +		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
>  	} else {
>  		open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
>  		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> 

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

Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Chuck Lever]

On Fri, Sep 06, 2024 at 10:08:29AM -0400, Jeff Layton wrote:
> On Thu, 2024-09-05 at 08:41 -0400, Jeff Layton wrote:
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index df69dc6af467..db90677fc016 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
> >  	}
> >  }
> >  
> > +static bool
> > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> > +		     struct kstat *stat)
> > +{
> > +	struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);
> > +	struct path path;
> > +
> > +	if (!nf)
> > +		return false;
> > +
> > +	path.mnt = currentfh->fh_export->ex_path.mnt;
> > +	path.dentry = file_dentry(nf->nf_file);
> > +
> > +	if (vfs_getattr(&path, stat,
> > +			(STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> 
> Minor oversight here.
> 
> I added STATX_INO when I was debugging, but we don't need it here. We
> should probably drop that flag (though it's mostly harmless). Chuck,
> would you be ok with fixing that up?

Fixed and squashed into nfsd-next.

-- 
Chuck Lever

Re: [PATCH v4 09/11] fs: handle delegated timestamps in setattr_copy_mgtime

[Christian Brauner]

On Thu, Sep 05, 2024 at 01:46:14PM GMT, Jeff Layton wrote:
> On Thu, 2024-09-05 at 11:44 -0400, Chuck Lever wrote:
> > On Thu, Sep 05, 2024 at 08:41:53AM -0400, Jeff Layton wrote:
> > > When updating the ctime on an inode for a SETATTR with a multigrain
> > > filesystem, we usually want to take the latest time we can get for the
> > > ctime. The exception to this rule is when there is a nfsd write
> > > delegation and the server is proxying timestamps from the client.
> > > 
> > > When nfsd gets a CB_GETATTR response, we want to update the timestamp
> > > value in the inode to the values that the client is tracking. The client
> > > doesn't send a ctime value (since that's always determined by the
> > > exported filesystem), but it can send a mtime value. In the case where
> > > it does, then we may need to update the ctime to a value commensurate
> > > with that instead of the current time.
> > > 
> > > If ATTR_DELEG is set, then use ia_ctime value instead of setting the
> > > timestamp to the current time.
> > > 
> > > With the addition of delegated timestamps we can also receive a request
> > > to update only the atime, but we may not need to set the ctime. Trust
> > > the ATTR_CTIME flag in the update and only update the ctime when it's
> > > set.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  fs/attr.c          | 28 +++++++++++++--------
> > >  fs/inode.c         | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/fs.h |  2 ++
> > >  3 files changed, 94 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/fs/attr.c b/fs/attr.c
> > > index 3bcbc45708a3..392eb62aa609 100644
> > > --- a/fs/attr.c
> > > +++ b/fs/attr.c
> > > @@ -286,16 +286,20 @@ static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr)
> > >  	unsigned int ia_valid = attr->ia_valid;
> > >  	struct timespec64 now;
> > >  
> > > -	/*
> > > -	 * If the ctime isn't being updated then nothing else should be
> > > -	 * either.
> > > -	 */
> > > -	if (!(ia_valid & ATTR_CTIME)) {
> > > -		WARN_ON_ONCE(ia_valid & (ATTR_ATIME|ATTR_MTIME));
> > > -		return;
> > > +	if (ia_valid & ATTR_CTIME) {
> > > +		/*
> > > +		 * In the case of an update for a write delegation, we must respect
> > > +		 * the value in ia_ctime and not use the current time.
> > > +		 */
> > > +		if (ia_valid & ATTR_DELEG)
> > > +			now = inode_set_ctime_deleg(inode, attr->ia_ctime);
> > > +		else
> > > +			now = inode_set_ctime_current(inode);
> > > +	} else {
> > > +		/* If ATTR_CTIME isn't set, then ATTR_MTIME shouldn't be either. */
> > > +		WARN_ON_ONCE(ia_valid & ATTR_MTIME);
> > >  	}
> > >  
> > > -	now = inode_set_ctime_current(inode);
> > >  	if (ia_valid & ATTR_ATIME_SET)
> > >  		inode_set_atime_to_ts(inode, attr->ia_atime);
> > >  	else if (ia_valid & ATTR_ATIME)
> > > @@ -354,8 +358,12 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode,
> > >  		inode_set_atime_to_ts(inode, attr->ia_atime);
> > >  	if (ia_valid & ATTR_MTIME)
> > >  		inode_set_mtime_to_ts(inode, attr->ia_mtime);
> > > -	if (ia_valid & ATTR_CTIME)
> > > -		inode_set_ctime_to_ts(inode, attr->ia_ctime);
> > > +	if (ia_valid & ATTR_CTIME) {
> > > +		if (ia_valid & ATTR_DELEG)
> > > +			inode_set_ctime_deleg(inode, attr->ia_ctime);
> > > +		else
> > > +			inode_set_ctime_to_ts(inode, attr->ia_ctime);
> > > +	}
> > >  }
> > >  EXPORT_SYMBOL(setattr_copy);
> > >  
> > 
> > This patch fails to apply cleanly to my copy of nfsd-next:
> > 
> >   error: `git apply --index`: error: patch failed: fs/attr.c:286
> >   error: fs/attr.c: patch does not apply
> > 
> > Before I try jiggling this to get it to apply, is there anything
> > I should know? I worry about a potential merge conflict here,
> > hopefully it will be no more complicated than that.
> > 
> > 
> 
> This is based on a combo of your nfsd-next branch and Christian's
> vfs.mgtime branch. This patch in particular depends on the multigrain
> patches in his tree.
> 
> I think we can just drop this patch from the series in your tree, and
> I'll get Christian to pick it up in his tree. The ctime setting will be
> a bit racy without it but everything should still work.
> 
> Sound ok? Christian, are you OK with picking this up in vfs.mgtime?

Yep, of course. Already picked it up.

Re: (subset) [PATCH v4 09/11] fs: handle delegated timestamps in setattr_copy_mgtime

[Christian Brauner]

On Thu, 05 Sep 2024 08:41:53 -0400, Jeff Layton wrote:
> When updating the ctime on an inode for a SETATTR with a multigrain
> filesystem, we usually want to take the latest time we can get for the
> ctime. The exception to this rule is when there is a nfsd write
> delegation and the server is proxying timestamps from the client.
> 
> When nfsd gets a CB_GETATTR response, we want to update the timestamp
> value in the inode to the values that the client is tracking. The client
> doesn't send a ctime value (since that's always determined by the
> exported filesystem), but it can send a mtime value. In the case where
> it does, then we may need to update the ctime to a value commensurate
> with that instead of the current time.
> 
> [...]

Applied to the vfs.mgtime branch of the vfs/vfs.git tree.
Patches in the vfs.mgtime branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.mgtime

[09/11] fs: handle delegated timestamps in setattr_copy_mgtime
        https://git.kernel.org/vfs/vfs/c/b0d5ea249d88

Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Chuck Lever]

On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote:
> At this point in compound processing, currentfh refers to the parent of
> the file, not the file itself. Get the correct dentry from the delegation
> stateid instead.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index df69dc6af467..db90677fc016 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
>  	}
>  }
>  
> +static bool
> +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> +		     struct kstat *stat)
> +{
> +	struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);

The xfstests workflow on NFSv4.2 exhausts the capacity of both the
main and scratch devices (backed by xfs) about half-way through
each test run.

Deleting all visible files on both devices frees only a little bit
of space. The test exports can be unshared but not unmounted
(EBUSY). Looks like unlinked but still open files, maybe.

Bisected to this here patch.

Should there be a matching nfsd_file_put() book-end for the new
find_rw_file() call site?


> +	struct path path;
> +
> +	if (!nf)
> +		return false;
> +
> +	path.mnt = currentfh->fh_export->ex_path.mnt;
> +	path.dentry = file_dentry(nf->nf_file);
> +
> +	if (vfs_getattr(&path, stat,
> +			(STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> +			AT_STATX_SYNC_AS_STAT))
> +		return false;
> +	return true;
> +}
> +
>  /*
>   * The Linux NFS server does not offer write delegations to NFSv4.0
>   * clients in order to avoid conflicts between write delegations and
> @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  	int cb_up;
>  	int status = 0;
>  	struct kstat stat;
> -	struct path path;
>  
>  	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
>  	open->op_recall = false;
> @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  	memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid));
>  
>  	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
> -		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> -		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> -		path.mnt = currentfh->fh_export->ex_path.mnt;
> -		path.dentry = currentfh->fh_dentry;
> -		if (vfs_getattr(&path, &stat,
> -				(STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> -				AT_STATX_SYNC_AS_STAT)) {
> +		if (!nfs4_delegation_stat(dp, currentfh, &stat)) {
>  			nfs4_put_stid(&dp->dl_stid);
>  			destroy_delegation(dp);
>  			goto out_no_deleg;
>  		}
> +		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
>  		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>  		dp->dl_cb_fattr.ncf_initial_cinfo =
>  			nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
> +		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
>  	} else {
>  		open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
>  		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> 
> -- 
> 2.46.0
> 

-- 
Chuck Lever

Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Jeff Layton]

On Sun, 2024-09-08 at 14:00 -0400, Chuck Lever wrote:
> On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote:
> > At this point in compound processing, currentfh refers to the parent of
> > the file, not the file itself. Get the correct dentry from the delegation
> > stateid instead.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
> >  1 file changed, 23 insertions(+), 8 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index df69dc6af467..db90677fc016 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
> >  	}
> >  }
> >  
> > +static bool
> > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> > +		     struct kstat *stat)
> > +{
> > +	struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);
> 
> The xfstests workflow on NFSv4.2 exhausts the capacity of both the
> main and scratch devices (backed by xfs) about half-way through
> each test run.
> 
> Deleting all visible files on both devices frees only a little bit
> of space. The test exports can be unshared but not unmounted
> (EBUSY). Looks like unlinked but still open files, maybe.
> 
> Bisected to this here patch.
> 
> Should there be a matching nfsd_file_put() book-end for the new
> find_rw_file() call site?
> 

Yes. Braino on my end. I was thinking that find_rw_file didn't take a
reference, but it does and we do need to put it. Would you like me to
respin, or do you just want to add it in the appropriate spot?

> 
> > +	struct path path;
> > +
> > +	if (!nf)
> > +		return false;
> > +
> > +	path.mnt = currentfh->fh_export->ex_path.mnt;
> > +	path.dentry = file_dentry(nf->nf_file);
> > +
> > +	if (vfs_getattr(&path, stat,
> > +			(STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> > +			AT_STATX_SYNC_AS_STAT))
> > +		return false;
> > +	return true;
> > +}
> > +
> >  /*
> >   * The Linux NFS server does not offer write delegations to NFSv4.0
> >   * clients in order to avoid conflicts between write delegations and
> > @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> >  	int cb_up;
> >  	int status = 0;
> >  	struct kstat stat;
> > -	struct path path;
> >  
> >  	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> >  	open->op_recall = false;
> > @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> >  	memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid));
> >  
> >  	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
> > -		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> > -		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > -		path.mnt = currentfh->fh_export->ex_path.mnt;
> > -		path.dentry = currentfh->fh_dentry;
> > -		if (vfs_getattr(&path, &stat,
> > -				(STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> > -				AT_STATX_SYNC_AS_STAT)) {
> > +		if (!nfs4_delegation_stat(dp, currentfh, &stat)) {
> >  			nfs4_put_stid(&dp->dl_stid);
> >  			destroy_delegation(dp);
> >  			goto out_no_deleg;
> >  		}
> > +		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> >  		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> >  		dp->dl_cb_fattr.ncf_initial_cinfo =
> >  			nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
> > +		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> >  	} else {
> >  		open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
> >  		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> > 
> > -- 
> > 2.46.0
> > 
> 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Cedric Blancher]

On Sun, 8 Sept 2024 at 22:40, Jeff Layton <jlayton@kernel.org> wrote:
>
> On Sun, 2024-09-08 at 14:00 -0400, Chuck Lever wrote:
> > On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote:
> > > At this point in compound processing, currentfh refers to the parent of
> > > the file, not the file itself. Get the correct dentry from the delegation
> > > stateid instead.
> > >
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
> > >  1 file changed, 23 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > index df69dc6af467..db90677fc016 100644
> > > --- a/fs/nfsd/nfs4state.c
> > > +++ b/fs/nfsd/nfs4state.c
> > > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
> > >     }
> > >  }
> > >
> > > +static bool
> > > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> > > +                struct kstat *stat)
> > > +{
> > > +   struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);
> >
> > The xfstests workflow on NFSv4.2 exhausts the capacity of both the
> > main and scratch devices (backed by xfs) about half-way through
> > each test run.
> >
> > Deleting all visible files on both devices frees only a little bit
> > of space. The test exports can be unshared but not unmounted
> > (EBUSY). Looks like unlinked but still open files, maybe.
> >
> > Bisected to this here patch.
> >
> > Should there be a matching nfsd_file_put() book-end for the new
> > find_rw_file() call site?
> >
>
> Yes. Braino on my end. I was thinking that find_rw_file didn't take a
> reference, but it does and we do need to put it. Would you like me to
> respin, or do you just want to add it in the appropriate spot?

Did the respin ever happen?

Ced
-- 
Cedric Blancher <cedric.blancher@gmail.com>
[https://plus.google.com/u/0/+CedricBlancher/]
Institute Pasteur

Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Cedric Blancher]

On Thu, 5 Sept 2024 at 14:42, Jeff Layton <jlayton@kernel.org> wrote:
>
> At this point in compound processing, currentfh refers to the parent of
> the file, not the file itself. Get the correct dentry from the delegation
> stateid instead.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index df69dc6af467..db90677fc016 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c

Is this commit going to be backported to Linux 6.6 LTS?

Ced
-- 
Cedric Blancher <cedric.blancher@gmail.com>
[https://plus.google.com/u/0/+CedricBlancher/]
Institute Pasteur

Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

[Chuck Lever]

On 12/27/24 2:53 AM, Cedric Blancher wrote:
> On Thu, 5 Sept 2024 at 14:42, Jeff Layton <jlayton@kernel.org> wrote:
>>
>> At this point in compound processing, currentfh refers to the parent of
>> the file, not the file itself. Get the correct dentry from the delegation
>> stateid instead.
>>
>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>> ---
>>   fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
>>   1 file changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index df69dc6af467..db90677fc016 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
> 
> Is this commit going to be backported to Linux 6.6 LTS?

The quoted patch is

  
https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/fs/nfsd/nfs4state.c?id=bf92e5008b17f935a6de8b708551e02c2294121c

in the master branch of Linus' tree. This commit has the following tag:

   Fixes: c5967721e106 ("NFSD: handle GETATTR conflict with write 
delegation")

Commit c5967721e106 was merged in 6.9. Thus bf92e5008b17 won't be
backported to kernels earlier than 6.9 unless they also have commit
c5967721e106. It looks like 6.6 LTS does not have that commit.


-- 
Chuck Lever