On 28 May 2025, at 8:56, Jeff Layton wrote: > On Wed, 2025-05-28 at 08:50 -0400, Benjamin Coddington wrote: >> From: Anne Marie Merritt <annemarie.merritt@xxxxxxxxxxxxxxx> >> >> Add tracking of the create time (a.k.a. btime) along with corresponding >> bitfields, request, and decode xdr routines. >> >> Signed-off-by: Anne Marie Merritt <annemarie.merritt@xxxxxxxxxxxxxxx> >> Signed-off-by: Lance Shelton <lance.shelton@xxxxxxxxxxxxxxx> >> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> >> --- >> fs/nfs/inode.c | 28 ++++++++++++++++++++++------ >> fs/nfs/nfs4proc.c | 14 +++++++++++++- >> fs/nfs/nfs4xdr.c | 24 ++++++++++++++++++++++++ >> fs/nfs/nfstrace.h | 3 ++- >> include/linux/nfs_fs.h | 7 +++++++ >> include/linux/nfs_xdr.h | 3 +++ >> 6 files changed, 71 insertions(+), 8 deletions(-) >> >> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c >> index 160f3478a835..fd84c24963b3 100644 >> --- a/fs/nfs/inode.c >> +++ b/fs/nfs/inode.c >> @@ -197,6 +197,7 @@ void nfs_set_cache_invalid(struct inode *inode, unsigned long flags) >> if (!(flags & NFS_INO_REVAL_FORCED)) >> flags &= ~(NFS_INO_INVALID_MODE | >> NFS_INO_INVALID_OTHER | >> + NFS_INO_INVALID_BTIME | >> NFS_INO_INVALID_XATTR); >> flags &= ~(NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_SIZE); >> } >> @@ -522,6 +523,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr) >> inode_set_atime(inode, 0, 0); >> inode_set_mtime(inode, 0, 0); >> inode_set_ctime(inode, 0, 0); >> + memset(&nfsi->btime, 0, sizeof(nfsi->btime)); >> inode_set_iversion_raw(inode, 0); >> inode->i_size = 0; >> clear_nlink(inode); >> @@ -545,6 +547,10 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr) >> inode_set_ctime_to_ts(inode, fattr->ctime); >> else if (fattr_supported & NFS_ATTR_FATTR_CTIME) >> nfs_set_cache_invalid(inode, NFS_INO_INVALID_CTIME); >> + if (fattr->valid & NFS_ATTR_FATTR_BTIME) >> + nfsi->btime = fattr->btime; >> + else if (fattr_supported & NFS_ATTR_FATTR_BTIME) >> + nfs_set_cache_invalid(inode, NFS_INO_INVALID_BTIME); >> if (fattr->valid & NFS_ATTR_FATTR_CHANGE) >> inode_set_iversion_raw(inode, fattr->change_attr); >> else >> @@ -1900,7 +1906,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; >> >> @@ -2219,10 +2225,13 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) >> nfs_fattr_fixup_delegated(inode, fattr); >> >> save_cache_validity = nfsi->cache_validity; >> - nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR >> - | NFS_INO_INVALID_ATIME >> - | NFS_INO_REVAL_FORCED >> - | NFS_INO_INVALID_BLOCKS); >> + nfsi->cache_validity &= >> + ~(NFS_INO_INVALID_ATIME | NFS_INO_REVAL_FORCED | >> + NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_CTIME | >> + NFS_INO_INVALID_MTIME | NFS_INO_INVALID_SIZE | >> + NFS_INO_INVALID_OTHER | NFS_INO_INVALID_BLOCKS | >> + NFS_INO_INVALID_NLINK | NFS_INO_INVALID_MODE | >> + NFS_INO_INVALID_BTIME); >> > > The delta above is a little curious. This patch is just adding > NFS_INO_INVALID_BTIME, but the patch above adds the clearing of several > other bits as well. Why does this logic change? Good point, I wonder if it was based on other attribute work at the time, the original was here: https://lore.kernel.org/linux-nfs/20211227190504.309612-3-trondmy@xxxxxxxxxx/ So I think what we're doing here is clearing what we know we don't have to check/update - I think we can drop this entire hunk, its a minor optimization, but hopefully we can get some expert opinion. Trond, do you want me to test with this hunk removed? Ben
