diff mbox series

[v4,6/9] xfs: switch to multigrain timestamps

Message ID 20240708-mgtime-v4-6-a0f3c6fb57f3@kernel.org (mailing list archive)
State New
Headers show
Series fs: multigrain timestamp redux | expand

Commit Message

Jeff Layton July 8, 2024, 3:53 p.m. UTC
Enable multigrain timestamps, which should ensure that there is an
apparent change to the timestamp whenever it has been written after
being actively observed via getattr.

Also, anytime the mtime changes, the ctime must also change, and those
are now the only two options for xfs_trans_ichgtime. Have that function
unconditionally bump the ctime, and ASSERT that XFS_ICHGTIME_CHG is
always set.

Finally, stop setting STATX_CHANGE_COOKIE in getattr, since the ctime
should give us better semantics now.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/xfs/libxfs/xfs_trans_inode.c |  6 +++---
 fs/xfs/xfs_iops.c               | 10 +++-------
 fs/xfs/xfs_super.c              |  2 +-
 3 files changed, 7 insertions(+), 11 deletions(-)

Comments

Darrick J. Wong July 8, 2024, 6:47 p.m. UTC | #1
On Mon, Jul 08, 2024 at 11:53:39AM -0400, Jeff Layton wrote:
> Enable multigrain timestamps, which should ensure that there is an
> apparent change to the timestamp whenever it has been written after
> being actively observed via getattr.
> 
> Also, anytime the mtime changes, the ctime must also change, and those
> are now the only two options for xfs_trans_ichgtime. Have that function
> unconditionally bump the ctime, and ASSERT that XFS_ICHGTIME_CHG is
> always set.
> 
> Finally, stop setting STATX_CHANGE_COOKIE in getattr, since the ctime
> should give us better semantics now.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/xfs/libxfs/xfs_trans_inode.c |  6 +++---
>  fs/xfs/xfs_iops.c               | 10 +++-------
>  fs/xfs/xfs_super.c              |  2 +-
>  3 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
> index 69fc5b981352..1f3639bbf5f0 100644
> --- a/fs/xfs/libxfs/xfs_trans_inode.c
> +++ b/fs/xfs/libxfs/xfs_trans_inode.c
> @@ -62,12 +62,12 @@ xfs_trans_ichgtime(
>  	ASSERT(tp);
>  	xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
>  
> -	tv = current_time(inode);
> +	/* If the mtime changes, then ctime must also change */
> +	ASSERT(flags & XFS_ICHGTIME_CHG);
>  
> +	tv = inode_set_ctime_current(inode);
>  	if (flags & XFS_ICHGTIME_MOD)
>  		inode_set_mtime_to_ts(inode, tv);
> -	if (flags & XFS_ICHGTIME_CHG)
> -		inode_set_ctime_to_ts(inode, tv);
>  	if (flags & XFS_ICHGTIME_CREATE)
>  		ip->i_crtime = tv;
>  }
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index a00dcbc77e12..d25872f818fa 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -592,8 +592,9 @@ xfs_vn_getattr(
>  	stat->gid = vfsgid_into_kgid(vfsgid);
>  	stat->ino = ip->i_ino;
>  	stat->atime = inode_get_atime(inode);
> -	stat->mtime = inode_get_mtime(inode);
> -	stat->ctime = inode_get_ctime(inode);
> +
> +	fill_mg_cmtime(stat, request_mask, inode);

Sooo... for setting up a commit-range operation[1], XFS_IOC_START_COMMIT
could populate its freshness data by calling:

	struct kstat dummy;

	fill_mg_ctime(&dummy, STATX_CTIME | STATX_MTIME, inode);

and then using dummy.[cm]time to populate the freshness data that it
gives to userspace, right?  Having set QUERIED, a write to the file
immediately afterwards will cause a (tiny) increase in ctime_nsec which
will cause the XFS_IOC_COMMIT_RANGE to reject the commit[2].  Right?

--D

[1] https://lore.kernel.org/linux-xfs/20240227174649.GL6184@frogsfrogsfrogs/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/commit/?h=atomic-file-commits&id=0520d89c2698874c1f56ddf52ec4b8a3595baa14

> +
>  	stat->blocks = XFS_FSB_TO_BB(mp, ip->i_nblocks + ip->i_delayed_blks);
>  
>  	if (xfs_has_v3inodes(mp)) {
> @@ -603,11 +604,6 @@ xfs_vn_getattr(
>  		}
>  	}
>  
> -	if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) {
> -		stat->change_cookie = inode_query_iversion(inode);
> -		stat->result_mask |= STATX_CHANGE_COOKIE;
> -	}
> -
>  	/*
>  	 * Note: If you add another clause to set an attribute flag, please
>  	 * update attributes_mask below.
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 27e9f749c4c7..210481b03fdb 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -2052,7 +2052,7 @@ static struct file_system_type xfs_fs_type = {
>  	.init_fs_context	= xfs_init_fs_context,
>  	.parameters		= xfs_fs_parameters,
>  	.kill_sb		= xfs_kill_sb,
> -	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
> +	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME,
>  };
>  MODULE_ALIAS_FS("xfs");
>  
> 
> -- 
> 2.45.2
> 
>
Jeff Layton July 8, 2024, 6:51 p.m. UTC | #2
On Mon, 2024-07-08 at 11:47 -0700, Darrick J. Wong wrote:
> On Mon, Jul 08, 2024 at 11:53:39AM -0400, Jeff Layton wrote:
> > Enable multigrain timestamps, which should ensure that there is an
> > apparent change to the timestamp whenever it has been written after
> > being actively observed via getattr.
> > 
> > Also, anytime the mtime changes, the ctime must also change, and those
> > are now the only two options for xfs_trans_ichgtime. Have that function
> > unconditionally bump the ctime, and ASSERT that XFS_ICHGTIME_CHG is
> > always set.
> > 
> > Finally, stop setting STATX_CHANGE_COOKIE in getattr, since the ctime
> > should give us better semantics now.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/xfs/libxfs/xfs_trans_inode.c |  6 +++---
> >  fs/xfs/xfs_iops.c               | 10 +++-------
> >  fs/xfs/xfs_super.c              |  2 +-
> >  3 files changed, 7 insertions(+), 11 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
> > index 69fc5b981352..1f3639bbf5f0 100644
> > --- a/fs/xfs/libxfs/xfs_trans_inode.c
> > +++ b/fs/xfs/libxfs/xfs_trans_inode.c
> > @@ -62,12 +62,12 @@ xfs_trans_ichgtime(
> >  	ASSERT(tp);
> >  	xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
> >  
> > -	tv = current_time(inode);
> > +	/* If the mtime changes, then ctime must also change */
> > +	ASSERT(flags & XFS_ICHGTIME_CHG);
> >  
> > +	tv = inode_set_ctime_current(inode);
> >  	if (flags & XFS_ICHGTIME_MOD)
> >  		inode_set_mtime_to_ts(inode, tv);
> > -	if (flags & XFS_ICHGTIME_CHG)
> > -		inode_set_ctime_to_ts(inode, tv);
> >  	if (flags & XFS_ICHGTIME_CREATE)
> >  		ip->i_crtime = tv;
> >  }
> > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> > index a00dcbc77e12..d25872f818fa 100644
> > --- a/fs/xfs/xfs_iops.c
> > +++ b/fs/xfs/xfs_iops.c
> > @@ -592,8 +592,9 @@ xfs_vn_getattr(
> >  	stat->gid = vfsgid_into_kgid(vfsgid);
> >  	stat->ino = ip->i_ino;
> >  	stat->atime = inode_get_atime(inode);
> > -	stat->mtime = inode_get_mtime(inode);
> > -	stat->ctime = inode_get_ctime(inode);
> > +
> > +	fill_mg_cmtime(stat, request_mask, inode);
> 
> Sooo... for setting up a commit-range operation[1], XFS_IOC_START_COMMIT
> could populate its freshness data by calling:
> 
> 	struct kstat dummy;
> 
> 	fill_mg_ctime(&dummy, STATX_CTIME | STATX_MTIME, inode);
> 
> and then using dummy.[cm]time to populate the freshness data that it
> gives to userspace, right?  Having set QUERIED, a write to the file
> immediately afterwards will cause a (tiny) increase in ctime_nsec which
> will cause the XFS_IOC_COMMIT_RANGE to reject the commit[2].  Right?
> 

Yes. Once you call fill_mg_ctime, the first write after that point
should cause the kernel to ensure that there is a distinct change in
the ctime.

IOW, I think this should alleviate the concerns I had before with using
timestamps with the XFS_IOC_COMMIT_RANGE interface.


> --D
> 
> [1] https://lore.kernel.org/linux-xfs/20240227174649.GL6184@frogsfrogsfrogs/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/commit/?h=atomic-file-commits&id=0520d89c2698874c1f56ddf52ec4b8a3595baa14
> 
> > +
> >  	stat->blocks = XFS_FSB_TO_BB(mp, ip->i_nblocks + ip->i_delayed_blks);
> >  
> >  	if (xfs_has_v3inodes(mp)) {
> > @@ -603,11 +604,6 @@ xfs_vn_getattr(
> >  		}
> >  	}
> >  
> > -	if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) {
> > -		stat->change_cookie = inode_query_iversion(inode);
> > -		stat->result_mask |= STATX_CHANGE_COOKIE;
> > -	}
> > -
> >  	/*
> >  	 * Note: If you add another clause to set an attribute flag, please
> >  	 * update attributes_mask below.
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index 27e9f749c4c7..210481b03fdb 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -2052,7 +2052,7 @@ static struct file_system_type xfs_fs_type = {
> >  	.init_fs_context	= xfs_init_fs_context,
> >  	.parameters		= xfs_fs_parameters,
> >  	.kill_sb		= xfs_kill_sb,
> > -	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
> > +	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME,
> >  };
> >  MODULE_ALIAS_FS("xfs");
> >  
> > 
> > -- 
> > 2.45.2
> > 
> >
Jeff Layton July 8, 2024, 7 p.m. UTC | #3
On Mon, 2024-07-08 at 14:51 -0400, Jeff Layton wrote:
> On Mon, 2024-07-08 at 11:47 -0700, Darrick J. Wong wrote:
> > On Mon, Jul 08, 2024 at 11:53:39AM -0400, Jeff Layton wrote:
> > > Enable multigrain timestamps, which should ensure that there is an
> > > apparent change to the timestamp whenever it has been written after
> > > being actively observed via getattr.
> > > 
> > > Also, anytime the mtime changes, the ctime must also change, and those
> > > are now the only two options for xfs_trans_ichgtime. Have that function
> > > unconditionally bump the ctime, and ASSERT that XFS_ICHGTIME_CHG is
> > > always set.
> > > 
> > > Finally, stop setting STATX_CHANGE_COOKIE in getattr, since the ctime
> > > should give us better semantics now.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  fs/xfs/libxfs/xfs_trans_inode.c |  6 +++---
> > >  fs/xfs/xfs_iops.c               | 10 +++-------
> > >  fs/xfs/xfs_super.c              |  2 +-
> > >  3 files changed, 7 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
> > > index 69fc5b981352..1f3639bbf5f0 100644
> > > --- a/fs/xfs/libxfs/xfs_trans_inode.c
> > > +++ b/fs/xfs/libxfs/xfs_trans_inode.c
> > > @@ -62,12 +62,12 @@ xfs_trans_ichgtime(
> > >  	ASSERT(tp);
> > >  	xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
> > >  
> > > -	tv = current_time(inode);
> > > +	/* If the mtime changes, then ctime must also change */
> > > +	ASSERT(flags & XFS_ICHGTIME_CHG);
> > >  
> > > +	tv = inode_set_ctime_current(inode);
> > >  	if (flags & XFS_ICHGTIME_MOD)
> > >  		inode_set_mtime_to_ts(inode, tv);
> > > -	if (flags & XFS_ICHGTIME_CHG)
> > > -		inode_set_ctime_to_ts(inode, tv);
> > >  	if (flags & XFS_ICHGTIME_CREATE)
> > >  		ip->i_crtime = tv;
> > >  }
> > > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> > > index a00dcbc77e12..d25872f818fa 100644
> > > --- a/fs/xfs/xfs_iops.c
> > > +++ b/fs/xfs/xfs_iops.c
> > > @@ -592,8 +592,9 @@ xfs_vn_getattr(
> > >  	stat->gid = vfsgid_into_kgid(vfsgid);
> > >  	stat->ino = ip->i_ino;
> > >  	stat->atime = inode_get_atime(inode);
> > > -	stat->mtime = inode_get_mtime(inode);
> > > -	stat->ctime = inode_get_ctime(inode);
> > > +
> > > +	fill_mg_cmtime(stat, request_mask, inode);
> > 
> > Sooo... for setting up a commit-range operation[1], XFS_IOC_START_COMMIT
> > could populate its freshness data by calling:
> > 
> > 	struct kstat dummy;
> > 
> > 	fill_mg_ctime(&dummy, STATX_CTIME | STATX_MTIME, inode);
> > 
> > and then using dummy.[cm]time to populate the freshness data that it
> > gives to userspace, right?  Having set QUERIED, a write to the file
> > immediately afterwards will cause a (tiny) increase in ctime_nsec which
> > will cause the XFS_IOC_COMMIT_RANGE to reject the commit[2].  Right?
> > 
> 
> Yes. Once you call fill_mg_ctime, the first write after that point
> should cause the kernel to ensure that there is a distinct change in
> the ctime.
> 
> IOW, I think this should alleviate the concerns I had before with using
> timestamps with the XFS_IOC_COMMIT_RANGE interface.
> 
> 

Oh, and to be clear, if you're _only_ worried about changes to the
contents of the file (and not the metadata), you should be able to do
this instead:

    fill_mg_ctime(&dummy, STATX_MTIME, inode);

...and that should avoid false positives from metadata-only changes.

Querying only the mtime still causes the QUERIED flag to be set, and
the kernel to give you distinct timestamps.

> > --D
> > 
> > [1] https://lore.kernel.org/linux-xfs/20240227174649.GL6184@frogsfrogsfrogs/
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/commit/?h=atomic-file-commits&id=0520d89c2698874c1f56ddf52ec4b8a3595baa14
> > 
> > > +
> > >  	stat->blocks = XFS_FSB_TO_BB(mp, ip->i_nblocks + ip->i_delayed_blks);
> > >  
> > >  	if (xfs_has_v3inodes(mp)) {
> > > @@ -603,11 +604,6 @@ xfs_vn_getattr(
> > >  		}
> > >  	}
> > >  
> > > -	if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) {
> > > -		stat->change_cookie = inode_query_iversion(inode);
> > > -		stat->result_mask |= STATX_CHANGE_COOKIE;
> > > -	}
> > > -
> > >  	/*
> > >  	 * Note: If you add another clause to set an attribute flag, please
> > >  	 * update attributes_mask below.
> > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > index 27e9f749c4c7..210481b03fdb 100644
> > > --- a/fs/xfs/xfs_super.c
> > > +++ b/fs/xfs/xfs_super.c
> > > @@ -2052,7 +2052,7 @@ static struct file_system_type xfs_fs_type = {
> > >  	.init_fs_context	= xfs_init_fs_context,
> > >  	.parameters		= xfs_fs_parameters,
> > >  	.kill_sb		= xfs_kill_sb,
> > > -	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
> > > +	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME,
> > >  };
> > >  MODULE_ALIAS_FS("xfs");
> > >  
> > > 
> > > -- 
> > > 2.45.2
> > > 
> > > 
>
Darrick J. Wong July 8, 2024, 7:01 p.m. UTC | #4
On Mon, Jul 08, 2024 at 02:51:07PM -0400, Jeff Layton wrote:
> On Mon, 2024-07-08 at 11:47 -0700, Darrick J. Wong wrote:
> > On Mon, Jul 08, 2024 at 11:53:39AM -0400, Jeff Layton wrote:
> > > Enable multigrain timestamps, which should ensure that there is an
> > > apparent change to the timestamp whenever it has been written after
> > > being actively observed via getattr.
> > > 
> > > Also, anytime the mtime changes, the ctime must also change, and those
> > > are now the only two options for xfs_trans_ichgtime. Have that function
> > > unconditionally bump the ctime, and ASSERT that XFS_ICHGTIME_CHG is
> > > always set.
> > > 
> > > Finally, stop setting STATX_CHANGE_COOKIE in getattr, since the ctime
> > > should give us better semantics now.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  fs/xfs/libxfs/xfs_trans_inode.c |  6 +++---
> > >  fs/xfs/xfs_iops.c               | 10 +++-------
> > >  fs/xfs/xfs_super.c              |  2 +-
> > >  3 files changed, 7 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
> > > index 69fc5b981352..1f3639bbf5f0 100644
> > > --- a/fs/xfs/libxfs/xfs_trans_inode.c
> > > +++ b/fs/xfs/libxfs/xfs_trans_inode.c
> > > @@ -62,12 +62,12 @@ xfs_trans_ichgtime(
> > >  	ASSERT(tp);
> > >  	xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
> > >  
> > > -	tv = current_time(inode);
> > > +	/* If the mtime changes, then ctime must also change */
> > > +	ASSERT(flags & XFS_ICHGTIME_CHG);
> > >  
> > > +	tv = inode_set_ctime_current(inode);
> > >  	if (flags & XFS_ICHGTIME_MOD)
> > >  		inode_set_mtime_to_ts(inode, tv);
> > > -	if (flags & XFS_ICHGTIME_CHG)
> > > -		inode_set_ctime_to_ts(inode, tv);
> > >  	if (flags & XFS_ICHGTIME_CREATE)
> > >  		ip->i_crtime = tv;
> > >  }
> > > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> > > index a00dcbc77e12..d25872f818fa 100644
> > > --- a/fs/xfs/xfs_iops.c
> > > +++ b/fs/xfs/xfs_iops.c
> > > @@ -592,8 +592,9 @@ xfs_vn_getattr(
> > >  	stat->gid = vfsgid_into_kgid(vfsgid);
> > >  	stat->ino = ip->i_ino;
> > >  	stat->atime = inode_get_atime(inode);
> > > -	stat->mtime = inode_get_mtime(inode);
> > > -	stat->ctime = inode_get_ctime(inode);
> > > +
> > > +	fill_mg_cmtime(stat, request_mask, inode);
> > 
> > Sooo... for setting up a commit-range operation[1], XFS_IOC_START_COMMIT
> > could populate its freshness data by calling:
> > 
> > 	struct kstat dummy;
> > 
> > 	fill_mg_ctime(&dummy, STATX_CTIME | STATX_MTIME, inode);
> > 
> > and then using dummy.[cm]time to populate the freshness data that it
> > gives to userspace, right?  Having set QUERIED, a write to the file
> > immediately afterwards will cause a (tiny) increase in ctime_nsec which
> > will cause the XFS_IOC_COMMIT_RANGE to reject the commit[2].  Right?
> > 
> 
> Yes. Once you call fill_mg_ctime, the first write after that point
> should cause the kernel to ensure that there is a distinct change in
> the ctime.
> 
> IOW, I think this should alleviate the concerns I had before with using
> timestamps with the XFS_IOC_COMMIT_RANGE interface.

Cool, thank you!  Apologies for roaring earlier.

--D

> > --D
> > 
> > [1] https://lore.kernel.org/linux-xfs/20240227174649.GL6184@frogsfrogsfrogs/
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/commit/?h=atomic-file-commits&id=0520d89c2698874c1f56ddf52ec4b8a3595baa14
> > 
> > > +
> > >  	stat->blocks = XFS_FSB_TO_BB(mp, ip->i_nblocks + ip->i_delayed_blks);
> > >  
> > >  	if (xfs_has_v3inodes(mp)) {
> > > @@ -603,11 +604,6 @@ xfs_vn_getattr(
> > >  		}
> > >  	}
> > >  
> > > -	if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) {
> > > -		stat->change_cookie = inode_query_iversion(inode);
> > > -		stat->result_mask |= STATX_CHANGE_COOKIE;
> > > -	}
> > > -
> > >  	/*
> > >  	 * Note: If you add another clause to set an attribute flag, please
> > >  	 * update attributes_mask below.
> > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > > index 27e9f749c4c7..210481b03fdb 100644
> > > --- a/fs/xfs/xfs_super.c
> > > +++ b/fs/xfs/xfs_super.c
> > > @@ -2052,7 +2052,7 @@ static struct file_system_type xfs_fs_type = {
> > >  	.init_fs_context	= xfs_init_fs_context,
> > >  	.parameters		= xfs_fs_parameters,
> > >  	.kill_sb		= xfs_kill_sb,
> > > -	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
> > > +	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME,
> > >  };
> > >  MODULE_ALIAS_FS("xfs");
> > >  
> > > 
> > > -- 
> > > 2.45.2
> > > 
> > > 
> 
> -- 
> Jeff Layton <jlayton@kernel.org>
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
index 69fc5b981352..1f3639bbf5f0 100644
--- a/fs/xfs/libxfs/xfs_trans_inode.c
+++ b/fs/xfs/libxfs/xfs_trans_inode.c
@@ -62,12 +62,12 @@  xfs_trans_ichgtime(
 	ASSERT(tp);
 	xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
 
-	tv = current_time(inode);
+	/* If the mtime changes, then ctime must also change */
+	ASSERT(flags & XFS_ICHGTIME_CHG);
 
