linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v1.1] nfs: Add support for the birth time attribute
@ 2023-08-22 10:00 Chen Hanxiao
  2023-08-22 18:16 ` Jeff Layton
  0 siblings, 1 reply; 3+ messages in thread
From: Chen Hanxiao @ 2023-08-22 10:00 UTC (permalink / raw)
  To: trond.myklebust, anna; +Cc: linux-nfs, Chen Hanxiao

nfsd already support btime by:
commit e377a3e698fb ("nfsd: Add support for the birth time attribute")

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>

---
1.1:
	minor fix

 fs/nfs/inode.c          | 14 +++++++++++++-
 fs/nfs/nfs4proc.c       |  3 +++
 fs/nfs/nfs4xdr.c        | 23 +++++++++++++++++++++++
 include/linux/nfs_fs.h  |  2 ++
 include/linux/nfs_xdr.h |  2 ++
 5 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 8172dd4135a1..36e322b993f8 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -835,6 +835,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 +846,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) {
@@ -911,6 +912,10 @@ int nfs_getattr(struct mnt_idmap *idmap, const struct path *path,
 out_no_revalidate:
 	/* Only return attributes that were revalidated. */
 	stat->result_mask = nfs_get_valid_attrmask(inode) | request_mask;
+	if (nfsi->btime.tv_sec == 0 && nfsi->btime.tv_nsec == 0)
+		stat->result_mask &= ~STATX_BTIME;
+	else
+		stat->btime = nfsi->btime;
 
 	generic_fillattr(&nop_mnt_idmap, inode, stat);
 	stat->ino = nfs_compat_user_ino64(NFS_FILEID(inode));
@@ -2189,6 +2194,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 +2343,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 e1a886b58354..c4717ae4b1b3 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..8e90c5bbf5e4 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 \
-- 
2.39.1


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

* Re: [RFC PATCH v1.1] nfs: Add support for the birth time attribute
  2023-08-22 10:00 [RFC PATCH v1.1] nfs: Add support for the birth time attribute Chen Hanxiao
@ 2023-08-22 18:16 ` Jeff Layton
  2023-08-25 10:32   ` 回复: " Hanxiao Chen (Fujitsu)
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Layton @ 2023-08-22 18:16 UTC (permalink / raw)
  To: Chen Hanxiao, trond.myklebust, anna; +Cc: linux-nfs

On Tue, 2023-08-22 at 18:00 +0800, Chen Hanxiao wrote:
> nfsd already support btime by:
> commit e377a3e698fb ("nfsd: Add support for the birth time attribute")
> 
> 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>
> 
> ---
> 1.1:
> 	minor fix
> 
>  fs/nfs/inode.c          | 14 +++++++++++++-
>  fs/nfs/nfs4proc.c       |  3 +++
>  fs/nfs/nfs4xdr.c        | 23 +++++++++++++++++++++++
>  include/linux/nfs_fs.h  |  2 ++
>  include/linux/nfs_xdr.h |  2 ++
>  5 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 8172dd4135a1..36e322b993f8 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -835,6 +835,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 +846,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) {
> @@ -911,6 +912,10 @@ int nfs_getattr(struct mnt_idmap *idmap, const struct path *path,
>  out_no_revalidate:
>  	/* Only return attributes that were revalidated. */
>  	stat->result_mask = nfs_get_valid_attrmask(inode) | request_mask;
> +	if (nfsi->btime.tv_sec == 0 && nfsi->btime.tv_nsec == 0)
> +		stat->result_mask &= ~STATX_BTIME;
> +	else
> +		stat->btime = nfsi->btime;
>  
>  	generic_fillattr(&nop_mnt_idmap, inode, stat);
>  	stat->ino = nfs_compat_user_ino64(NFS_FILEID(inode));
> @@ -2189,6 +2194,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;
> +

I'm not sure we actually need the "else" part here, or the INVALID_BTIME
bit. The btime isn't expected to change, really ever, so if the server
didn't report it, I think we're generally safe to trust whatever we have
in cache.

>  	if (fattr->valid & NFS_ATTR_FATTR_CTIME)
>  		inode->i_ctime = fattr->ctime;
>  	else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
> @@ -2332,6 +2343,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 e1a886b58354..c4717ae4b1b3 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..8e90c5bbf5e4 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 \

Looks good otherwise.

Note that Trond had patches a few years ago that added this support but
they got dropped for some reason. It might be good to follow back up on
that discussion and make sure you address any concerns that were brought
up there.

-- 
Jeff Layton <jlayton@kernel.org>

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

* 回复: [RFC PATCH v1.1] nfs: Add support for the birth time attribute
  2023-08-22 18:16 ` Jeff Layton
