On 13 May 2025, at 12:36, Aurélien Couderc wrote:
> On Tue, May 13, 2025 at 6:08 PM Benjamin Coddington <bcodding@xxxxxxxxxx> 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@xxxxxxxxxxx>
>>>
>>> ---
>>> 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@xxxxxxxxxx>
>> Reviewed-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
>
> Reviewed-by: Aurélien Couderc <aurelien.couderc2002@xxxxxxxxx>
>
> 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
