2023-08-08 17:48:38

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v7 07/13] xfs: have xfs_vn_update_time gets its own timestamp

On Mon 07-08-23 15:38:38, Jeff Layton wrote:
> In later patches we're going to drop the "now" parameter from the
> update_time operation. Prepare XFS for this by reworking how it fetches
> timestamps and sets them in the inode. Ensure that we update the ctime
> even if only S_MTIME is set.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/xfs/xfs_iops.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 731f45391baa..72d18e7840f5 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1037,6 +1037,7 @@ xfs_vn_update_time(
> int log_flags = XFS_ILOG_TIMESTAMP;
> struct xfs_trans *tp;
> int error;
> + struct timespec64 now = current_time(inode);

No need to fetch current_time() here where you overwrite it just a bit
later...

> @@ -1056,12 +1057,15 @@ xfs_vn_update_time(
> return error;
>
> xfs_ilock(ip, XFS_ILOCK_EXCL);
> - if (flags & S_CTIME)
> - inode_set_ctime_to_ts(inode, *now);
> + if (flags & (S_CTIME|S_MTIME))
> + now = inode_set_ctime_current(inode);
> + else
> + now = current_time(inode);
> +
> if (flags & S_MTIME)
> - inode->i_mtime = *now;
> + inode->i_mtime = now;
> if (flags & S_ATIME)
> - inode->i_atime = *now;
> + inode->i_atime = now;
>
> xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> xfs_trans_log_inode(tp, ip, log_flags);

Otherwise the patch looks good to me so feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR