[PATCH v1 0/3] NFS Client btime support

[PATCH v1 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.

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            | 48 ++++++++++++++++++++++++++-------
 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, 117 insertions(+), 41 deletions(-)

-- 
2.47.0

[PATCH v1 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>
Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/inode.c            |  5 ++--
 include/linux/nfs_fs_sb.h |  2 +-
 include/linux/nfs_xdr.h   | 54 +++++++++++++++++++--------------------
 3 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 1aa67fca69b2..d4e449fa076e 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -2164,10 +2164,11 @@ 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%lx)\n",
 			__func__, inode->i_sb->s_id, inode->i_ino,
 			nfs_display_fhandle_hash(NFS_FH(inode)),
-			atomic_read(&inode->i_count), fattr->valid);
+			atomic_read(&inode->i_count),
+			(unsigned long)fattr->valid);
 
 	if (!(fattr->valid & NFS_ATTR_FATTR_FILEID)) {
 		/* Only a mounted-on-fileid? Just exit */
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index f00bfcee7120..056d0ad38756 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -168,8 +168,8 @@ struct nfs_server {
 #define NFS_MOUNT_SHUTDOWN			0x08000000
 #define NFS_MOUNT_NO_ALIGNWRITE		0x10000000
 
-	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 9155a6ffc370..b7b06f0d2fb9 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 v1 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>
Signed-off-by: Benjamin Coddington <bcodding@redhat.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 d4e449fa076e..ffbcecb65bc7 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -195,6 +195,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);
 	}
@@ -520,6 +521,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);
@@ -543,6 +545,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
@@ -1898,7 +1904,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;
 
@@ -2218,10 +2224,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);
@@ -2260,7 +2269,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;
@@ -2294,6 +2304,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 6e95db6c17e9..8b81b378ba5f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -219,6 +219,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,
@@ -240,6 +241,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
@@ -320,6 +322,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;
@@ -1291,7 +1296,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;
@@ -4032,6 +4038,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;
@@ -5739,6 +5749,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 e8ac3f615f93..5f0d08b500ef 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 b7b06f0d2fb9..2fd7890214df 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 v1 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>
Signed-off-by: Benjamin Coddington <bcodding@redhat.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 ffbcecb65bc7..6045d0a2e882 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -892,6 +892,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;
 
@@ -911,6 +912,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;
@@ -921,6 +925,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;
@@ -931,9 +936,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);
@@ -965,7 +973,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;
 
@@ -989,6 +997,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)
@@ -1010,6 +1020,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 22c973316f0b..ba77bd8cbe6d 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 v1 1/3] Expand the type of nfs_fattr->valid

[Jeff Layton]

On Thu, 2025-05-15 at 10:40 -0400, Benjamin Coddington wrote:
> 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>
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
>  fs/nfs/inode.c            |  5 ++--
>  include/linux/nfs_fs_sb.h |  2 +-
>  include/linux/nfs_xdr.h   | 54 +++++++++++++++++++--------------------
>  3 files changed, 31 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 1aa67fca69b2..d4e449fa076e 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -2164,10 +2164,11 @@ 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%lx)\n",
>  			__func__, inode->i_sb->s_id, inode->i_ino,
>  			nfs_display_fhandle_hash(NFS_FH(inode)),
> -			atomic_read(&inode->i_count), fattr->valid);
> +			atomic_read(&inode->i_count),
> +			(unsigned long)fattr->valid);

Why the cast? You could just set the format to %llx and pass fattr-
>valid as-is?

