On Fri, Jul 14, 2023 at 06:53:45AM -0400, Jeff Layton wrote: > On Thu, 2023-07-13 at 23:35 -0700, Darrick J. Wong wrote: > > On Thu, Jul 13, 2023 at 08:15:21PM -0400, Jeff Layton wrote: > > > On Fri, 2023-07-14 at 01:53 +0200, Anthony Iliopoulos wrote: > > > > On Thu, Jul 13, 2023 at 07:00:54PM -0400, Jeff Layton wrote: > > > > > Nothing ever sets this flag, which makes sense since the create time is > > > > > set at inode instantiation and is never changed. Remove it and the > > > > > handling of it in xfs_trans_ichgtime. > > > > > > > > It is currently used by xfs_repair during recreating the root inode and > > > > the internal realtime inodes when needed (libxfs is exported to xfsprogs > > > > so there are userspace consumers of this code). > > > > > > > > > > Ahh thanks. I didn't think to look at userland for this. We can drop > > > this patch, and I'll respin #6. > > > > > > Looking briefly at xfsprogs, it looks like XFS_ICHGTIME_CREATE is never > > > set without also setting XFS_ICHGTIME_CHG. Is that safe assumption? > > > > There are four timestamps in an xfs inode and an ICHGTIME flag for each: > > MOD is mtime, CHG is ctime, CREATE is crtime/btime, and ACCESS is atime. > > I'd rather leave it that way than tie flags together. > > > > > > I wasn't suggesting to tie any flags together. I just don't see any > scenario where it's OK to call xfs_trans_ichgtime() without > XFS_ICHGTIME_CHG set. It has to change if either of the other times > change. Oh! That's correct, I don't know of any place where the [bam]time get updated without also bumping ctime. --D > > > > > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > > > > --- > > > > > fs/xfs/libxfs/xfs_shared.h | 2 -- > > > > > fs/xfs/libxfs/xfs_trans_inode.c | 2 -- > > > > > 2 files changed, 4 deletions(-) > > > > > > > > > > diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h > > > > > index c4381388c0c1..8989fff21723 100644 > > > > > --- a/fs/xfs/libxfs/xfs_shared.h > > > > > +++ b/fs/xfs/libxfs/xfs_shared.h > > > > > @@ -126,8 +126,6 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp, > > > > > */ > > > > > #define XFS_ICHGTIME_MOD 0x1 /* data fork modification timestamp */ > > > > > #define XFS_ICHGTIME_CHG 0x2 /* inode field change timestamp */ > > > > > -#define XFS_ICHGTIME_CREATE 0x4 /* inode create timestamp */ > > > > > - > > > > > > > > > > /* > > > > > * Symlink decoding/encoding functions > > > > > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c > > > > > index 6b2296ff248a..0c9df8df6d4a 100644 > > > > > --- a/fs/xfs/libxfs/xfs_trans_inode.c > > > > > +++ b/fs/xfs/libxfs/xfs_trans_inode.c > > > > > @@ -68,8 +68,6 @@ xfs_trans_ichgtime( > > > > > inode->i_mtime = tv; > > > > > if (flags & XFS_ICHGTIME_CHG) > > > > > inode_set_ctime_to_ts(inode, tv); > > > > > - if (flags & XFS_ICHGTIME_CREATE) > > > > > - ip->i_crtime = tv; > > > > > } > > > > > > > > > > /* > > > > > > > > > > -- > > > > > 2.41.0 > > > > > > > > > > > > > > > > > > > -- > > > Jeff Layton <jlayton@xxxxxxxxxx> > > -- > Jeff Layton <jlayton@xxxxxxxxxx>
