[PATCH v2 0/3] NFS Client btime support

[PATCH v2 0/3] NFS Client btime support

[Benjamin Coddington]

We're getting requests to allow the client to support btime, current kNFSD
already supports this for compatible filesystems.

I've rebased and worked the first three patches from
https://lore.kernel.org/linux-nfs/20211227190504.309612-1-trondmy@kernel.org/
onto v6.14, and I've been driving them around bakeathon without any signs
of trouble yet this week.

Thanks Trond and Anne for this (years-ago) work.

v2 - fix unnecessary cast to printk

Anne Marie Merritt (1):
  nfs: Add timecreate to nfs inode

Trond Myklebust (2):
  Expand the type of nfs_fattr->valid
  NFS: Return the file btime in the statx results when appropriate

 fs/nfs/inode.c            | 45 ++++++++++++++++++++++++-------
 fs/nfs/nfs4proc.c         | 14 +++++++++-
 fs/nfs/nfs4trace.h        |  3 ++-
 fs/nfs/nfs4xdr.c          | 24 +++++++++++++++++
 fs/nfs/nfstrace.h         |  3 ++-
 include/linux/nfs_fs.h    |  7 +++++
 include/linux/nfs_fs_sb.h |  2 +-
 include/linux/nfs_xdr.h   | 57 ++++++++++++++++++++-------------------
 8 files changed, 115 insertions(+), 40 deletions(-)

-- 
2.47.0

[PATCH v2 1/3] Expand the type of nfs_fattr->valid

[Benjamin Coddington]

From: Trond Myklebust <trond.myklebust@primarydata.com>

We need to be able to track more than 32 attributes per inode.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/inode.c            |  2 +-
 include/linux/nfs_fs_sb.h |  2 +-
 include/linux/nfs_xdr.h   | 54 +++++++++++++++++++--------------------
 3 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 119e447758b9..160f3478a835 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -2166,7 +2166,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 	bool attr_changed = false;
 	bool have_delegation;
 
