linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2] nfsv4: Add support for the birth time attribute
@ 2023-09-01  8:34 Chen Hanxiao
  2023-09-08 10:26 ` 回复: " Hanxiao Chen (Fujitsu)
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Chen Hanxiao @ 2023-09-01  8:34 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Jeff Layton, linux-nfs

nfsd already support btime by commit e377a3e698.

This patch enable nfs to report btime in nfs_getattr.
If underlying filesystem supports "btime" timestamp,
statx will report btime for STATX_BTIME.

Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>

---
v1.1:
	minor fix
v2:
	properly set cache validity

 fs/nfs/inode.c          | 28 ++++++++++++++++++++++++----
 fs/nfs/nfs4proc.c       |  3 +++
 fs/nfs/nfs4xdr.c        | 23 +++++++++++++++++++++++
 include/linux/nfs_fs.h  |  2 ++
 include/linux/nfs_xdr.h |  5 ++++-
 5 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 8172dd4135a1..cfdf68b07982 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -196,7 +196,8 @@ 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_XATTR);
+				   NFS_INO_INVALID_XATTR |
+				   NFS_INO_INVALID_BTIME);
 		flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
 	}
 
@@ -515,6 +516,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 		memset(&inode->i_atime, 0, sizeof(inode->i_atime));
 		memset(&inode->i_mtime, 0, sizeof(inode->i_mtime));
 		memset(&inode->i_ctime, 0, sizeof(inode->i_ctime));
+		memset(&nfsi->btime, 0, sizeof(nfsi->btime));
 		inode_set_iversion_raw(inode, 0);
 		inode->i_size = 0;
 		clear_nlink(inode);
@@ -538,6 +540,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 			inode->i_ctime = 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
@@ -835,6 +841,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);
+	struct nfs_inode *nfsi = NFS_I(inode);
 	unsigned long cache_validity;
 	int err = 0;
 	bool force_sync = query_flags & AT_STATX_FORCE_SYNC;
@@ -845,7 +852,7 @@ 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 ((query_flags & AT_STATX_DONT_SYNC) && !force_sync) {
@@ -920,6 +927,10 @@ 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;
+	if (!(server->fattr_valid & NFS_ATTR_FATTR_BTIME))
+		stat->result_mask &= ~STATX_BTIME;
+	else
+		stat->btime = nfsi->btime;
 out:
 	trace_nfs_getattr_exit(inode, err);
 	return err;
@@ -1803,7 +1814,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;
 
@@ -2122,7 +2133,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
 			| NFS_INO_INVALID_ATIME
 			| NFS_INO_REVAL_FORCED
-			| NFS_INO_INVALID_BLOCKS);
+			| NFS_INO_INVALID_BLOCKS
+			| NFS_INO_INVALID_BTIME);
 
 	/* Do atomic weak cache consistency updates */
 	nfs_wcc_update_inode(inode, fattr);
@@ -2161,6 +2173,7 @@ 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_BTIME
 					| NFS_INO_INVALID_OTHER;
 				if (S_ISDIR(inode->i_mode))
 					nfs_force_lookup_revalidate(inode);
@@ -2189,6 +2202,12 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 		nfsi->cache_validity |=
 			save_cache_validity & NFS_INO_INVALID_MTIME;
 
+	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;
+
 	if (fattr->valid & NFS_ATTR_FATTR_CTIME)
 		inode->i_ctime = fattr->ctime;
 	else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
@@ -2332,6 +2351,7 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
 #endif /* CONFIG_NFS_V4 */
 #ifdef CONFIG_NFS_V4_2
 	nfsi->xattr_cache = NULL;
+	memset(&nfsi->btime, 0, sizeof(nfsi->btime));
 #endif
 	nfs_netfs_inode_init(nfsi);
 
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 832fa226b8f2..024a057a055c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -211,6 +211,7 @@ const u32 nfs4_fattr_bitmap[3] = {
 	| FATTR4_WORD1_TIME_ACCESS
 	| FATTR4_WORD1_TIME_METADATA
 	| FATTR4_WORD1_TIME_MODIFY
+	| FATTR4_WORD1_TIME_CREATE
 	| FATTR4_WORD1_MOUNTED_ON_FILEID,
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
 	FATTR4_WORD2_SECURITY_LABEL
@@ -3909,6 +3910,8 @@ 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_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;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index deec76cf5afe..df3699eb29e8 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4171,6 +4171,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;
@@ -4730,6 +4748,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/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 279262057a92..ba8c1872494d 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -159,6 +159,7 @@ struct nfs_inode {
 	unsigned long		read_cache_jiffies;
 	unsigned long		attrtimeo;
 	unsigned long		attrtimeo_timestamp;
+	struct timespec64       btime;
 
 	unsigned long		attr_gencount;
 
@@ -298,6 +299,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 12bbb5c63664..6c9ab5be0694 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	(1U << 23)
 #define NFS_ATTR_FATTR_GROUP_NAME	(1U << 24)
 #define NFS_ATTR_FATTR_V4_SECURITY_LABEL (1U << 25)
+#define NFS_ATTR_FATTR_BTIME		(1U << 26)
 
 #define NFS_ATTR_FATTR (NFS_ATTR_FATTR_TYPE \
 		| NFS_ATTR_FATTR_MODE \
@@ -126,7 +128,8 @@ struct nfs_fattr {
 		| NFS_ATTR_FATTR_SPACE_USED)
 #define NFS_ATTR_FATTR_V4 (NFS_ATTR_FATTR \
 		| NFS_ATTR_FATTR_SPACE_USED \
-		| NFS_ATTR_FATTR_V4_SECURITY_LABEL)
+		| NFS_ATTR_FATTR_V4_SECURITY_LABEL \
+		| NFS_ATTR_FATTR_BTIME)
 
 /*
  * Maximal number of supported layout drivers.
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* 回复: [RFC PATCH v2] nfsv4: Add support for the birth time attribute
  2023-09-01  8:34 [RFC PATCH v2] nfsv4: Add support for the birth time attribute Chen Hanxiao
@ 2023-09-08 10:26 ` Hanxiao Chen (Fujitsu)
  2025-05-13 16:08 ` Benjamin Coddington
  2025-05-13 19:42 ` Benjamin Coddington
  2 siblings, 0 replies; 7+ messages in thread
