Re: [PATCH v5 5/8] xfs: XFS_ICHGTIME_CREATE is unused

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux