Re: [PATCH 08/11] xfs: widen ondisk timestamps to deal with y2038 problem

Darrick J. Wong
2020-08-18T15:55:58Z

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@gmail.com> wrote:
> >
> > On Tue, Aug 18, 2020 at 1:57 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > >
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > >
> > > 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@oracle.com>
> > > ---
> > ...
> >
> > > 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.

--D

>
> Thanks,
> Amir.