| Message ID | 20230807-mgctime-v7-7-d1dec143a704@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs: implement multigrain timestamps | expand |
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 <jlayton@kernel.org> > --- > 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 <jack@suse.cz> Honza
On Tue, Aug 08, 2023 at 11:39:03AM +0200, Jan Kara wrote: > 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 <jlayton@kernel.org> > > --- > > 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... It also shadows the @now parameter of that function. Since that function parameter is dropped in follow-up patches I simply s/now/time/g it here. In any case, fixed in-tree.
On Mon, Aug 07, 2023 at 03:38:38PM -0400, 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 <jlayton@kernel.org> > --- > 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); > > trace_xfs_update_time(ip); > > @@ -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)) Minor nit: spaces around ^ the operator. Otherwise looks ok to me... Acked-by: Darrick J. Wong <djwong@kernel.org> --D > + 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); > > -- > 2.41.0 >
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); trace_xfs_update_time(ip); @@ -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);
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 <jlayton@kernel.org> --- fs/xfs/xfs_iops.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)