From: Hanxiao Chen (Fujitsu) @ 2023-09-08 10:26 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Jeff Layton, linux-nfs@vger.kernel.org



> -----邮件原件-----
> 发件人: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> 发送时间: 2023年9月1日 16:34
> 收件人: Trond Myklebust <trond.myklebust@hammerspace.com>; Anna
> Schumaker <anna@kernel.org>
> 抄送: Jeff Layton <jlayton@kernel.org>; linux-nfs@vger.kernel.org
> 主题: [RFC PATCH v2] nfsv4: Add support for the birth time attribute
> 
> nfsd already support btime by commit e377a3e698.
> 
> This patch enable nfs to report btime in nfs_getattr.
> If underlying filesystem supports "btime" timestamp, statx will report btime for
> STATX_BTIME.
> 
> Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> 
> ---
> v1.1:
> 	minor fix
> v2:
> 	properly set cache validity
> 

Hi, Trond

	Any comments?

Regards,
- Chen

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] nfsv4: Add support for the birth time attribute
  2023-09-01  8:34 [RFC PATCH v2] nfsv4: Add support for the birth time attribute Chen Hanxiao
  2023-09-08 10:26 ` 回复: " Hanxiao Chen (Fujitsu)
@ 2025-05-13 16:08 ` Benjamin Coddington
  2025-05-13 16:36   ` Aurélien Couderc
  2025-08-15 15:20   ` Aurélien Couderc
  2025-05-13 19:42 ` Benjamin Coddington
  2 siblings, 2 replies; 7+ messages in thread
From: Benjamin Coddington @ 2025-05-13 16:08 UTC (permalink / raw)
  To: Chen Hanxiao; +Cc: Trond Myklebust, Anna Schumaker, Jeff Layton, linux-nfs

I'm interested in this work, Chen are you still interested in moving this
forward?   I have another question below --

On 1 Sep 2023, at 4:34, Chen Hanxiao wrote:

> nfsd already support btime by commit e377a3e698.
>
> This patch enable nfs to report btime in nfs_getattr.
> If underlying filesystem supports "btime" timestamp,
> statx will report btime for STATX_BTIME.
>
> Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
>
> ---
> v1.1:
> 	minor fix
> v2:
> 	properly set cache validity
>
>  fs/nfs/inode.c          | 28 ++++++++++++++++++++++++----
>  fs/nfs/nfs4proc.c       |  3 +++
>  fs/nfs/nfs4xdr.c        | 23 +++++++++++++++++++++++
>  include/linux/nfs_fs.h  |  2 ++
>  include/linux/nfs_xdr.h |  5 ++++-
>  5 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 8172dd4135a1..cfdf68b07982 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -196,7 +196,8 @@ 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_XATTR);
> +				   NFS_INO_INVALID_XATTR |
> +				   NFS_INO_INVALID_BTIME);
>  		flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
>  	}
>
> @@ -515,6 +516,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>  		memset(&inode->i_atime, 0, sizeof(inode->i_atime));
>  		memset(&inode->i_mtime, 0, sizeof(inode->i_mtime));
>  		memset(&inode->i_ctime, 0, sizeof(inode->i_ctime));
> +		memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>  		inode_set_iversion_raw(inode, 0);
>  		inode->i_size = 0;
>  		clear_nlink(inode);
> @@ -538,6 +540,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>  			inode->i_ctime = 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
> @@ -835,6 +841,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);
> +	struct nfs_inode *nfsi = NFS_I(inode);
>  	unsigned long cache_validity;
>  	int err = 0;
>  	bool force_sync = query_flags & AT_STATX_FORCE_SYNC;
> @@ -845,7 +852,7 @@ 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 ((query_flags & AT_STATX_DONT_SYNC) && !force_sync) {
> @@ -920,6 +927,10 @@ 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;
> +	if (!(server->fattr_valid & NFS_ATTR_FATTR_BTIME))
> +		stat->result_mask &= ~STATX_BTIME;
> +	else
> +		stat->btime = nfsi->btime;
>  out:
>  	trace_nfs_getattr_exit(inode, err);
>  	return err;
> @@ -1803,7 +1814,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;
>
> @@ -2122,7 +2133,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
>  			| NFS_INO_INVALID_ATIME
>  			| NFS_INO_REVAL_FORCED
> -			| NFS_INO_INVALID_BLOCKS);
> +			| NFS_INO_INVALID_BLOCKS
> +			| NFS_INO_INVALID_BTIME);
>
>  	/* Do atomic weak cache consistency updates */
>  	nfs_wcc_update_inode(inode, fattr);
> @@ -2161,6 +2173,7 @@ 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_BTIME
>  					| NFS_INO_INVALID_OTHER;
>  				if (S_ISDIR(inode->i_mode))
>  					nfs_force_lookup_revalidate(inode);
> @@ -2189,6 +2202,12 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  		nfsi->cache_validity |=
>  			save_cache_validity & NFS_INO_INVALID_MTIME;
>
> +	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;
> +
>  	if (fattr->valid & NFS_ATTR_FATTR_CTIME)
>  		inode->i_ctime = fattr->ctime;
>  	else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
> @@ -2332,6 +2351,7 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
>  #endif /* CONFIG_NFS_V4 */
>  #ifdef CONFIG_NFS_V4_2
>  	nfsi->xattr_cache = NULL;
> +	memset(&nfsi->btime, 0, sizeof(nfsi->btime));


^^ is this redundant if we're going to do it anyway in nfs_fhget for I_NEW?

.. actually, I don't understand why were doing /any/ nfsi member
initialization here.. am I missing something?

Otherwise, this gets

Tested-by: Benjamin Coddington <bcodding@redhat.com>
Reviewed-by: Benjamin Coddington <bcodding@redhat.com>

Ben


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] nfsv4: Add support for the birth time attribute
  2025-05-13 16:08 ` Benjamin Coddington
@ 2025-05-13 16:36   ` Aurélien Couderc
  2025-05-13 17:47     ` Benjamin Coddington
  2025-08-15 15:20   ` Aurélien Couderc
  1 sibling, 1 reply; 7+ messages in thread
From: Aurélien Couderc @ 2025-05-13 16:36 UTC (permalink / raw)
  To: linux-nfs

On Tue, May 13, 2025 at 6:08 PM Benjamin Coddington <bcodding@redhat.com> wrote:
>
> I'm interested in this work, Chen are you still interested in moving this
> forward?   I have another question below --
>
> On 1 Sep 2023, at 4:34, Chen Hanxiao wrote:
>
> > nfsd already support btime by commit e377a3e698.
> >
> > This patch enable nfs to report btime in nfs_getattr.
> > If underlying filesystem supports "btime" timestamp,
> > statx will report btime for STATX_BTIME.
> >
> > Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> >
> > ---
> > v1.1:
> >       minor fix
> > v2:
> >       properly set cache validity
> >
> >  fs/nfs/inode.c          | 28 ++++++++++++++++++++++++----
> >  fs/nfs/nfs4proc.c       |  3 +++
> >  fs/nfs/nfs4xdr.c        | 23 +++++++++++++++++++++++
> >  include/linux/nfs_fs.h  |  2 ++
> >  include/linux/nfs_xdr.h |  5 ++++-
> >  5 files changed, 56 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > index 8172dd4135a1..cfdf68b07982 100644
> > --- a/fs/nfs/inode.c
> > +++ b/fs/nfs/inode.c
> > @@ -196,7 +196,8 @@ 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_XATTR);
> > +                                NFS_INO_INVALID_XATTR |
> > +                                NFS_INO_INVALID_BTIME);
> >               flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
> >       }
> >
> > @@ -515,6 +516,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
> >               memset(&inode->i_atime, 0, sizeof(inode->i_atime));
> >               memset(&inode->i_mtime, 0, sizeof(inode->i_mtime));
> >               memset(&inode->i_ctime, 0, sizeof(inode->i_ctime));
> > +             memset(&nfsi->btime, 0, sizeof(nfsi->btime));
> >               inode_set_iversion_raw(inode, 0);
> >               inode->i_size = 0;
> >               clear_nlink(inode);
> > @@ -538,6 +540,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
> >                       inode->i_ctime = 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
> > @@ -835,6 +841,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);
> > +     struct nfs_inode *nfsi = NFS_I(inode);
> >       unsigned long cache_validity;
> >       int err = 0;
> >       bool force_sync = query_flags & AT_STATX_FORCE_SYNC;
> > @@ -845,7 +852,7 @@ 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 ((query_flags & AT_STATX_DONT_SYNC) && !force_sync) {
> > @@ -920,6 +927,10 @@ 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;
> > +     if (!(server->fattr_valid & NFS_ATTR_FATTR_BTIME))
> > +             stat->result_mask &= ~STATX_BTIME;
> > +     else
> > +             stat->btime = nfsi->btime;
> >  out:
> >       trace_nfs_getattr_exit(inode, err);
> >       return err;
> > @@ -1803,7 +1814,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;
> >
> > @@ -2122,7 +2133,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
> >       nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
> >                       | NFS_INO_INVALID_ATIME
> >                       | NFS_INO_REVAL_FORCED
> > -                     | NFS_INO_INVALID_BLOCKS);
> > +                     | NFS_INO_INVALID_BLOCKS
> > +                     | NFS_INO_INVALID_BTIME);
> >
> >       /* Do atomic weak cache consistency updates */
> >       nfs_wcc_update_inode(inode, fattr);
> > @@ -2161,6 +2173,7 @@ 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_BTIME
> >                                       | NFS_INO_INVALID_OTHER;
> >                               if (S_ISDIR(inode->i_mode))
> >                                       nfs_force_lookup_revalidate(inode);
> > @@ -2189,6 +2202,12 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
> >               nfsi->cache_validity |=
> >                       save_cache_validity & NFS_INO_INVALID_MTIME;
> >
> > +     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;
> > +
> >       if (fattr->valid & NFS_ATTR_FATTR_CTIME)
> >               inode->i_ctime = fattr->ctime;
> >       else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
> > @@ -2332,6 +2351,7 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
> >  #endif /* CONFIG_NFS_V4 */
> >  #ifdef CONFIG_NFS_V4_2
> >       nfsi->xattr_cache = NULL;
> > +     memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>
>
> ^^ is this redundant if we're going to do it anyway in nfs_fhget for I_NEW?
>
> .. actually, I don't understand why were doing /any/ nfsi member
> initialization here.. am I missing something?
>
> Otherwise, this gets
>
> Tested-by: Benjamin Coddington <bcodding@redhat.com>
> Reviewed-by: Benjamin Coddington <bcodding@redhat.com>

Reviewed-by: Aurélien Couderc <aurelien.couderc2002@gmail.com>

I am astonished that birth timestamp support in the Linux NFS client
wasn't implemented... earlier.

Just curious: What happens with this functionality if one filesystem
exported by nfsd supports birth timestamps, and another filesystem
does not support birth timestamps?

Aurélien
-- 
Aurélien Couderc <aurelien.couderc2002@gmail.com>
Big Data/Data mining expert, chess enthusiast

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] nfsv4: Add support for the birth time attribute
  2025-05-13 16:36   ` Aurélien Couderc
