On Tue, Aug 18, 2020 at 08:53:45AM -0700, Darrick J. Wong wrote: > On Tue, Aug 18, 2020 at 03:53:57PM +0300, Amir Goldstein wrote: > > On Tue, Aug 18, 2020 at 3:00 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > > On Tue, Aug 18, 2020 at 1:57 AM Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote: > > > > > > > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > > > > > > Redesign the ondisk timestamps to be a simple unsigned 64-bit counter of > > > > nanoseconds since 14 Dec 1901 (i.e. the minimum time in the 32-bit unix > > > > time epoch). This enables us to handle dates up to 2486, which solves > > > > the y2038 problem. > > > > > > > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > > --- > > > ... > > > > > > > diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c > > > > index 9f036053fdb7..b354825f4e51 100644 > > > > --- a/fs/xfs/scrub/inode.c > > > > +++ b/fs/xfs/scrub/inode.c > > > > @@ -190,6 +190,11 @@ xchk_inode_flags2( > > > > if ((flags2 & XFS_DIFLAG2_DAX) && (flags2 & XFS_DIFLAG2_REFLINK)) > > > > goto bad; > > > > > > > > + /* the incore bigtime iflag always follows the feature flag */ > > > > + if (!!xfs_sb_version_hasbigtime(&mp->m_sb) ^ > > > > + !!(flags2 & XFS_DIFLAG2_BIGTIME)) > > > > + goto bad; > > > > + > > > > > > Seems like flags2 are not the incore iflags and that the dinode iflags > > > can very well > > > have no bigtime on fs with bigtime support: > > > > > > xchk_dinode(... > > > ... > > > flags2 = be64_to_cpu(dip->di_flags2); > > > > > > What am I missing? > > > > > > > Another question. Do we need to worry about xfs_reinit_inode()? > > It seems not because incore inode should already have the correct bigtime > > flag. Right? > > I sure hope so. Any inode being fed to xfs_reinit_inode was fully read > into memory, then we tore down the VFS inode, but before we tore down > the XFS inode, someone wanted it back, so all we have to do is reset > the VFS part of the incore state. > > Therefore, we don't have to do anything about the XFS part of the incore > state. :) > > > But by same logic, xfs_ifree() should already have the correct > > bigtime flag as well, so we don't need to set the flag, maybe assert the flag? > > Right. I think that piece can go since we set the incore flag > opportunistically now. Bleh. I missed the subtlety of: ip->i_d.di_flags2 = 0; if (xfs_sb_version_hasbigtime(&ip->i_mount->m_sb)) ip->i_d.di_flags2 |= XFS_DIFLAG2_BIGTIME; First we zero flags2 entirely, then we re-enable bigtime so that the final ctime update is written in the correct format. I guess that could be simplified to: /* * Preserve the bigtime flag so that ctime accurately stores the * deletion time. */ ip->i_d.di_flags2 &= ~XFS_DIFLAG2_BIGTIME; > --D > > > > > Thanks, > > Amir.