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

[Dave Chinner <david@fromorbit.com>]

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