Re: [PATCH 06/15] xfs: bind together the front and back ends of the file range exchange code

Dave Chinner
2024-04-08T00:05:22Z

On Tue, Mar 26, 2024 at 06:54:24PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> So far, we've constructed the front end of the file range exchange code
> that does all the checking; and the back end of the file mapping
> exchange code that actually does the work. Glue these two pieces
> together so that we can turn on the functionality.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_exchrange.c | 364 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_exchrange.h | 1
> fs/xfs/xfs_mount.h | 5 +
> fs/xfs/xfs_trace.c | 1
> fs/xfs/xfs_trace.h | 163 +++++++++++++++++++++
> 5 files changed, 532 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_exchrange.c b/fs/xfs/xfs_exchrange.c
> index ff8a2712f657c..1642ea1bd7b30 100644
> --- a/fs/xfs/xfs_exchrange.c
> +++ b/fs/xfs/xfs_exchrange.c
> @@ -12,8 +12,15 @@
> #include "xfs_defer.h"
> #include "xfs_inode.h"
> #include "xfs_trans.h"
> +#include "xfs_quota.h"
> +#include "xfs_bmap_util.h"
> +#include "xfs_reflink.h"
> +#include "xfs_trace.h"
> #include "xfs_exchrange.h"
> #include "xfs_exchmaps.h"
> +#include "xfs_sb.h"
> +#include "xfs_icache.h"
> +#include "xfs_log.h"
> #include <linux/fsnotify.h>
>
> /*
> @@ -47,6 +54,34 @@ xfs_exchrange_possible(
> xfs_can_add_incompat_log_features(mp, false);
> }
>
> +/*
> + * Get permission to use log-assisted atomic exchange of file mappings.
> + * Callers must not be running any transactions or hold any ILOCKs.
> + */
> +int
> +xfs_exchrange_enable(
> + struct xfs_mount *mp)
> +{
> + int error = 0;
> +
> + /* Mapping exchange log intent items are already enabled */
> + if (xfs_sb_version_haslogexchmaps(&mp->m_sb))
> + return 0;

I must have missed this earlier. We don't do "sb_version" checks on
the superblock anymore - that wrapper should be removed from
whatever patch introduced it. Enabled features set a feature bit on
the xfs_mount when the feature is set/detected and the code runs
xfs_has_foo(mp) against the mount, not the superblock.

i.e. Please define a XFS_FEAT_EXCHMAPS field and set that appropriately
when enabling the feature and check it here...

> +
> + if (!xfs_exchrange_upgradeable(mp))
> + return -EOPNOTSUPP;
> +
> + error = xfs_add_incompat_log_feature(mp,
> + XFS_SB_FEAT_INCOMPAT_LOG_EXCHMAPS);
> + if (error)
> + return error;
> +
> + xfs_warn_mount(mp, XFS_OPSTATE_WARNED_EXCHMAPS,
> + "EXPERIMENTAL atomic file range exchange feature in use. Use at your own risk!");

This doesn't need to use xfs_warn_mount() because we can only turn
the feature on once per mount now. Hence the opstate bit can go away
and it can just use xfs_warn()....

-Dave.
--
Dave Chinner
david@fromorbit.com