-	dfprintk(VFS, "NFS: %s(%s/%lu fh_crc=0x%08x ct=%d info=0x%x)\n",
+	dfprintk(VFS, "NFS: %s(%s/%lu fh_crc=0x%08x ct=%d info=0x%llx)\n",
 			__func__, inode->i_sb->s_id, inode->i_ino,
 			nfs_display_fhandle_hash(NFS_FH(inode)),
 			atomic_read(&inode->i_count), fattr->valid);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index ee03f3cef30c..4b38b8e709f8 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -171,8 +171,8 @@ struct nfs_server {
 #define NFS_MOUNT_FORCE_RDIRPLUS	0x20000000
 #define NFS_MOUNT_NETUNREACH_FATAL	0x40000000
 
-	unsigned int		fattr_valid;	/* Valid attributes */
 	unsigned int		caps;		/* server capabilities */
+	__u64			fattr_valid;	/* Valid attributes */
 	unsigned int		rsize;		/* read size */
 	unsigned int		rpages;		/* read size (in pages) */
 	unsigned int		wsize;		/* write size */
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 67f6632f723b..9cacbbd14787 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -45,7 +45,7 @@ struct nfs4_threshold {
 };
 
 struct nfs_fattr {
-	unsigned int		valid;		/* which fields are valid */
+	__u64			valid;		/* which fields are valid */
 	umode_t			mode;
 	__u32			nlink;
 	kuid_t			uid;
@@ -80,32 +80,32 @@ struct nfs_fattr {
 	struct nfs4_label	*label;
 };
 
-#define NFS_ATTR_FATTR_TYPE		(1U << 0)
-#define NFS_ATTR_FATTR_MODE		(1U << 1)
-#define NFS_ATTR_FATTR_NLINK		(1U << 2)
-#define NFS_ATTR_FATTR_OWNER		(1U << 3)
-#define NFS_ATTR_FATTR_GROUP		(1U << 4)
-#define NFS_ATTR_FATTR_RDEV		(1U << 5)
-#define NFS_ATTR_FATTR_SIZE		(1U << 6)
-#define NFS_ATTR_FATTR_PRESIZE		(1U << 7)
-#define NFS_ATTR_FATTR_BLOCKS_USED	(1U << 8)
-#define NFS_ATTR_FATTR_SPACE_USED	(1U << 9)
-#define NFS_ATTR_FATTR_FSID		(1U << 10)
-#define NFS_ATTR_FATTR_FILEID		(1U << 11)
-#define NFS_ATTR_FATTR_ATIME		(1U << 12)
-#define NFS_ATTR_FATTR_MTIME		(1U << 13)
-#define NFS_ATTR_FATTR_CTIME		(1U << 14)
-#define NFS_ATTR_FATTR_PREMTIME		(1U << 15)
-#define NFS_ATTR_FATTR_PRECTIME		(1U << 16)
-#define NFS_ATTR_FATTR_CHANGE		(1U << 17)
-#define NFS_ATTR_FATTR_PRECHANGE	(1U << 18)
-#define NFS_ATTR_FATTR_V4_LOCATIONS	(1U << 19)
-#define NFS_ATTR_FATTR_V4_REFERRAL	(1U << 20)
-#define NFS_ATTR_FATTR_MOUNTPOINT	(1U << 21)
-#define NFS_ATTR_FATTR_MOUNTED_ON_FILEID (1U << 22)
-#define NFS_ATTR_FATTR_OWNER_NAME	(1U << 23)
-#define NFS_ATTR_FATTR_GROUP_NAME	(1U << 24)
-#define NFS_ATTR_FATTR_V4_SECURITY_LABEL (1U << 25)
+#define NFS_ATTR_FATTR_TYPE		BIT_ULL(0)
+#define NFS_ATTR_FATTR_MODE		BIT_ULL(1)
+#define NFS_ATTR_FATTR_NLINK		BIT_ULL(2)
+#define NFS_ATTR_FATTR_OWNER		BIT_ULL(3)
+#define NFS_ATTR_FATTR_GROUP		BIT_ULL(4)
+#define NFS_ATTR_FATTR_RDEV		BIT_ULL(5)
+#define NFS_ATTR_FATTR_SIZE		BIT_ULL(6)
+#define NFS_ATTR_FATTR_PRESIZE		BIT_ULL(7)
+#define NFS_ATTR_FATTR_BLOCKS_USED	BIT_ULL(8)
+#define NFS_ATTR_FATTR_SPACE_USED	BIT_ULL(9)
+#define NFS_ATTR_FATTR_FSID		BIT_ULL(10)
+#define NFS_ATTR_FATTR_FILEID		BIT_ULL(11)
+#define NFS_ATTR_FATTR_ATIME		BIT_ULL(12)
+#define NFS_ATTR_FATTR_MTIME		BIT_ULL(13)
+#define NFS_ATTR_FATTR_CTIME		BIT_ULL(14)
+#define NFS_ATTR_FATTR_PREMTIME		BIT_ULL(15)
+#define NFS_ATTR_FATTR_PRECTIME		BIT_ULL(16)
+#define NFS_ATTR_FATTR_CHANGE		BIT_ULL(17)
+#define NFS_ATTR_FATTR_PRECHANGE	BIT_ULL(18)
+#define NFS_ATTR_FATTR_V4_LOCATIONS	BIT_ULL(19)
+#define NFS_ATTR_FATTR_V4_REFERRAL	BIT_ULL(20)
+#define NFS_ATTR_FATTR_MOUNTPOINT	BIT_ULL(21)
+#define NFS_ATTR_FATTR_MOUNTED_ON_FILEID BIT_ULL(22)
+#define NFS_ATTR_FATTR_OWNER_NAME	BIT_ULL(23)
+#define NFS_ATTR_FATTR_GROUP_NAME	BIT_ULL(24)
+#define NFS_ATTR_FATTR_V4_SECURITY_LABEL BIT_ULL(25)
 
 #define NFS_ATTR_FATTR (NFS_ATTR_FATTR_TYPE \
 		| NFS_ATTR_FATTR_MODE \
-- 
2.47.0

[PATCH v2 2/3] nfs: Add timecreate to nfs inode

[Benjamin Coddington]

From: Anne Marie Merritt <annemarie.merritt@primarydata.com>

Add tracking of the create time (a.k.a. btime) along with corresponding
bitfields, request, and decode xdr routines.

Signed-off-by: Anne Marie Merritt <annemarie.merritt@primarydata.com>
Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/inode.c          | 28 ++++++++++++++++++++++------
 fs/nfs/nfs4proc.c       | 14 +++++++++++++-
 fs/nfs/nfs4xdr.c        | 24 ++++++++++++++++++++++++
 fs/nfs/nfstrace.h       |  3 ++-
 include/linux/nfs_fs.h  |  7 +++++++
 include/linux/nfs_xdr.h |  3 +++
 6 files changed, 71 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 160f3478a835..fd84c24963b3 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -197,6 +197,7 @@ void nfs_set_cache_invalid(struct inode *inode, unsigned long flags)
 		if (!(flags & NFS_INO_REVAL_FORCED))
 			flags &= ~(NFS_INO_INVALID_MODE |
 				   NFS_INO_INVALID_OTHER |
+				   NFS_INO_INVALID_BTIME |
 				   NFS_INO_INVALID_XATTR);
 		flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
 	}
@@ -522,6 +523,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 		inode_set_atime(inode, 0, 0);
 		inode_set_mtime(inode, 0, 0);
 		inode_set_ctime(inode, 0, 0);
+		memset(&nfsi->btime, 0, sizeof(nfsi->btime));
 		inode_set_iversion_raw(inode, 0);
 		inode->i_size = 0;
 		clear_nlink(inode);
@@ -545,6 +547,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 			inode_set_ctime_to_ts(inode, fattr->ctime);
 		else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
 			nfs_set_cache_invalid(inode, NFS_INO_INVALID_CTIME);
+		if (fattr->valid & NFS_ATTR_FATTR_BTIME)
+			nfsi->btime = fattr->btime;
+		else if (fattr_supported & NFS_ATTR_FATTR_BTIME)
+			nfs_set_cache_invalid(inode, NFS_INO_INVALID_BTIME);
 		if (fattr->valid & NFS_ATTR_FATTR_CHANGE)
 			inode_set_iversion_raw(inode, fattr->change_attr);
 		else
@@ -1900,7 +1906,7 @@ static int nfs_inode_finish_partial_attr_update(const struct nfs_fattr *fattr,
 		NFS_INO_INVALID_ATIME | NFS_INO_INVALID_CTIME |
 		NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
 		NFS_INO_INVALID_BLOCKS | NFS_INO_INVALID_OTHER |
-		NFS_INO_INVALID_NLINK;
+		NFS_INO_INVALID_NLINK | NFS_INO_INVALID_BTIME;
 	unsigned long cache_validity = NFS_I(inode)->cache_validity;
 	enum nfs4_change_attr_type ctype = NFS_SERVER(inode)->change_attr_type;
 
@@ -2219,10 +2225,13 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 	nfs_fattr_fixup_delegated(inode, fattr);
 
 	save_cache_validity = nfsi->cache_validity;
-	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
-			| NFS_INO_INVALID_ATIME
-			| NFS_INO_REVAL_FORCED
-			| NFS_INO_INVALID_BLOCKS);
+	nfsi->cache_validity &=
+		~(NFS_INO_INVALID_ATIME | NFS_INO_REVAL_FORCED |
+		  NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_CTIME |
+		  NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
+		  NFS_INO_INVALID_OTHER | NFS_INO_INVALID_BLOCKS |
+		  NFS_INO_INVALID_NLINK | NFS_INO_INVALID_MODE |
+		  NFS_INO_INVALID_BTIME);
 
 	/* Do atomic weak cache consistency updates */
 	nfs_wcc_update_inode(inode, fattr);
@@ -2261,7 +2270,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 					| NFS_INO_INVALID_BLOCKS
 					| NFS_INO_INVALID_NLINK
 					| NFS_INO_INVALID_MODE
-					| NFS_INO_INVALID_OTHER;
+					| NFS_INO_INVALID_OTHER
+					| NFS_INO_INVALID_BTIME;
 				if (S_ISDIR(inode->i_mode))
 					nfs_force_lookup_revalidate(inode);
 				attr_changed = true;
@@ -2295,6 +2305,12 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 		nfsi->cache_validity |=
 			save_cache_validity & NFS_INO_INVALID_CTIME;
 
+	if (fattr->valid & NFS_ATTR_FATTR_BTIME)
+		nfsi->btime = fattr->btime;
+	else if (fattr_supported & NFS_ATTR_FATTR_BTIME)
+		nfsi->cache_validity |=
+			save_cache_validity & NFS_INO_INVALID_BTIME;
+
 	/* Check if our cached file size is stale */
 	if (fattr->valid & NFS_ATTR_FATTR_SIZE) {
 		new_isize = nfs_size_to_loff_t(fattr->size);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index b1d2122bd5a7..f7fb61f805a3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -222,6 +222,7 @@ const u32 nfs4_fattr_bitmap[3] = {
 	| FATTR4_WORD1_RAWDEV
 	| FATTR4_WORD1_SPACE_USED
 	| FATTR4_WORD1_TIME_ACCESS
+	| FATTR4_WORD1_TIME_CREATE
 	| FATTR4_WORD1_TIME_METADATA
 	| FATTR4_WORD1_TIME_MODIFY
 	| FATTR4_WORD1_MOUNTED_ON_FILEID,
@@ -243,6 +244,7 @@ static const u32 nfs4_pnfs_open_bitmap[3] = {
 	| FATTR4_WORD1_RAWDEV
 	| FATTR4_WORD1_SPACE_USED
 	| FATTR4_WORD1_TIME_ACCESS
+	| FATTR4_WORD1_TIME_CREATE
 	| FATTR4_WORD1_TIME_METADATA
 	| FATTR4_WORD1_TIME_MODIFY,
 	FATTR4_WORD2_MDSTHRESHOLD
@@ -323,6 +325,9 @@ static void nfs4_bitmap_copy_adjust(__u32 *dst, const __u32 *src,
 	if (!(cache_validity & NFS_INO_INVALID_OTHER))
 		dst[1] &= ~(FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP);
 
+	if (!(cache_validity & NFS_INO_INVALID_BTIME))
+		dst[1] &= ~FATTR4_WORD1_TIME_CREATE;
+
 	if (nfs_have_delegated_mtime(inode)) {
 		if (!(cache_validity & NFS_INO_INVALID_ATIME))
 			dst[1] &= ~FATTR4_WORD1_TIME_ACCESS;
@@ -1307,7 +1312,8 @@ nfs4_update_changeattr_locked(struct inode *inode,
 				NFS_INO_INVALID_ACCESS | NFS_INO_INVALID_ACL |
 				NFS_INO_INVALID_SIZE | NFS_INO_INVALID_OTHER |
 				NFS_INO_INVALID_BLOCKS | NFS_INO_INVALID_NLINK |
-				NFS_INO_INVALID_MODE | NFS_INO_INVALID_XATTR;
+				NFS_INO_INVALID_MODE | NFS_INO_INVALID_BTIME |
+				NFS_INO_INVALID_XATTR;
 		nfsi->attrtimeo = NFS_MINATTRTIMEO(inode);
 	}
 	nfsi->attrtimeo_timestamp = jiffies;
@@ -4047,6 +4053,10 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 			server->fattr_valid &= ~NFS_ATTR_FATTR_CTIME;
 		if (!(res.attr_bitmask[1] & FATTR4_WORD1_TIME_MODIFY))
 			server->fattr_valid &= ~NFS_ATTR_FATTR_MTIME;
+		if (!(res.attr_bitmask[1] & FATTR4_WORD1_TIME_MODIFY))
+			server->fattr_valid &= ~NFS_ATTR_FATTR_MTIME;
+		if (!(res.attr_bitmask[1] & FATTR4_WORD1_TIME_CREATE))
+			server->fattr_valid &= ~NFS_ATTR_FATTR_BTIME;
 		memcpy(server->attr_bitmask_nl, res.attr_bitmask,
 				sizeof(server->attr_bitmask));
 		server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
@@ -5773,6 +5783,8 @@ void nfs4_bitmask_set(__u32 bitmask[], const __u32 src[],
 		bitmask[1] |= FATTR4_WORD1_TIME_MODIFY;
 	if (cache_validity & NFS_INO_INVALID_BLOCKS)
 		bitmask[1] |= FATTR4_WORD1_SPACE_USED;
+	if (cache_validity & NFS_INO_INVALID_BTIME)
+		bitmask[1] |= FATTR4_WORD1_TIME_CREATE;
 
 	if (cache_validity & NFS_INO_INVALID_SIZE)
 		bitmask[0] |= FATTR4_WORD0_SIZE;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 55bef5fbfa47..f8d019c9d58d 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1623,6 +1623,7 @@ static void encode_readdir(struct xdr_stream *xdr, const struct nfs4_readdir_arg
 			| FATTR4_WORD1_RAWDEV
 			| FATTR4_WORD1_SPACE_USED
 			| FATTR4_WORD1_TIME_ACCESS
+			| FATTR4_WORD1_TIME_CREATE
 			| FATTR4_WORD1_TIME_METADATA
 			| FATTR4_WORD1_TIME_MODIFY;
 		attrs[2] |= FATTR4_WORD2_SECURITY_LABEL;
@@ -4207,6 +4208,24 @@ static int decode_attr_time_access(struct xdr_stream *xdr, uint32_t *bitmap, str
 	return status;
 }
 
+static int decode_attr_time_create(struct xdr_stream *xdr, uint32_t *bitmap, struct timespec64 *time)
+{
+	int status = 0;
+
+	time->tv_sec = 0;
+	time->tv_nsec = 0;
+	if (unlikely(bitmap[1] & (FATTR4_WORD1_TIME_CREATE - 1U)))
+		return -EIO;
+	if (likely(bitmap[1] & FATTR4_WORD1_TIME_CREATE)) {
+		status = decode_attr_time(xdr, time);
+		if (status == 0)
+			status = NFS_ATTR_FATTR_BTIME;
+		bitmap[1] &= ~FATTR4_WORD1_TIME_CREATE;
+	}
+	dprintk("%s: btime=%lld\n", __func__, time->tv_sec);
+	return status;
+}
+
 static int decode_attr_time_metadata(struct xdr_stream *xdr, uint32_t *bitmap, struct timespec64 *time)
 {
 	int status = 0;
@@ -4781,6 +4800,11 @@ static int decode_getfattr_attrs(struct xdr_stream *xdr, uint32_t *bitmap,
 		goto xdr_error;
 	fattr->valid |= status;
 
+	status = decode_attr_time_create(xdr, bitmap, &fattr->btime);
+	if (status < 0)
+		goto xdr_error;
+	fattr->valid |= status;
+
 	status = decode_attr_time_metadata(xdr, bitmap, &fattr->ctime);
 	if (status < 0)
 		goto xdr_error;
diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index 7a058bd8c566..f49f064c5ee5 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -32,7 +32,8 @@
 			{ NFS_INO_INVALID_BLOCKS, "INVALID_BLOCKS" }, \
 			{ NFS_INO_INVALID_XATTR, "INVALID_XATTR" }, \
 			{ NFS_INO_INVALID_NLINK, "INVALID_NLINK" }, \
-			{ NFS_INO_INVALID_MODE, "INVALID_MODE" })
+			{ NFS_INO_INVALID_MODE, "INVALID_MODE" }, \
+			{ NFS_INO_INVALID_BTIME, "INVALID_BTIME" })
 
 #define nfs_show_nfsi_flags(v) \
 	__print_flags(v, "|", \
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 67ae2c3f41d2..5cc5f7f02887 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -160,6 +160,12 @@ struct nfs_inode {
 	unsigned long		flags;			/* atomic bit ops */
 	unsigned long		cache_validity;		/* bit mask */
 
+	/*
+	 * NFS Attributes not included in struct inode
+	 */
+
+	struct timespec64	btime;
+
 	/*
 	 * read_cache_jiffies is when we started read-caching this inode.
 	 * attrtimeo is for how long the cached information is assumed
@@ -316,6 +322,7 @@ struct nfs4_copy_state {
 #define NFS_INO_INVALID_XATTR	BIT(15)		/* xattrs are invalid */
 #define NFS_INO_INVALID_NLINK	BIT(16)		/* cached nlinks is invalid */
 #define NFS_INO_INVALID_MODE	BIT(17)		/* cached mode is invalid */
+#define NFS_INO_INVALID_BTIME	BIT(18)		/* cached btime is invalid */
 
 #define NFS_INO_INVALID_ATTR	(NFS_INO_INVALID_CHANGE \
 		| NFS_INO_INVALID_CTIME \
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 9cacbbd14787..ac4bff6e9913 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -67,6 +67,7 @@ struct nfs_fattr {
 	struct timespec64	atime;
 	struct timespec64	mtime;
 	struct timespec64	ctime;
+	struct timespec64	btime;
 	__u64			change_attr;	/* NFSv4 change attribute */
 	__u64			pre_change_attr;/* pre-op NFSv4 change attribute */
 	__u64			pre_size;	/* pre_op_attr.size	  */
@@ -106,6 +107,7 @@ struct nfs_fattr {
 #define NFS_ATTR_FATTR_OWNER_NAME	BIT_ULL(23)
 #define NFS_ATTR_FATTR_GROUP_NAME	BIT_ULL(24)
 #define NFS_ATTR_FATTR_V4_SECURITY_LABEL BIT_ULL(25)
+#define NFS_ATTR_FATTR_BTIME		BIT_ULL(26)
 
 #define NFS_ATTR_FATTR (NFS_ATTR_FATTR_TYPE \
 		| NFS_ATTR_FATTR_MODE \
@@ -126,6 +128,7 @@ struct nfs_fattr {
 		| NFS_ATTR_FATTR_SPACE_USED)
 #define NFS_ATTR_FATTR_V4 (NFS_ATTR_FATTR \
 		| NFS_ATTR_FATTR_SPACE_USED \
+		| NFS_ATTR_FATTR_BTIME \
 		| NFS_ATTR_FATTR_V4_SECURITY_LABEL)
 
 /*
-- 
2.47.0

[PATCH v2 3/3] NFS: Return the file btime in the statx results when appropriate

[Benjamin Coddington]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

If the server supports the NFSv4.x "create_time" attribute, then return
it as part of the statx results.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/inode.c     | 15 +++++++++++++--
 fs/nfs/nfs4trace.h |  3 ++-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index fd84c24963b3..62b7b3c93577 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -894,6 +894,7 @@ static void nfs_readdirplus_parent_cache_hit(struct dentry *dentry)
 
 static u32 nfs_get_valid_attrmask(struct inode *inode)
 {
+	u64 fattr_valid = NFS_SERVER(inode)->fattr_valid;
 	unsigned long cache_validity = READ_ONCE(NFS_I(inode)->cache_validity);
 	u32 reply_mask = STATX_INO | STATX_TYPE;
 
@@ -913,6 +914,9 @@ static u32 nfs_get_valid_attrmask(struct inode *inode)
 		reply_mask |= STATX_UID | STATX_GID;
 	if (!(cache_validity & NFS_INO_INVALID_BLOCKS))
 		reply_mask |= STATX_BLOCKS;
+	if (!(cache_validity & NFS_INO_INVALID_BTIME) &&
+	    (fattr_valid & NFS_ATTR_FATTR_BTIME))
+		reply_mask |= STATX_BTIME;
 	if (!(cache_validity & NFS_INO_INVALID_CHANGE))
 		reply_mask |= STATX_CHANGE_COOKIE;
 	return reply_mask;
@@ -923,6 +927,7 @@ int nfs_getattr(struct mnt_idmap *idmap, const struct path *path,
 {
 	struct inode *inode = d_inode(path->dentry);
 	struct nfs_server *server = NFS_SERVER(inode);
+	u64 fattr_valid = server->fattr_valid;
 	unsigned long cache_validity;
 	int err = 0;
 	bool force_sync = query_flags & AT_STATX_FORCE_SYNC;
@@ -933,9 +938,12 @@ int nfs_getattr(struct mnt_idmap *idmap, const struct path *path,
 
 	request_mask &= STATX_TYPE | STATX_MODE | STATX_NLINK | STATX_UID |
 			STATX_GID | STATX_ATIME | STATX_MTIME | STATX_CTIME |
-			STATX_INO | STATX_SIZE | STATX_BLOCKS |
+			STATX_INO | STATX_SIZE | STATX_BLOCKS | STATX_BTIME |
 			STATX_CHANGE_COOKIE;
 
+	if (!(fattr_valid & NFS_ATTR_FATTR_BTIME))
+		request_mask &= ~STATX_BTIME;
+
 	if ((query_flags & AT_STATX_DONT_SYNC) && !force_sync) {
 		if (readdirplus_enabled)
 			nfs_readdirplus_parent_cache_hit(path->dentry);
@@ -967,7 +975,7 @@ int nfs_getattr(struct mnt_idmap *idmap, const struct path *path,
 	/* Is the user requesting attributes that might need revalidation? */
 	if (!(request_mask & (STATX_MODE|STATX_NLINK|STATX_ATIME|STATX_CTIME|
 					STATX_MTIME|STATX_UID|STATX_GID|
-					STATX_SIZE|STATX_BLOCKS|
+					STATX_SIZE|STATX_BLOCKS|STATX_BTIME|
 					STATX_CHANGE_COOKIE)))
 		goto out_no_revalidate;
 
@@ -991,6 +999,8 @@ int nfs_getattr(struct mnt_idmap *idmap, const struct path *path,
 		do_update |= cache_validity & NFS_INO_INVALID_OTHER;
 	if (request_mask & STATX_BLOCKS)
 		do_update |= cache_validity & NFS_INO_INVALID_BLOCKS;
+	if (request_mask & STATX_BTIME)
+		do_update |= cache_validity & NFS_INO_INVALID_BTIME;
 
 	if (do_update) {
 		if (readdirplus_enabled)
@@ -1012,6 +1022,7 @@ int nfs_getattr(struct mnt_idmap *idmap, const struct path *path,
 		stat->attributes |= STATX_ATTR_CHANGE_MONOTONIC;
 	if (S_ISDIR(inode->i_mode))
 		stat->blksize = NFS_SERVER(inode)->dtsize;
+	stat->btime = NFS_I(inode)->btime;
 out:
 	trace_nfs_getattr_exit(inode, err);
 	return err;
diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
index deab4c0e21a0..553e45502588 100644
--- a/fs/nfs/nfs4trace.h
+++ b/fs/nfs/nfs4trace.h
@@ -30,7 +30,8 @@
 		{ NFS_ATTR_FATTR_CTIME, "CTIME" }, \
 		{ NFS_ATTR_FATTR_CHANGE, "CHANGE" }, \
 		{ NFS_ATTR_FATTR_OWNER_NAME, "OWNER_NAME" }, \
-		{ NFS_ATTR_FATTR_GROUP_NAME, "GROUP_NAME" })
+		{ NFS_ATTR_FATTR_GROUP_NAME, "GROUP_NAME" }, \
+		{ NFS_ATTR_FATTR_BTIME, "BTIME" })
 
 DECLARE_EVENT_CLASS(nfs4_clientid_event,
 		TP_PROTO(
-- 
2.47.0

Re: [PATCH v2 2/3] nfs: Add timecreate to nfs inode

[Jeff Layton]

On Wed, 2025-05-28 at 08:50 -0400, Benjamin Coddington wrote:
> From: Anne Marie Merritt <annemarie.merritt@primarydata.com>
> 
> Add tracking of the create time (a.k.a. btime) along with corresponding
> bitfields, request, and decode xdr routines.
> 
> Signed-off-by: Anne Marie Merritt <annemarie.merritt@primarydata.com>
> Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  fs/nfs/inode.c          | 28 ++++++++++++++++++++++------
>  fs/nfs/nfs4proc.c       | 14 +++++++++++++-
>  fs/nfs/nfs4xdr.c        | 24 ++++++++++++++++++++++++
>  fs/nfs/nfstrace.h       |  3 ++-
>  include/linux/nfs_fs.h  |  7 +++++++
>  include/linux/nfs_xdr.h |  3 +++
>  6 files changed, 71 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 160f3478a835..fd84c24963b3 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -197,6 +197,7 @@ void nfs_set_cache_invalid(struct inode *inode, unsigned long flags)
>  		if (!(flags & NFS_INO_REVAL_FORCED))
>  			flags &= ~(NFS_INO_INVALID_MODE |
>  				   NFS_INO_INVALID_OTHER |
> +				   NFS_INO_INVALID_BTIME |
>  				   NFS_INO_INVALID_XATTR);
>  		flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
>  	}
> @@ -522,6 +523,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>  		inode_set_atime(inode, 0, 0);
>  		inode_set_mtime(inode, 0, 0);
>  		inode_set_ctime(inode, 0, 0);
> +		memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>  		inode_set_iversion_raw(inode, 0);
>  		inode->i_size = 0;
>  		clear_nlink(inode);
> @@ -545,6 +547,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>  			inode_set_ctime_to_ts(inode, fattr->ctime);
>  		else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
>  			nfs_set_cache_invalid(inode, NFS_INO_INVALID_CTIME);
> +		if (fattr->valid & NFS_ATTR_FATTR_BTIME)
> +			nfsi->btime = fattr->btime;
> +		else if (fattr_supported & NFS_ATTR_FATTR_BTIME)
> +			nfs_set_cache_invalid(inode, NFS_INO_INVALID_BTIME);
>  		if (fattr->valid & NFS_ATTR_FATTR_CHANGE)
>  			inode_set_iversion_raw(inode, fattr->change_attr);
>  		else
> @@ -1900,7 +1906,7 @@ static int nfs_inode_finish_partial_attr_update(const struct nfs_fattr *fattr,
>  		NFS_INO_INVALID_ATIME | NFS_INO_INVALID_CTIME |
>  		NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
>  		NFS_INO_INVALID_BLOCKS | NFS_INO_INVALID_OTHER |
> -		NFS_INO_INVALID_NLINK;
> +		NFS_INO_INVALID_NLINK | NFS_INO_INVALID_BTIME;
>  	unsigned long cache_validity = NFS_I(inode)->cache_validity;
>  	enum nfs4_change_attr_type ctype = NFS_SERVER(inode)->change_attr_type;
>  
> @@ -2219,10 +2225,13 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  	nfs_fattr_fixup_delegated(inode, fattr);
>  
>  	save_cache_validity = nfsi->cache_validity;
> -	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
> -			| NFS_INO_INVALID_ATIME
> -			| NFS_INO_REVAL_FORCED
> -			| NFS_INO_INVALID_BLOCKS);
> +	nfsi->cache_validity &=
> +		~(NFS_INO_INVALID_ATIME | NFS_INO_REVAL_FORCED |
> +		  NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_CTIME |
> +		  NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
> +		  NFS_INO_INVALID_OTHER | NFS_INO_INVALID_BLOCKS |
> +		  NFS_INO_INVALID_NLINK | NFS_INO_INVALID_MODE |
> +		  NFS_INO_INVALID_BTIME);
>  

The delta above is a little curious. This patch is just adding
NFS_INO_INVALID_BTIME, but the patch above adds the clearing of several
other bits as well. Why does this logic change?

>  	/* Do atomic weak cache consistency updates */
>  	nfs_wcc_update_inode(inode, fattr);
> @@ -2261,7 +2270,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  					| NFS_INO_INVALID_BLOCKS
>  					| NFS_INO_INVALID_NLINK
>  					| NFS_INO_INVALID_MODE
> -					| NFS_INO_INVALID_OTHER;
> +					| NFS_INO_INVALID_OTHER
> +					| NFS_INO_INVALID_BTIME;
>  				if (S_ISDIR(inode->i_mode))
>  					nfs_force_lookup_revalidate(inode);
>  				attr_changed = true;
> @@ -2295,6 +2305,12 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  		nfsi->cache_validity |=
>  			save_cache_validity & NFS_INO_INVALID_CTIME;
>  
> +	if (fattr->valid & NFS_ATTR_FATTR_BTIME)
> +		nfsi->btime = fattr->btime;
> +	else if (fattr_supported & NFS_ATTR_FATTR_BTIME)
> +		nfsi->cache_validity |=
> +			save_cache_validity & NFS_INO_INVALID_BTIME;
> +
>  	/* Check if our cached file size is stale */
>  	if (fattr->valid & NFS_ATTR_FATTR_SIZE) {
>  		new_isize = nfs_size_to_loff_t(fattr->size);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index b1d2122bd5a7..f7fb61f805a3 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -222,6 +222,7 @@ const u32 nfs4_fattr_bitmap[3] = {
>  	| FATTR4_WORD1_RAWDEV
>  	| FATTR4_WORD1_SPACE_USED
>  	| FATTR4_WORD1_TIME_ACCESS
> +	| FATTR4_WORD1_TIME_CREATE
>  	| FATTR4_WORD1_TIME_METADATA
>  	| FATTR4_WORD1_TIME_MODIFY
>  	| FATTR4_WORD1_MOUNTED_ON_FILEID,
> @@ -243,6 +244,7 @@ static const u32 nfs4_pnfs_open_bitmap[3] = {
>  	| FATTR4_WORD1_RAWDEV
>  	| FATTR4_WORD1_SPACE_USED
>  	| FATTR4_WORD1_TIME_ACCESS
> +	| FATTR4_WORD1_TIME_CREATE
>  	| FATTR4_WORD1_TIME_METADATA
>  	| FATTR4_WORD1_TIME_MODIFY,
>  	FATTR4_WORD2_MDSTHRESHOLD
> @@ -323,6 +325,9 @@ static void nfs4_bitmap_copy_adjust(__u32 *dst, const __u32 *src,
>  	if (!(cache_validity & NFS_INO_INVALID_OTHER))
>  		dst[1] &= ~(FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP);
>  
> +	if (!(cache_validity & NFS_INO_INVALID_BTIME))
> +		dst[1] &= ~FATTR4_WORD1_TIME_CREATE;
> +
>  	if (nfs_have_delegated_mtime(inode)) {
>  		if (!(cache_validity & NFS_INO_INVALID_ATIME))
>  			dst[1] &= ~FATTR4_WORD1_TIME_ACCESS;
> @@ -1307,7 +1312,8 @@ nfs4_update_changeattr_locked(struct inode *inode,
>  				NFS_INO_INVALID_ACCESS | NFS_INO_INVALID_ACL |
>  				NFS_INO_INVALID_SIZE | NFS_INO_INVALID_OTHER |
>  				NFS_INO_INVALID_BLOCKS | NFS_INO_INVALID_NLINK |
> -				NFS_INO_INVALID_MODE | NFS_INO_INVALID_XATTR;
> +				NFS_INO_INVALID_MODE | NFS_INO_INVALID_BTIME |
> +				NFS_INO_INVALID_XATTR;
>  		nfsi->attrtimeo = NFS_MINATTRTIMEO(inode);
>  	}
>  	nfsi->attrtimeo_timestamp = jiffies;
> @@ -4047,6 +4053,10 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
>  			server->fattr_valid &= ~NFS_ATTR_FATTR_CTIME;
>  		if (!(res.attr_bitmask[1] & FATTR4_WORD1_TIME_MODIFY))
>  			server->fattr_valid &= ~NFS_ATTR_FATTR_MTIME;
> +		if (!(res.attr_bitmask[1] & FATTR4_WORD1_TIME_MODIFY))
> +			server->fattr_valid &= ~NFS_ATTR_FATTR_MTIME;
> +		if (!(res.attr_bitmask[1] & FATTR4_WORD1_TIME_CREATE))
> +			server->fattr_valid &= ~NFS_ATTR_FATTR_BTIME;
>  		memcpy(server->attr_bitmask_nl, res.attr_bitmask,
>  				sizeof(server->attr_bitmask));
>  		server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
> @@ -5773,6 +5783,8 @@ void nfs4_bitmask_set(__u32 bitmask[], const __u32 src[],
>  		bitmask[1] |= FATTR4_WORD1_TIME_MODIFY;
>  	if (cache_validity & NFS_INO_INVALID_BLOCKS)
>  		bitmask[1] |= FATTR4_WORD1_SPACE_USED;
> +	if (cache_validity & NFS_INO_INVALID_BTIME)
> +		bitmask[1] |= FATTR4_WORD1_TIME_CREATE;
>  
>  	if (cache_validity & NFS_INO_INVALID_SIZE)
>  		bitmask[0] |= FATTR4_WORD0_SIZE;
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 55bef5fbfa47..f8d019c9d58d 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -1623,6 +1623,7 @@ static void encode_readdir(struct xdr_stream *xdr, const struct nfs4_readdir_arg
>  			| FATTR4_WORD1_RAWDEV
>  			| FATTR4_WORD1_SPACE_USED
>  			| FATTR4_WORD1_TIME_ACCESS
> +			| FATTR4_WORD1_TIME_CREATE
>  			| FATTR4_WORD1_TIME_METADATA
>  			| FATTR4_WORD1_TIME_MODIFY;
>  		attrs[2] |= FATTR4_WORD2_SECURITY_LABEL;
> @@ -4207,6 +4208,24 @@ static int decode_attr_time_access(struct xdr_stream *xdr, uint32_t *bitmap, str
>  	return status;
>  }
>  
> +static int decode_attr_time_create(struct xdr_stream *xdr, uint32_t *bitmap, struct timespec64 *time)
> +{
> +	int status = 0;
> +
> +	time->tv_sec = 0;
> +	time->tv_nsec = 0;
> +	if (unlikely(bitmap[1] & (FATTR4_WORD1_TIME_CREATE - 1U)))
> +		return -EIO;
> +	if (likely(bitmap[1] & FATTR4_WORD1_TIME_CREATE)) {
> +		status = decode_attr_time(xdr, time);
> +		if (status == 0)
> +			status = NFS_ATTR_FATTR_BTIME;
> +		bitmap[1] &= ~FATTR4_WORD1_TIME_CREATE;
> +	}
> +	dprintk("%s: btime=%lld\n", __func__, time->tv_sec);
> +	return status;
> +}
> +
>  static int decode_attr_time_metadata(struct xdr_stream *xdr, uint32_t *bitmap, struct timespec64 *time)
>  {
>  	int status = 0;
> @@ -4781,6 +4800,11 @@ static int decode_getfattr_attrs(struct xdr_stream *xdr, uint32_t *bitmap,
>  		goto xdr_error;
>  	fattr->valid |= status;
>  
> +	status = decode_attr_time_create(xdr, bitmap, &fattr->btime);
> +	if (status < 0)
> +		goto xdr_error;
> +	fattr->valid |= status;
> +
>  	status = decode_attr_time_metadata(xdr, bitmap, &fattr->ctime);
>  	if (status < 0)
>  		goto xdr_error;
> diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
> index 7a058bd8c566..f49f064c5ee5 100644
> --- a/fs/nfs/nfstrace.h
> +++ b/fs/nfs/nfstrace.h
> @@ -32,7 +32,8 @@
>  			{ NFS_INO_INVALID_BLOCKS, "INVALID_BLOCKS" }, \
>  			{ NFS_INO_INVALID_XATTR, "INVALID_XATTR" }, \
>  			{ NFS_INO_INVALID_NLINK, "INVALID_NLINK" }, \
> -			{ NFS_INO_INVALID_MODE, "INVALID_MODE" })
> +			{ NFS_INO_INVALID_MODE, "INVALID_MODE" }, \
> +			{ NFS_INO_INVALID_BTIME, "INVALID_BTIME" })
>  
>  #define nfs_show_nfsi_flags(v) \
>  	__print_flags(v, "|", \
> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index 67ae2c3f41d2..5cc5f7f02887 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -160,6 +160,12 @@ struct nfs_inode {
>  	unsigned long		flags;			/* atomic bit ops */
>  	unsigned long		cache_validity;		/* bit mask */
>  
> +	/*
> +	 * NFS Attributes not included in struct inode
> +	 */
> +
> +	struct timespec64	btime;
> +
>  	/*
>  	 * read_cache_jiffies is when we started read-caching this inode.
>  	 * attrtimeo is for how long the cached information is assumed
> @@ -316,6 +322,7 @@ struct nfs4_copy_state {
>  #define NFS_INO_INVALID_XATTR	BIT(15)		/* xattrs are invalid */
>  #define NFS_INO_INVALID_NLINK	BIT(16)		/* cached nlinks is invalid */
>  #define NFS_INO_INVALID_MODE	BIT(17)		/* cached mode is invalid */
> +#define NFS_INO_INVALID_BTIME	BIT(18)		/* cached btime is invalid */
>  
>  #define NFS_INO_INVALID_ATTR	(NFS_INO_INVALID_CHANGE \
>  		| NFS_INO_INVALID_CTIME \
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 9cacbbd14787..ac4bff6e9913 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -67,6 +67,7 @@ struct nfs_fattr {
>  	struct timespec64	atime;
>  	struct timespec64	mtime;
>  	struct timespec64	ctime;
> +	struct timespec64	btime;
>  	__u64			change_attr;	/* NFSv4 change attribute */
>  	__u64			pre_change_attr;/* pre-op NFSv4 change attribute */
>  	__u64			pre_size;	/* pre_op_attr.size	  */
> @@ -106,6 +107,7 @@ struct nfs_fattr {
>  #define NFS_ATTR_FATTR_OWNER_NAME	BIT_ULL(23)
>  #define NFS_ATTR_FATTR_GROUP_NAME	BIT_ULL(24)
>  #define NFS_ATTR_FATTR_V4_SECURITY_LABEL BIT_ULL(25)
> +#define NFS_ATTR_FATTR_BTIME		BIT_ULL(26)
>  
>  #define NFS_ATTR_FATTR (NFS_ATTR_FATTR_TYPE \
>  		| NFS_ATTR_FATTR_MODE \
> @@ -126,6 +128,7 @@ struct nfs_fattr {
>  		| NFS_ATTR_FATTR_SPACE_USED)
>  #define NFS_ATTR_FATTR_V4 (NFS_ATTR_FATTR \
>  		| NFS_ATTR_FATTR_SPACE_USED \
> +		| NFS_ATTR_FATTR_BTIME \
>  		| NFS_ATTR_FATTR_V4_SECURITY_LABEL)
>  
>  /*

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 2/3] nfs: Add timecreate to nfs inode

[Benjamin Coddington]

On 28 May 2025, at 8:56, Jeff Layton wrote:

> On Wed, 2025-05-28 at 08:50 -0400, Benjamin Coddington wrote:
>> From: Anne Marie Merritt <annemarie.merritt@primarydata.com>
>>
>> Add tracking of the create time (a.k.a. btime) along with corresponding
>> bitfields, request, and decode xdr routines.
>>
>> Signed-off-by: Anne Marie Merritt <annemarie.merritt@primarydata.com>
>> Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com>
>> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
>> ---
>>  fs/nfs/inode.c          | 28 ++++++++++++++++++++++------
>>  fs/nfs/nfs4proc.c       | 14 +++++++++++++-
>>  fs/nfs/nfs4xdr.c        | 24 ++++++++++++++++++++++++
>>  fs/nfs/nfstrace.h       |  3 ++-
>>  include/linux/nfs_fs.h  |  7 +++++++
>>  include/linux/nfs_xdr.h |  3 +++
>>  6 files changed, 71 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>> index 160f3478a835..fd84c24963b3 100644
>> --- a/fs/nfs/inode.c
>> +++ b/fs/nfs/inode.c
>> @@ -197,6 +197,7 @@ void nfs_set_cache_invalid(struct inode *inode, unsigned long flags)
>>  		if (!(flags & NFS_INO_REVAL_FORCED))
>>  			flags &= ~(NFS_INO_INVALID_MODE |
>>  				   NFS_INO_INVALID_OTHER |
>> +				   NFS_INO_INVALID_BTIME |
>>  				   NFS_INO_INVALID_XATTR);
>>  		flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
>>  	}
>> @@ -522,6 +523,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>>  		inode_set_atime(inode, 0, 0);
>>  		inode_set_mtime(inode, 0, 0);
>>  		inode_set_ctime(inode, 0, 0);
>> +		memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>>  		inode_set_iversion_raw(inode, 0);
>>  		inode->i_size = 0;
>>  		clear_nlink(inode);
>> @@ -545,6 +547,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>>  			inode_set_ctime_to_ts(inode, fattr->ctime);
>>  		else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
>>  			nfs_set_cache_invalid(inode, NFS_INO_INVALID_CTIME);
>> +		if (fattr->valid & NFS_ATTR_FATTR_BTIME)
>> +			nfsi->btime = fattr->btime;
>> +		else if (fattr_supported & NFS_ATTR_FATTR_BTIME)
>> +			nfs_set_cache_invalid(inode, NFS_INO_INVALID_BTIME);
>>  		if (fattr->valid & NFS_ATTR_FATTR_CHANGE)
>>  			inode_set_iversion_raw(inode, fattr->change_attr);
>>  		else
>> @@ -1900,7 +1906,7 @@ static int nfs_inode_finish_partial_attr_update(const struct nfs_fattr *fattr,
>>  		NFS_INO_INVALID_ATIME | NFS_INO_INVALID_CTIME |
>>  		NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
>>  		NFS_INO_INVALID_BLOCKS | NFS_INO_INVALID_OTHER |
>> -		NFS_INO_INVALID_NLINK;
>> +		NFS_INO_INVALID_NLINK | NFS_INO_INVALID_BTIME;
>>  	unsigned long cache_validity = NFS_I(inode)->cache_validity;
>>  	enum nfs4_change_attr_type ctype = NFS_SERVER(inode)->change_attr_type;
>>
>> @@ -2219,10 +2225,13 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>>  	nfs_fattr_fixup_delegated(inode, fattr);
>>
>>  	save_cache_validity = nfsi->cache_validity;
>> -	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
>> -			| NFS_INO_INVALID_ATIME
>> -			| NFS_INO_REVAL_FORCED
>> -			| NFS_INO_INVALID_BLOCKS);
>> +	nfsi->cache_validity &=
>> +		~(NFS_INO_INVALID_ATIME | NFS_INO_REVAL_FORCED |
>> +		  NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_CTIME |
>> +		  NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
>> +		  NFS_INO_INVALID_OTHER | NFS_INO_INVALID_BLOCKS |
>> +		  NFS_INO_INVALID_NLINK | NFS_INO_INVALID_MODE |
>> +		  NFS_INO_INVALID_BTIME);
>>
>
> The delta above is a little curious. This patch is just adding
> NFS_INO_INVALID_BTIME, but the patch above adds the clearing of several
> other bits as well. Why does this logic change?

Good point, I wonder if it was based on other attribute work at the time,
the original was here:
https://lore.kernel.org/linux-nfs/20211227190504.309612-3-trondmy@kernel.org/

So I think what we're doing here is clearing what we know we don't have to
check/update - I think we can drop this entire hunk, its a minor
optimization, but hopefully we can get some expert opinion.   Trond, do you
want me to test with this hunk removed?

Ben

Re: [PATCH v2 2/3] nfs: Add timecreate to nfs inode

[Jeff Layton]

On Wed, 2025-05-28 at 11:13 -0400, Benjamin Coddington wrote:
> On 28 May 2025, at 8:56, Jeff Layton wrote:
> 
> > On Wed, 2025-05-28 at 08:50 -0400, Benjamin Coddington wrote:
> > > From: Anne Marie Merritt <annemarie.merritt@primarydata.com>
> > > 
> > > Add tracking of the create time (a.k.a. btime) along with corresponding
> > > bitfields, request, and decode xdr routines.
> > > 
> > > Signed-off-by: Anne Marie Merritt <annemarie.merritt@primarydata.com>
> > > Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com>
> > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > ---
> > >  fs/nfs/inode.c          | 28 ++++++++++++++++++++++------
> > >  fs/nfs/nfs4proc.c       | 14 +++++++++++++-
> > >  fs/nfs/nfs4xdr.c        | 24 ++++++++++++++++++++++++
> > >  fs/nfs/nfstrace.h       |  3 ++-
> > >  include/linux/nfs_fs.h  |  7 +++++++
> > >  include/linux/nfs_xdr.h |  3 +++
> > >  6 files changed, 71 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > > index 160f3478a835..fd84c24963b3 100644
> > > --- a/fs/nfs/inode.c
> > > +++ b/fs/nfs/inode.c
> > > @@ -197,6 +197,7 @@ void nfs_set_cache_invalid(struct inode *inode, unsigned long flags)
> > >  		if (!(flags & NFS_INO_REVAL_FORCED))
> > >  			flags &= ~(NFS_INO_INVALID_MODE |
> > >  				   NFS_INO_INVALID_OTHER |
> > > +				   NFS_INO_INVALID_BTIME |
> > >  				   NFS_INO_INVALID_XATTR);
> > >  		flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
> > >  	}
> > > @@ -522,6 +523,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
> > >  		inode_set_atime(inode, 0, 0);
> > >  		inode_set_mtime(inode, 0, 0);
> > >  		inode_set_ctime(inode, 0, 0);
> > > +		memset(&nfsi->btime, 0, sizeof(nfsi->btime));
> > >  		inode_set_iversion_raw(inode, 0);
> > >  		inode->i_size = 0;
> > >  		clear_nlink(inode);
> > > @@ -545,6 +547,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
> > >  			inode_set_ctime_to_ts(inode, fattr->ctime);
> > >  		else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
> > >  			nfs_set_cache_invalid(inode, NFS_INO_INVALID_CTIME);
> > > +		if (fattr->valid & NFS_ATTR_FATTR_BTIME)
> > > +			nfsi->btime = fattr->btime;
> > > +		else if (fattr_supported & NFS_ATTR_FATTR_BTIME)
> > > +			nfs_set_cache_invalid(inode, NFS_INO_INVALID_BTIME);
> > >  		if (fattr->valid & NFS_ATTR_FATTR_CHANGE)
> > >  			inode_set_iversion_raw(inode, fattr->change_attr);
> > >  		else
> > > @@ -1900,7 +1906,7 @@ static int nfs_inode_finish_partial_attr_update(const struct nfs_fattr *fattr,
> > >  		NFS_INO_INVALID_ATIME | NFS_INO_INVALID_CTIME |
> > >  		NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
> > >  		NFS_INO_INVALID_BLOCKS | NFS_INO_INVALID_OTHER |
> > > -		NFS_INO_INVALID_NLINK;
> > > +		NFS_INO_INVALID_NLINK | NFS_INO_INVALID_BTIME;
> > >  	unsigned long cache_validity = NFS_I(inode)->cache_validity;
> > >  	enum nfs4_change_attr_type ctype = NFS_SERVER(inode)->change_attr_type;
> > > 
> > > @@ -2219,10 +2225,13 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
> > >  	nfs_fattr_fixup_delegated(inode, fattr);
> > > 
> > >  	save_cache_validity = nfsi->cache_validity;
> > > -	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
> > > -			| NFS_INO_INVALID_ATIME
> > > -			| NFS_INO_REVAL_FORCED
> > > -			| NFS_INO_INVALID_BLOCKS);
> > > +	nfsi->cache_validity &=
> > > +		~(NFS_INO_INVALID_ATIME | NFS_INO_REVAL_FORCED |
> > > +		  NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_CTIME |
> > > +		  NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
> > > +		  NFS_INO_INVALID_OTHER | NFS_INO_INVALID_BLOCKS |
> > > +		  NFS_INO_INVALID_NLINK | NFS_INO_INVALID_MODE |
> > > +		  NFS_INO_INVALID_BTIME);
> > > 
> > 
> > The delta above is a little curious. This patch is just adding
> > NFS_INO_INVALID_BTIME, but the patch above adds the clearing of several
> > other bits as well. Why does this logic change?
> 
> Good point, I wonder if it was based on other attribute work at the time,
> the original was here:
> https://lore.kernel.org/linux-nfs/20211227190504.309612-3-trondmy@kernel.org/
> 
> So I think what we're doing here is clearing what we know we don't have to
> check/update - I think we can drop this entire hunk, its a minor
> optimization, but hopefully we can get some expert opinion.   Trond, do you
> want me to test with this hunk removed?

Looking little closer, I think what happened there is that
NFS_INO_INVALID_ATTR got unrolled into individual attr flags. So there
may be no net change here (other than adding BTIME).

Still, it'd be best to do that in a separate patch, IMO, esp. since
this is in nfs_update_inode(), which is already so "fiddly".
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 2/3] nfs: Add timecreate to nfs inode

[Trond Myklebust]

On Wed, 2025-05-28 at 11:27 -0400, Jeff Layton wrote:
> On Wed, 2025-05-28 at 11:13 -0400, Benjamin Coddington wrote:
> > On 28 May 2025, at 8:56, Jeff Layton wrote:
> > 
> > > On Wed, 2025-05-28 at 08:50 -0400, Benjamin Coddington wrote:
> > > > From: Anne Marie Merritt <annemarie.merritt@primarydata.com>
> > > > 
> > > > Add tracking of the create time (a.k.a. btime) along with
> > > > corresponding
> > > > bitfields, request, and decode xdr routines.
> > > > 
> > > > Signed-off-by: Anne Marie Merritt
> > > > <annemarie.merritt@primarydata.com>
> > > > Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com>
> > > > Signed-off-by: Trond Myklebust
> > > > <trond.myklebust@hammerspace.com>
> > > > ---
> > > >  fs/nfs/inode.c          | 28 ++++++++++++++++++++++------
> > > >  fs/nfs/nfs4proc.c       | 14 +++++++++++++-
> > > >  fs/nfs/nfs4xdr.c        | 24 ++++++++++++++++++++++++
> > > >  fs/nfs/nfstrace.h       |  3 ++-
> > > >  include/linux/nfs_fs.h  |  7 +++++++
> > > >  include/linux/nfs_xdr.h |  3 +++
> > > >  6 files changed, 71 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > > > index 160f3478a835..fd84c24963b3 100644
> > > > --- a/fs/nfs/inode.c
> > > > +++ b/fs/nfs/inode.c
> > > > @@ -197,6 +197,7 @@ void nfs_set_cache_invalid(struct inode
> > > > *inode, unsigned long flags)
> > > >  		if (!(flags & NFS_INO_REVAL_FORCED))
> > > >  			flags &= ~(NFS_INO_INVALID_MODE |
> > > >  				   NFS_INO_INVALID_OTHER |
> > > > +				   NFS_INO_INVALID_BTIME |
> > > >  				   NFS_INO_INVALID_XATTR);
> > > >  		flags &= ~(NFS_INO_INVALID_CHANGE |
> > > > NFS_INO_INVALID_SIZE);
> > > >  	}
> > > > @@ -522,6 +523,7 @@ nfs_fhget(struct super_block *sb, struct
> > > > nfs_fh *fh, struct nfs_fattr *fattr)
> > > >  		inode_set_atime(inode, 0, 0);
> > > >  		inode_set_mtime(inode, 0, 0);
> > > >  		inode_set_ctime(inode, 0, 0);
> > > > +		memset(&nfsi->btime, 0, sizeof(nfsi->btime));
> > > >  		inode_set_iversion_raw(inode, 0);
> > > >  		inode->i_size = 0;
> > > >  		clear_nlink(inode);
> > > > @@ -545,6 +547,10 @@ nfs_fhget(struct super_block *sb, struct
> > > > nfs_fh *fh, struct nfs_fattr *fattr)
> > > >  			inode_set_ctime_to_ts(inode, fattr-
> > > > >ctime);
> > > >  		else if (fattr_supported &
> > > > NFS_ATTR_FATTR_CTIME)
> > > >  			nfs_set_cache_invalid(inode,
> > > > NFS_INO_INVALID_CTIME);
> > > > +		if (fattr->valid & NFS_ATTR_FATTR_BTIME)
> > > > +			nfsi->btime = fattr->btime;
> > > > +		else if (fattr_supported &
> > > > NFS_ATTR_FATTR_BTIME)
> > > > +			nfs_set_cache_invalid(inode,
> > > > NFS_INO_INVALID_BTIME);
> > > >  		if (fattr->valid & NFS_ATTR_FATTR_CHANGE)
> > > >  			inode_set_iversion_raw(inode, fattr-
> > > > >change_attr);
> > > >  		else
> > > > @@ -1900,7 +1906,7 @@ static int
> > > > nfs_inode_finish_partial_attr_update(const struct nfs_fattr
> > > > *fattr,
> > > >  		NFS_INO_INVALID_ATIME | NFS_INO_INVALID_CTIME
> > > > |
> > > >  		NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
> > > >  		NFS_INO_INVALID_BLOCKS | NFS_INO_INVALID_OTHER
> > > > |
> > > > -		NFS_INO_INVALID_NLINK;
> > > > +		NFS_INO_INVALID_NLINK | NFS_INO_INVALID_BTIME;
> > > >  	unsigned long cache_validity = NFS_I(inode)-
> > > > >cache_validity;
> > > >  	enum nfs4_change_attr_type ctype = NFS_SERVER(inode)-
> > > > >change_attr_type;
> > > > 
> > > > @@ -2219,10 +2225,13 @@ static int nfs_update_inode(struct
> > > > inode *inode, struct nfs_fattr *fattr)
> > > >  	nfs_fattr_fixup_delegated(inode, fattr);
> > > > 
> > > >  	save_cache_validity = nfsi->cache_validity;
> > > > -	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
> > > > -			| NFS_INO_INVALID_ATIME
> > > > -			| NFS_INO_REVAL_FORCED
> > > > -			| NFS_INO_INVALID_BLOCKS);
> > > > +	nfsi->cache_validity &=
> > > > +		~(NFS_INO_INVALID_ATIME | NFS_INO_REVAL_FORCED
> > > > |
> > > > +		  NFS_INO_INVALID_CHANGE |
> > > > NFS_INO_INVALID_CTIME |
> > > > +		  NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE
> > > > |
> > > > +		  NFS_INO_INVALID_OTHER |
> > > > NFS_INO_INVALID_BLOCKS |
> > > > +		  NFS_INO_INVALID_NLINK | NFS_INO_INVALID_MODE
> > > > |
> > > > +		  NFS_INO_INVALID_BTIME);
> > > > 
> > > 
> > > The delta above is a little curious. This patch is just adding
> > > NFS_INO_INVALID_BTIME, but the patch above adds the clearing of
> > > several
> > > other bits as well. Why does this logic change?
> > 
> > Good point, I wonder if it was based on other attribute work at the
> > time,
> > the original was here:
> > https://lore.kernel.org/linux-nfs/20211227190504.309612-3-trondmy@kernel.org/
> > 
> > So I think what we're doing here is clearing what we know we don't
> > have to
> > check/update - I think we can drop this entire hunk, its a minor
> > optimization, but hopefully we can get some expert opinion.  
> > Trond, do you
> > want me to test with this hunk removed?
> 
> Looking little closer, I think what happened there is that
> NFS_INO_INVALID_ATTR got unrolled into individual attr flags. So
> there
> may be no net change here (other than adding BTIME).
> 
> Still, it'd be best to do that in a separate patch, IMO, esp. since
> this is in nfs_update_inode(), which is already so "fiddly".

NFS_INO_INVALID_ATTR is a dinosaur that I'd like to see extinct. We
have fine grained attribute tracking these days, and so keeping the old
coarse grained one around is just confusing.

That said, I'm fine with this being a separate patch.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Re: [PATCH v2 2/3] nfs: Add timecreate to nfs inode

[Benjamin Coddington]

On 28 May 2025, at 16:16, Trond Myklebust wrote:

> On Wed, 2025-05-28 at 11:27 -0400, Jeff Layton wrote:
>> On Wed, 2025-05-28 at 11:13 -0400, Benjamin Coddington wrote:
>>> On 28 May 2025, at 8:56, Jeff Layton wrote:
>>>
>>>> On Wed, 2025-05-28 at 08:50 -0400, Benjamin Coddington wrote:
>>>>> From: Anne Marie Merritt <annemarie.merritt@primarydata.com>
>>>>>
>>>>> Add tracking of the create time (a.k.a. btime) along with
>>>>> corresponding
>>>>> bitfields, request, and decode xdr routines.
>>>>>
>>>>> Signed-off-by: Anne Marie Merritt
>>>>> <annemarie.merritt@primarydata.com>
>>>>> Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com>
>>>>> Signed-off-by: Trond Myklebust
>>>>> <trond.myklebust@hammerspace.com>
>>>>> ---
>>>>>  fs/nfs/inode.c          | 28 ++++++++++++++++++++++------
>>>>>  fs/nfs/nfs4proc.c       | 14 +++++++++++++-
>>>>>  fs/nfs/nfs4xdr.c        | 24 ++++++++++++++++++++++++
>>>>>  fs/nfs/nfstrace.h       |  3 ++-
>>>>>  include/linux/nfs_fs.h  |  7 +++++++
>>>>>  include/linux/nfs_xdr.h |  3 +++
>>>>>  6 files changed, 71 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>>>>> index 160f3478a835..fd84c24963b3 100644
>>>>> --- a/fs/nfs/inode.c
>>>>> +++ b/fs/nfs/inode.c
>>>>> @@ -197,6 +197,7 @@ void nfs_set_cache_invalid(struct inode
>>>>> *inode, unsigned long flags)
>>>>>  		if (!(flags & NFS_INO_REVAL_FORCED))
>>>>>  			flags &= ~(NFS_INO_INVALID_MODE |
>>>>>  				   NFS_INO_INVALID_OTHER |
>>>>> +				   NFS_INO_INVALID_BTIME |
>>>>>  				   NFS_INO_INVALID_XATTR);
>>>>>  		flags &= ~(NFS_INO_INVALID_CHANGE |
>>>>> NFS_INO_INVALID_SIZE);
>>>>>  	}
>>>>> @@ -522,6 +523,7 @@ nfs_fhget(struct super_block *sb, struct
>>>>> nfs_fh *fh, struct nfs_fattr *fattr)
>>>>>  		inode_set_atime(inode, 0, 0);
>>>>>  		inode_set_mtime(inode, 0, 0);
>>>>>  		inode_set_ctime(inode, 0, 0);
>>>>> +		memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>>>>>  		inode_set_iversion_raw(inode, 0);
>>>>>  		inode->i_size = 0;
>>>>>  		clear_nlink(inode);
>>>>> @@ -545,6 +547,10 @@ nfs_fhget(struct super_block *sb, struct
>>>>> nfs_fh *fh, struct nfs_fattr *fattr)
>>>>>  			inode_set_ctime_to_ts(inode, fattr-
>>>>>> ctime);
>>>>>  		else if (fattr_supported &
>>>>> NFS_ATTR_FATTR_CTIME)
>>>>>  			nfs_set_cache_invalid(inode,
>>>>> NFS_INO_INVALID_CTIME);
>>>>> +		if (fattr->valid & NFS_ATTR_FATTR_BTIME)
>>>>> +			nfsi->btime = fattr->btime;
>>>>> +		else if (fattr_supported &
>>>>> NFS_ATTR_FATTR_BTIME)
>>>>> +			nfs_set_cache_invalid(inode,
>>>>> NFS_INO_INVALID_BTIME);
>>>>>  		if (fattr->valid & NFS_ATTR_FATTR_CHANGE)
>>>>>  			inode_set_iversion_raw(inode, fattr-
>>>>>> change_attr);
>>>>>  		else
>>>>> @@ -1900,7 +1906,7 @@ static int
>>>>> nfs_inode_finish_partial_attr_update(const struct nfs_fattr
>>>>> *fattr,
>>>>>  		NFS_INO_INVALID_ATIME | NFS_INO_INVALID_CTIME
>>>>> |
>>>>>  		NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE |
>>>>>  		NFS_INO_INVALID_BLOCKS | NFS_INO_INVALID_OTHER
>>>>> |
>>>>> -		NFS_INO_INVALID_NLINK;
>>>>> +		NFS_INO_INVALID_NLINK | NFS_INO_INVALID_BTIME;
>>>>>  	unsigned long cache_validity = NFS_I(inode)-
>>>>>> cache_validity;
>>>>>  	enum nfs4_change_attr_type ctype = NFS_SERVER(inode)-
>>>>>> change_attr_type;
>>>>>
>>>>> @@ -2219,10 +2225,13 @@ static int nfs_update_inode(struct
>>>>> inode *inode, struct nfs_fattr *fattr)
>>>>>  	nfs_fattr_fixup_delegated(inode, fattr);
>>>>>
>>>>>  	save_cache_validity = nfsi->cache_validity;
>>>>> -	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
>>>>> -			| NFS_INO_INVALID_ATIME
>>>>> -			| NFS_INO_REVAL_FORCED
>>>>> -			| NFS_INO_INVALID_BLOCKS);
>>>>> +	nfsi->cache_validity &=
>>>>> +		~(NFS_INO_INVALID_ATIME | NFS_INO_REVAL_FORCED
>>>>> |
>>>>> +		  NFS_INO_INVALID_CHANGE |
>>>>> NFS_INO_INVALID_CTIME |
>>>>> +		  NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE
>>>>> |
>>>>> +		  NFS_INO_INVALID_OTHER |
>>>>> NFS_INO_INVALID_BLOCKS |
>>>>> +		  NFS_INO_INVALID_NLINK | NFS_INO_INVALID_MODE
>>>>> |
>>>>> +		  NFS_INO_INVALID_BTIME);
>>>>>
>>>>
>>>> The delta above is a little curious. This patch is just adding
>>>> NFS_INO_INVALID_BTIME, but the patch above adds the clearing of
>>>> several
>>>> other bits as well. Why does this logic change?
>>>
>>> Good point, I wonder if it was based on other attribute work at the
>>> time,
>>> the original was here:
>>> https://lore.kernel.org/linux-nfs/20211227190504.309612-3-trondmy@kernel.org/
>>>
>>> So I think what we're doing here is clearing what we know we don't
>>> have to
>>> check/update - I think we can drop this entire hunk, its a minor
>>> optimization, but hopefully we can get some expert opinion.  
>>> Trond, do you
>>> want me to test with this hunk removed?
>>
>> Looking little closer, I think what happened there is that
>> NFS_INO_INVALID_ATTR got unrolled into individual attr flags. So
>> there
>> may be no net change here (other than adding BTIME).
>>
>> Still, it'd be best to do that in a separate patch, IMO, esp. since
>> this is in nfs_update_inode(), which is already so "fiddly".
>
> NFS_INO_INVALID_ATTR is a dinosaur that I'd like to see extinct. We
> have fine grained attribute tracking these days, and so keeping the old
> coarse grained one around is just confusing.
>
> That said, I'm fine with this being a separate patch.

Ok - I'll resend this on v3 adding BTIME to NFS_INO_INVALID_ATTR.

Ben