On Sun, May 31, 2020 at 03:30:42PM +0300, Amir Goldstein wrote:
> > @@ -265,17 +278,35 @@ xfs_inode_from_disk(
> > if (to->di_version == 3) {
> > inode_set_iversion_queried(inode,
> > be64_to_cpu(from->di_changecount));
> > - xfs_inode_from_disk_timestamp(&to->di_crtime, &from->di_crtime);
> > + xfs_inode_from_disk_timestamp(from, &to->di_crtime,
> > + &from->di_crtime);
> > to->di_flags2 = be64_to_cpu(from->di_flags2);
> > to->di_cowextsize = be32_to_cpu(from->di_cowextsize);
> > + /*
> > + * Convert this inode's timestamps to bigtime format the next
> > + * time we write it out to disk.
> > + */
> > + if (xfs_sb_version_hasbigtime(&mp->m_sb))
> > + to->di_flags2 |= XFS_DIFLAG2_BIGTIME;
> > }
>
> This feels wrong.
> incore inode has a union for timestamp.
> This flag should indicate how the union should be interpreted
> otherwise it is going to be very easy to stumble on that in future code.
Hm? I think you've gotten it backwards.
The incore inode (struct xfs_icdinode) has a struct timespec64.
The ondisk inode (struct xfs_dinode) has a union xfs_timestamp.
xfs_inode_from_disk_timestamp uses the ondisk inode (from) to convert
the ondisk timestamp (&from->di_crtime) to the incore inode
(&to->di_crtime).
In other words, we use the ondisk flag and union to load the ondisk
timestamp into a format-agnostic incore structure. Then we set BIGTIME
in the incore inode (to->di_flags2).
If and when we write the inode back out to disk, we'll see that BIGTIME
is set in the incore inode, and convert the incore structure into the
bigtime encoding on disk.
> So either convert incore timestamp now or check hasbigtime when
> we write to disk.
That's more or less what this is doing. We read the timestamp in from
disk in whatever format it's in, and then set ourselves up to write it
back out in bigtime format.
--D
> Thanks,
> Amir.