@ 2023-08-25 10:32   ` Hanxiao Chen (Fujitsu)
  0 siblings, 0 replies; 3+ messages in thread
From: Hanxiao Chen (Fujitsu) @ 2023-08-25 10:32 UTC (permalink / raw)
  To: Jeff Layton, Trond Myklebust; +Cc: linux-nfs@vger.kernel.org, Anna Schumaker

在 2023/8/23 2:16, Jeff Layton 写道:
> On Tue, 2023-08-22 at 18:00 +0800, Chen Hanxiao wrote:
>> nfsd already support btime by:
>> commit e377a3e698fb ("nfsd: Add support for the birth time attribute")
>>
>> 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>
>>
>> ---
>> 1.1:
>> 	minor fix
>>
>>  fs/nfs/inode.c          | 14 +++++++++++++-
>>  fs/nfs/nfs4proc.c       |  3 +++
>>  fs/nfs/nfs4xdr.c        | 23 +++++++++++++++++++++++
>>  include/linux/nfs_fs.h  |  2 ++
>>  include/linux/nfs_xdr.h |  2 ++
>>  5 files changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>> index 8172dd4135a1..36e322b993f8 100644
>> --- a/fs/nfs/inode.c
>> +++ b/fs/nfs/inode.c
>> @@ -835,6 +835,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 +846,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) {
>> @@ -911,6 +912,10 @@ int nfs_getattr(struct mnt_idmap *idmap, const struct path *path,
>>  out_no_revalidate:
>>  	/* Only return attributes that were revalidated. */
>>  	stat->result_mask = nfs_get_valid_attrmask(inode) | request_mask;
>> +	if (nfsi->btime.tv_sec == 0 && nfsi->btime.tv_nsec == 0)
>> +		stat->result_mask &= ~STATX_BTIME;
>> +	else
>> +		stat->btime = nfsi->btime;
>>  
>>  	generic_fillattr(&nop_mnt_idmap, inode, stat);
>>  	stat->ino = nfs_compat_user_ino64(NFS_FILEID(inode));
>> @@ -2189,6 +2194,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;
>> +
> I'm not sure we actually need the "else" part here, or the INVALID_BTIME
> bit. The btime isn't expected to change, really ever, so if the server
> didn't report it, I think we're generally safe to trust whatever we have
> in cache.
Sure, will be fixed.
>>  	if (fattr->valid & NFS_ATTR_FATTR_CTIME)
>>  		inode->i_ctime = fattr->ctime;
>>  	else if (fattr_supported & NFS_ATTR_FATTR_CTIME)
>> @@ -2332,6 +2343,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 e1a886b58354..c4717ae4b1b3 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..8e90c5bbf5e4 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 \
> Looks good otherwise.
>
> Note that Trond had patches a few years ago that added this support but
> they got dropped for some reason. It might be good to follow back up on
> that discussion and make sure you address any concerns that were brought
> up there.

I googled Trond's patch, found:

[PATCH 0/8] Support btime and otherhttps://lore.kernel.org/linux-nfs/20211217204854.439578-1-trondmy@kernel.org/ NFSv4 specific attributes - trondmy (kernel.org)

IIUC, I didn't find some concerns from reviewers.

Hi, Trond, could you pls give some hints?

I'll try to post my v2 patch if I can address them.

Regards,

- Chen

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

end of thread, other threads:[~2023-08-25 10:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-22 10:00 [RFC PATCH v1.1] nfs: Add support for the birth time attribute Chen Hanxiao
2023-08-22 18:16 ` Jeff Layton
2023-08-25 10:32   ` 回复: " Hanxiao Chen (Fujitsu)

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).