Re: [PATCH 6/8] xfs: don't ifdef around the exact minlen allocations

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Sep 18, 2024 at 07:30:08AM +0200, Christoph Hellwig wrote:
> Exact minlen allocations only exist as an error injection tool for debug
> builds.  Currently this is implemented using ifdefs, which means the code
> isn't even compiled for non-XFS_DEBUG builds.  Enhance the compile test
> coverage by always building the code and use the compilers' dead code
> elimination to remove it from the generated binary instead.
> 
> The only downside is that the alloc_minlen_only field is unconditionally
> added to struct xfs_alloc_args now, but by moving it around and packing
> it tightly this doesn't actually increase the size of the structure.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good now,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_alloc.c | 7 ++-----
>  fs/xfs/libxfs/xfs_alloc.h | 4 +---
>  fs/xfs/libxfs/xfs_bmap.c  | 6 ------
>  3 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 59326f84f6a571..04f64cf9777e21 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2766,7 +2766,6 @@ xfs_alloc_commit_autoreap(
>  		xfs_defer_item_unpause(tp, aarp->dfp);
>  }
>  
> -#ifdef DEBUG
>  /*
>   * Check if an AGF has a free extent record whose length is equal to
>   * args->minlen.
> @@ -2806,7 +2805,6 @@ xfs_exact_minlen_extent_available(
>  
>  	return error;
>  }
> -#endif
>  
>  /*
>   * Decide whether to use this allocation group for this allocation.
> @@ -2880,15 +2878,14 @@ xfs_alloc_fix_freelist(
>  	if (!xfs_alloc_space_available(args, need, alloc_flags))
>  		goto out_agbp_relse;
>  
> -#ifdef DEBUG
> -	if (args->alloc_minlen_only) {
> +	if (IS_ENABLED(CONFIG_XFS_DEBUG) && args->alloc_minlen_only) {
>  		int stat;
>  
>  		error = xfs_exact_minlen_extent_available(args, agbp, &stat);
>  		if (error || !stat)
>  			goto out_agbp_relse;
>  	}
> -#endif
> +
>  	/*
>  	 * Make the freelist shorter if it's too long.
>  	 *
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index fae170825be064..0165452e7cd055 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -53,11 +53,9 @@ typedef struct xfs_alloc_arg {
>  	int		datatype;	/* mask defining data type treatment */
>  	char		wasdel;		/* set if allocation was prev delayed */
>  	char		wasfromfl;	/* set if allocation is from freelist */
> +	bool		alloc_minlen_only; /* allocate exact minlen extent */
>  	struct xfs_owner_info	oinfo;	/* owner of blocks being allocated */
>  	enum xfs_ag_resv_type	resv;	/* block reservation to use */
> -#ifdef DEBUG
> -	bool		alloc_minlen_only; /* allocate exact minlen extent */
> -#endif
>  } xfs_alloc_arg_t;
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index d5a8403b469b9d..5263b66bbd3c60 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3477,7 +3477,6 @@ xfs_bmap_process_allocated_extent(
>  	xfs_bmap_alloc_account(ap);
>  }
>  
> -#ifdef DEBUG
>  static int
>  xfs_bmap_exact_minlen_extent_alloc(
>  	struct xfs_bmalloca	*ap)
> @@ -3539,11 +3538,6 @@ xfs_bmap_exact_minlen_extent_alloc(
>  
>  	return 0;
>  }
> -#else
> -
> -#define xfs_bmap_exact_minlen_extent_alloc(bma) (-EFSCORRUPTED)
> -
> -#endif
>  
>  /*
>   * If we are not low on available data blocks and we are allocating at
> -- 
> 2.45.2
> 
>