diff mbox series

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

Message ID 20240918053117.774001-7-hch@lst.de (mailing list archive)
State Accepted, archived
Headers show
Series [1/8] xfs: merge xfs_attr_leaf_try_add into xfs_attr_leaf_addname | expand

Commit Message

Christoph Hellwig Sept. 18, 2024, 5:30 a.m. UTC
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>
---
 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(-)

Comments

Darrick J. Wong Sept. 18, 2024, 3:40 p.m. UTC | #1
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
> 
>
diff mbox series

Patch

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