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 <[email protected]>
>
> ---
> 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 <[email protected]>
?? 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 <[email protected]>
>>
>> ---
>> 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/[email protected]/ 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