Re: [PATCH 06/11] xfs: refactor inode timestamp coding

Darrick J. Wong
2020-08-18T15:42:12Z

On Tue, Aug 18, 2020 at 02:20:22PM +0300, Amir Goldstein 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>
> >
> > Refactor inode timestamp encoding and decoding into helper functions so
> > that we can add extra behaviors in subsequent patches.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>
> With nit below...
>
> ...
>
> > diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> > index d95a00376fad..c2f9a0adeed2 100644
> > --- a/fs/xfs/xfs_inode_item.c
> > +++ b/fs/xfs/xfs_inode_item.c
> > @@ -295,6 +295,15 @@ xfs_inode_item_format_attr_fork(
> > }
> > }
> >
> > +static inline void
> > +xfs_from_log_timestamp(
> > + struct xfs_timestamp *ts,
> > + const struct xfs_ictimestamp *its)
> > +{
> > + ts->t_sec = cpu_to_be32(its->t_sec);
> > + ts->t_nsec = cpu_to_be32(its->t_nsec);
> > +}
> > +
> > void
> > xfs_log_dinode_to_disk(
>
> Following convention of xfs_inode_{to,from}_disk_timestamp()
> I think it would be less confusing to name these helpers
> xfs_log_to_disk_timestamp()
>
> and...
>
> >
> > +static inline void
> > +xfs_to_log_timestamp(
> > + struct xfs_ictimestamp *its,
> > + const struct timespec64 *ts)
> > +{
> > + its->t_sec = ts->tv_sec;
> > + its->t_nsec = ts->tv_nsec;
> > +}
> > +
> > static void
> > xfs_inode_to_log_dinode(
>
> xfs_inode_to_log_timestamp()
>
> Because xfs_{to,from}_log_timestamp() may sound like a matching pair,
> to your average code reviewer, but they are not.

Ok, will do.

--D

> Thanks,
> Amir.