@ 2025-05-13 17:47     ` Benjamin Coddington
  0 siblings, 0 replies; 7+ messages in thread
From: Benjamin Coddington @ 2025-05-13 17:47 UTC (permalink / raw)
  To: Aurélien Couderc; +Cc: linux-nfs

On 13 May 2025, at 12:36, Aurélien Couderc wrote:

> On Tue, May 13, 2025 at 6:08 PM Benjamin Coddington <bcodding@redhat.com> wrote:
>>
>> I'm interested in this work, Chen are you still interested in moving this
>> forward?   I have another question below --
>>
>> On 1 Sep 2023, at 4:34, Chen Hanxiao wrote:
>>
>>> nfsd already support btime by commit e377a3e698.
>>>
>>> This patch enable nfs to report btime in nfs_getattr.
>>> If underlying filesystem supports "btime" timestamp,
>>> statx will report btime for STATX_BTIME.
>>>
>>> Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
>>>
>>> ---
>>> v1.1:
>>>       minor fix
>>> v2:
>>>       properly set cache validity
>>>
>>>  fs/nfs/inode.c          | 28 ++++++++++++++++++++++++----
>>>  fs/nfs/nfs4proc.c       |  3 +++
>>>  fs/nfs/nfs4xdr.c        | 23 +++++++++++++++++++++++
>>>  include/linux/nfs_fs.h  |  2 ++
>>>  include/linux/nfs_xdr.h |  5 ++++-
>>>  5 files changed, 56 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>>> index 8172dd4135a1..cfdf68b07982 100644
>>> --- a/fs/nfs/inode.c
>>> +++ b/fs/nfs/inode.c
>>> @@ -196,7 +196,8 @@ 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_XATTR);
>>> +                                NFS_INO_INVALID_XATTR |
>>> +                                NFS_INO_INVALID_BTIME);
>>>               flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
>>>       }
>>>
>>> @@ -515,6 +516,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>>>               memset(&inode->i_atime, 0, sizeof(inode->i_atime));
>>>               memset(&inode->i_mtime, 0, sizeof(inode->i_mtime));
>>>               memset(&inode->i_ctime, 0, sizeof(inode->i_ctime));
>>> +             memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>>>               inode_set_iversion_raw(inode, 0);
>>>               inode->i_size = 0;
>>>               clear_nlink(inode);
>>> @@ -538,6 +540,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>>>                       inode->i_ctime = 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
>>> @@ -835,6 +841,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);
>>> +     struct nfs_inode *nfsi = NFS_I(inode);
>>>       unsigned long cache_validity;
>>>       int err = 0;
>>>       bool force_sync = query_flags & AT_STATX_FORCE_SYNC;
>>> @@ -845,7 +852,7 @@ 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 ((query_flags & AT_STATX_DONT_SYNC) && !force_sync) {
>>> @@ -920,6 +927,10 @@ 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;
>>> +     if (!(server->fattr_valid & NFS_ATTR_FATTR_BTIME))
>>> +             stat->result_mask &= ~STATX_BTIME;
>>> +     else
>>> +             stat->btime = nfsi->btime;
>>>  out:
>>>       trace_nfs_getattr_exit(inode, err);
>>>       return err;
>>> @@ -1803,7 +1814,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;
>>>
>>> @@ -2122,7 +2133,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>>>       nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
>>>                       | NFS_INO_INVALID_ATIME
>>>                       | NFS_INO_REVAL_FORCED
>>> -                     | NFS_INO_INVALID_BLOCKS);
>>> +                     | NFS_INO_INVALID_BLOCKS
>>> +                     | NFS_INO_INVALID_BTIME);
>>>
>>>       /* Do atomic weak cache consistency updates */
>>>       nfs_wcc_update_inode(inode, fattr);
>>> @@ -2161,6 +2173,7 @@ 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_BTIME
>>>                                       | NFS_INO_INVALID_OTHER;
>>>                               if (S_ISDIR(inode->i_mode))
>>>                                       nfs_force_lookup_revalidate(inode);
>>> @@ -2189,6 +2202,12 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>>>               nfsi->cache_validity |=
>>>                       save_cache_validity & NFS_INO_INVALID_MTIME;
>>>
>>> +     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;
>>> +
>>>       if (fattr->valid & NFS_ATTR_FATTR_CTIME)
>>>               inode->i_ctime = fattr->ctime;
>>>       else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
>>> @@ -2332,6 +2351,7 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
>>>  #endif /* CONFIG_NFS_V4 */
>>>  #ifdef CONFIG_NFS_V4_2
>>>       nfsi->xattr_cache = NULL;
>>> +     memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>>
>>
>> ^^ is this redundant if we're going to do it anyway in nfs_fhget for I_NEW?
>>
>> .. actually, I don't understand why were doing /any/ nfsi member
>> initialization here.. am I missing something?
>>
>> Otherwise, this gets
>>
>> Tested-by: Benjamin Coddington <bcodding@redhat.com>
>> Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
>
> Reviewed-by: Aurélien Couderc <aurelien.couderc2002@gmail.com>
>
> I am astonished that birth timestamp support in the Linux NFS client
> wasn't implemented... earlier.
>
> Just curious: What happens with this functionality if one filesystem
> exported by nfsd supports birth timestamps, and another filesystem
> does not support birth timestamps?

Without explicitly looking, I think the attribute would just not be listed
in the supported attributes as queried by the client during mount, and the
client would just behave as it does now.

Ben


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] nfsv4: Add support for the birth time attribute
  2023-09-01  8:34 [RFC PATCH v2] nfsv4: Add support for the birth time attribute Chen Hanxiao
  2023-09-08 10:26 ` 回复: " Hanxiao Chen (Fujitsu)
  2025-05-13 16:08 ` Benjamin Coddington
@ 2025-05-13 19:42 ` Benjamin Coddington
  2 siblings, 0 replies; 7+ messages in thread