>  
>  	if (!(fattr->valid & NFS_ATTR_FATTR_FILEID)) {
>  		/* Only a mounted-on-fileid? Just exit */
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index f00bfcee7120..056d0ad38756 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -168,8 +168,8 @@ struct nfs_server {
>  #define NFS_MOUNT_SHUTDOWN			0x08000000
>  #define NFS_MOUNT_NO_ALIGNWRITE		0x10000000
>  
> -	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 9155a6ffc370..b7b06f0d2fb9 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 \

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v1 1/3] Expand the type of nfs_fattr->valid

[Benjamin Coddington]

On 16 May 2025, at 11:05, Jeff Layton wrote:

> On Thu, 2025-05-15 at 10:40 -0400, Benjamin Coddington wrote:
>> 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>
>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>> ---
>>  fs/nfs/inode.c            |  5 ++--
>>  include/linux/nfs_fs_sb.h |  2 +-
>>  include/linux/nfs_xdr.h   | 54 +++++++++++++++++++--------------------
>>  3 files changed, 31 insertions(+), 30 deletions(-)
>>
>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>> index 1aa67fca69b2..d4e449fa076e 100644
>> --- a/fs/nfs/inode.c
>> +++ b/fs/nfs/inode.c
>> @@ -2164,10 +2164,11 @@ 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%lx)\n",
>>  			__func__, inode->i_sb->s_id, inode->i_ino,
>>  			nfs_display_fhandle_hash(NFS_FH(inode)),
>> -			atomic_read(&inode->i_count), fattr->valid);
>> +			atomic_read(&inode->i_count),
>> +			(unsigned long)fattr->valid);
>
> Why the cast? You could just set the format to %llx and pass fattr-
>> valid as-is?

Yes of course, the cast will be removed.

Ben

Re: [PATCH v1 1/3] Expand the type of nfs_fattr->valid

[Anna Schumaker]

Hi Ben,

On 5/16/25 11:31 AM, Benjamin Coddington wrote:
> On 16 May 2025, at 11:05, Jeff Layton wrote:
> 
>> On Thu, 2025-05-15 at 10:40 -0400, Benjamin Coddington wrote:
>>> 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>
>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>>> ---
>>>  fs/nfs/inode.c            |  5 ++--
>>>  include/linux/nfs_fs_sb.h |  2 +-
>>>  include/linux/nfs_xdr.h   | 54 +++++++++++++++++++--------------------
>>>  3 files changed, 31 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>>> index 1aa67fca69b2..d4e449fa076e 100644
>>> --- a/fs/nfs/inode.c
>>> +++ b/fs/nfs/inode.c
>>> @@ -2164,10 +2164,11 @@ 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%lx)\n",
>>>  			__func__, inode->i_sb->s_id, inode->i_ino,
>>>  			nfs_display_fhandle_hash(NFS_FH(inode)),
>>> -			atomic_read(&inode->i_count), fattr->valid);
>>> +			atomic_read(&inode->i_count),
>>> +			(unsigned long)fattr->valid);
>>
>> Why the cast? You could just set the format to %llx and pass fattr-
>>> valid as-is?
> 
> Yes of course, the cast will be removed.

I'm not seeing any objections to adding btime. I was wondering if you're planning
a v2 of these patches, or should I fix up the casting in my tree?

Anna

> 
> Ben
> 

Re: [PATCH v1 1/3] Expand the type of nfs_fattr->valid

[Benjamin Coddington]

On 27 May 2025, at 17:26, Anna Schumaker wrote:

> Hi Ben,
>
> On 5/16/25 11:31 AM, Benjamin Coddington wrote:
>> On 16 May 2025, at 11:05, Jeff Layton wrote:
>>
>>> On Thu, 2025-05-15 at 10:40 -0400, Benjamin Coddington wrote:
>>>> 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>
>>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>>>> ---
>>>>  fs/nfs/inode.c            |  5 ++--
>>>>  include/linux/nfs_fs_sb.h |  2 +-
>>>>  include/linux/nfs_xdr.h   | 54 +++++++++++++++++++--------------------
>>>>  3 files changed, 31 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>>>> index 1aa67fca69b2..d4e449fa076e 100644
>>>> --- a/fs/nfs/inode.c
>>>> +++ b/fs/nfs/inode.c
>>>> @@ -2164,10 +2164,11 @@ 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%lx)\n",
>>>>  			__func__, inode->i_sb->s_id, inode->i_ino,
>>>>  			nfs_display_fhandle_hash(NFS_FH(inode)),
>>>> -			atomic_read(&inode->i_count), fattr->valid);
>>>> +			atomic_read(&inode->i_count),
>>>> +			(unsigned long)fattr->valid);
>>>
>>> Why the cast? You could just set the format to %llx and pass fattr-
>>>> valid as-is?
>>
>> Yes of course, the cast will be removed.
>
> I'm not seeing any objections to adding btime. I was wondering if you're planning
> a v2 of these patches, or should I fix up the casting in my tree?

I'll send a v2, hoping for more review - thanks Anna!

Ben