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

[Amir Goldstein <amir73il@gmail.com>]

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.

Thanks,
Amir.