From: Benjamin Coddington @ 2025-05-13 19:42 UTC (permalink / raw)
  To: Chen Hanxiao; +Cc: Trond Myklebust, Anna Schumaker, Jeff Layton, linux-nfs

On 1 Sep 2023, at 4:34, Chen Hanxiao wrote:

> nfsd already support btime by commit e377a3e698.
>
> This patch enable nfs to report btime in nfs_getattr.
> If underlying filesystem supports "btime" timestamp,
> statx will report btime for STATX_BTIME.
>
> Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
>
> ---
> v1.1:
> 	minor fix
> v2:
> 	properly set cache validity
>
>  fs/nfs/inode.c          | 28 ++++++++++++++++++++++++----
>  fs/nfs/nfs4proc.c       |  3 +++
>  fs/nfs/nfs4xdr.c        | 23 +++++++++++++++++++++++
>  include/linux/nfs_fs.h  |  2 ++
>  include/linux/nfs_xdr.h |  5 ++++-
>  5 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 8172dd4135a1..cfdf68b07982 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -196,7 +196,8 @@ 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_XATTR);
> +				   NFS_INO_INVALID_XATTR |
> +				   NFS_INO_INVALID_BTIME);
>  		flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
>  	}
>
> @@ -515,6 +516,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>  		memset(&inode->i_atime, 0, sizeof(inode->i_atime));
>  		memset(&inode->i_mtime, 0, sizeof(inode->i_mtime));
>  		memset(&inode->i_ctime, 0, sizeof(inode->i_ctime));
> +		memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>  		inode_set_iversion_raw(inode, 0);
>  		inode->i_size = 0;
>  		clear_nlink(inode);
> @@ -538,6 +540,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
>  			inode->i_ctime = 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
> @@ -835,6 +841,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);
> +	struct nfs_inode *nfsi = NFS_I(inode);
>  	unsigned long cache_validity;
>  	int err = 0;
>  	bool force_sync = query_flags & AT_STATX_FORCE_SYNC;
> @@ -845,7 +852,7 @@ 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 ((query_flags & AT_STATX_DONT_SYNC) && !force_sync) {
> @@ -920,6 +927,10 @@ 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;
> +	if (!(server->fattr_valid & NFS_ATTR_FATTR_BTIME))
> +		stat->result_mask &= ~STATX_BTIME;
> +	else
> +		stat->btime = nfsi->btime;
>  out:
>  	trace_nfs_getattr_exit(inode, err);
>  	return err;
> @@ -1803,7 +1814,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;
>
> @@ -2122,7 +2133,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
>  			| NFS_INO_INVALID_ATIME
>  			| NFS_INO_REVAL_FORCED
> -			| NFS_INO_INVALID_BLOCKS);
> +			| NFS_INO_INVALID_BLOCKS
> +			| NFS_INO_INVALID_BTIME);
>
>  	/* Do atomic weak cache consistency updates */
>  	nfs_wcc_update_inode(inode, fattr);
> @@ -2161,6 +2173,7 @@ 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_BTIME
>  					| NFS_INO_INVALID_OTHER;
>  				if (S_ISDIR(inode->i_mode))
>  					nfs_force_lookup_revalidate(inode);
> @@ -2189,6 +2202,12 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
>  		nfsi->cache_validity |=
>  			save_cache_validity & NFS_INO_INVALID_MTIME;
>
> +	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;
> +
>  	if (fattr->valid & NFS_ATTR_FATTR_CTIME)
>  		inode->i_ctime = fattr->ctime;
>  	else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
> @@ -2332,6 +2351,7 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
>  #endif /* CONFIG_NFS_V4 */
>  #ifdef CONFIG_NFS_V4_2
>  	nfsi->xattr_cache = NULL;
> +	memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>  #endif
>  	nfs_netfs_inode_init(nfsi);
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 832fa226b8f2..024a057a055c 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -211,6 +211,7 @@ const u32 nfs4_fattr_bitmap[3] = {
>  	| FATTR4_WORD1_TIME_ACCESS
>  	| FATTR4_WORD1_TIME_METADATA
>  	| FATTR4_WORD1_TIME_MODIFY
> +	| FATTR4_WORD1_TIME_CREATE
>  	| FATTR4_WORD1_MOUNTED_ON_FILEID,
>  #ifdef CONFIG_NFS_V4_SECURITY_LABEL
>  	FATTR4_WORD2_SECURITY_LABEL
> @@ -3909,6 +3910,8 @@ 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_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;
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index deec76cf5afe..df3699eb29e8 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -4171,6 +4171,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;
> @@ -4730,6 +4748,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/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index 279262057a92..ba8c1872494d 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -159,6 +159,7 @@ struct nfs_inode {
>  	unsigned long		read_cache_jiffies;
>  	unsigned long		attrtimeo;
>  	unsigned long		attrtimeo_timestamp;
> +	struct timespec64       btime;
>
>  	unsigned long		attr_gencount;


Just had another thought here, we probably don't want to bump the
attr_gencount down to the next cacheline, here's my pahole output with this
patch:


    /* --- cacheline 2 boundary (128 bytes) was 16 bytes ago --- */
    long unsigned int          flags;                /*   144     8 */
    long unsigned int          cache_validity;       /*   152     8 */
    long unsigned int          read_cache_jiffies;   /*   160     8 */
    long unsigned int          attrtimeo;            /*   168     8 */
    long unsigned int          attrtimeo_timestamp;  /*   176     8 */
    struct timespec64          btime;                /*   184    16 */
    /* --- cacheline 3 boundary (192 bytes) was 8 bytes ago --- */
    long unsigned int          attr_gencount;        /*   200     8 */
    struct rb_root             access_cache;         /*   208     8 */
    struct list_head           access_cache_entry_lru; /*   216    16 */
    struct list_head           access_cache_inode_lru; /*   232    16 */


Maybe a better place would be after the xattr_cache?

Ben


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] nfsv4: Add support for the birth time attribute
  2025-05-13 16:08 ` Benjamin Coddington
  2025-05-13 16:36   ` Aurélien Couderc
@ 2025-08-15 15:20   ` Aurélien Couderc
  1 sibling, 0 replies; 7+ messages in thread
From: Aurélien Couderc @ 2025-08-15 15:20 UTC (permalink / raw)
  To: linux-nfs

What is the status of this patch? did it ever made it into the Linus
mainline kernel?

Aurélien

On Tue, May 13, 2025 at 6:08 PM Benjamin Coddington <bcodding@redhat.com> wrote:
>
> I'm interested in this work, Chen are you still interested in moving this
> forward?   I have another question below --
>
> On 1 Sep 2023, at 4:34, Chen Hanxiao wrote:
>
> > nfsd already support btime by commit e377a3e698.
> >
> > This patch enable nfs to report btime in nfs_getattr.
> > If underlying filesystem supports "btime" timestamp,
> > statx will report btime for STATX_BTIME.
> >
> > Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> >
> > ---
> > v1.1:
> >       minor fix
> > v2:
> >       properly set cache validity
> >
> >  fs/nfs/inode.c          | 28 ++++++++++++++++++++++++----
> >  fs/nfs/nfs4proc.c       |  3 +++
> >  fs/nfs/nfs4xdr.c        | 23 +++++++++++++++++++++++
> >  include/linux/nfs_fs.h  |  2 ++
> >  include/linux/nfs_xdr.h |  5 ++++-
> >  5 files changed, 56 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > index 8172dd4135a1..cfdf68b07982 100644
> > --- a/fs/nfs/inode.c
> > +++ b/fs/nfs/inode.c
> > @@ -196,7 +196,8 @@ 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_XATTR);
> > +                                NFS_INO_INVALID_XATTR |
> > +                                NFS_INO_INVALID_BTIME);
> >               flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE);
> >       }
> >
> > @@ -515,6 +516,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
> >               memset(&inode->i_atime, 0, sizeof(inode->i_atime));
> >               memset(&inode->i_mtime, 0, sizeof(inode->i_mtime));
> >               memset(&inode->i_ctime, 0, sizeof(inode->i_ctime));
> > +             memset(&nfsi->btime, 0, sizeof(nfsi->btime));
> >               inode_set_iversion_raw(inode, 0);
> >               inode->i_size = 0;
> >               clear_nlink(inode);
> > @@ -538,6 +540,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
> >                       inode->i_ctime = 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
> > @@ -835,6 +841,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);
> > +     struct nfs_inode *nfsi = NFS_I(inode);
> >       unsigned long cache_validity;
> >       int err = 0;
> >       bool force_sync = query_flags & AT_STATX_FORCE_SYNC;
> > @@ -845,7 +852,7 @@ 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 ((query_flags & AT_STATX_DONT_SYNC) && !force_sync) {
> > @@ -920,6 +927,10 @@ 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;
> > +     if (!(server->fattr_valid & NFS_ATTR_FATTR_BTIME))
> > +             stat->result_mask &= ~STATX_BTIME;
> > +     else
> > +             stat->btime = nfsi->btime;
> >  out:
> >       trace_nfs_getattr_exit(inode, err);
> >       return err;
> > @@ -1803,7 +1814,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;
> >
> > @@ -2122,7 +2133,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
> >       nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
> >                       | NFS_INO_INVALID_ATIME
> >                       | NFS_INO_REVAL_FORCED
> > -                     | NFS_INO_INVALID_BLOCKS);
> > +                     | NFS_INO_INVALID_BLOCKS
> > +                     | NFS_INO_INVALID_BTIME);
> >
> >       /* Do atomic weak cache consistency updates */
> >       nfs_wcc_update_inode(inode, fattr);
> > @@ -2161,6 +2173,7 @@ 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_BTIME
> >                                       | NFS_INO_INVALID_OTHER;
> >                               if (S_ISDIR(inode->i_mode))
> >                                       nfs_force_lookup_revalidate(inode);
> > @@ -2189,6 +2202,12 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
> >               nfsi->cache_validity |=
> >                       save_cache_validity & NFS_INO_INVALID_MTIME;
> >
> > +     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;
> > +
> >       if (fattr->valid & NFS_ATTR_FATTR_CTIME)
> >               inode->i_ctime = fattr->ctime;
> >       else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
> > @@ -2332,6 +2351,7 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
> >  #endif /* CONFIG_NFS_V4 */
> >  #ifdef CONFIG_NFS_V4_2
> >       nfsi->xattr_cache = NULL;
> > +     memset(&nfsi->btime, 0, sizeof(nfsi->btime));
>
>
> ^^ is this redundant if we're going to do it anyway in nfs_fhget for I_NEW?
>
> .. actually, I don't understand why were doing /any/ nfsi member
> initialization here.. am I missing something?
>
> Otherwise, this gets
>
> Tested-by: Benjamin Coddington <bcodding@redhat.com>
> Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
>
> Ben
>
>


-- 
Aurélien Couderc <aurelien.couderc2002@gmail.com>
Big Data/Data mining expert, chess enthusiast

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-08-15 15:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-01  8:34 [RFC PATCH v2] nfsv4: Add support for the birth time attribute Chen Hanxiao
2023-09-08 10:26 ` 回复: " Hanxiao Chen (Fujitsu)
2025-05-13 16:08 ` Benjamin Coddington
2025-05-13 16:36   ` Aurélien Couderc
2025-05-13 17:47     ` Benjamin Coddington
2025-08-15 15:20   ` Aurélien Couderc
2025-05-13 19:42 ` Benjamin Coddington

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).