+	tv = inode_set_ctime_current(inode);
 	if (flags & XFS_ICHGTIME_MOD)
 		inode_set_mtime_to_ts(inode, tv);
-	if (flags & XFS_ICHGTIME_CHG)
-		inode_set_ctime_to_ts(inode, tv);
 	if (flags & XFS_ICHGTIME_CREATE)
 		ip->i_crtime = tv;
 }
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index a00dcbc77e12..d25872f818fa 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -592,8 +592,9 @@  xfs_vn_getattr(
 	stat->gid = vfsgid_into_kgid(vfsgid);
 	stat->ino = ip->i_ino;
 	stat->atime = inode_get_atime(inode);
-	stat->mtime = inode_get_mtime(inode);
-	stat->ctime = inode_get_ctime(inode);
+
+	fill_mg_cmtime(stat, request_mask, inode);
+
 	stat->blocks = XFS_FSB_TO_BB(mp, ip->i_nblocks + ip->i_delayed_blks);
 
 	if (xfs_has_v3inodes(mp)) {
@@ -603,11 +604,6 @@  xfs_vn_getattr(
 		}
 	}
 
-	if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) {
-		stat->change_cookie = inode_query_iversion(inode);
-		stat->result_mask |= STATX_CHANGE_COOKIE;
-	}
-
 	/*
 	 * Note: If you add another clause to set an attribute flag, please
 	 * update attributes_mask below.
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 27e9f749c4c7..210481b03fdb 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -2052,7 +2052,7 @@  static struct file_system_type xfs_fs_type = {
 	.init_fs_context	= xfs_init_fs_context,
 	.parameters		= xfs_fs_parameters,
 	.kill_sb		= xfs_kill_sb,
-	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
+	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME,
 };
 MODULE_ALIAS_FS("xfs");