diff mbox series

[5/6] xfs: call xfs_bmap_exact_minlen_extent_alloc from xfs_bmap_btalloc

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

Commit Message

Christoph Hellwig Aug. 20, 2024, 5:04 p.m. UTC 
xfs_bmap_exact_minlen_extent_alloc duplicates the args setup in
xfs_bmap_btalloc.  Switch to call it from xfs_bmap_btalloc after
doing the basic setup.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c | 61 +++++++++-------------------------------
 1 file changed, 13 insertions(+), 48 deletions(-)

Comments

Christoph Hellwig Aug. 21, 2024, 8:24 a.m. UTC | #1 
On Tue, Aug 20, 2024 at 07:04:56PM +0200, Christoph Hellwig wrote:
> xfs_bmap_exact_minlen_extent_alloc duplicates the args setup in
> xfs_bmap_btalloc.  Switch to call it from xfs_bmap_btalloc after
> doing the basic setup.

And the buildbot correctly points out that we want the following fixup
for non-DEBUG builds:

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 03b23875330734..0f793673d73dcb 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3543,9 +3543,13 @@ xfs_bmap_exact_minlen_extent_alloc(
 	return xfs_bmap_btalloc_low_space(ap, args);
 }
 #else
-
-#define xfs_bmap_exact_minlen_extent_alloc(bma) (-EFSCORRUPTED)
-
+static int
+xfs_bmap_exact_minlen_extent_alloc(
+	struct xfs_bmalloca	*ap,
+	struct xfs_alloc_arg	*args)
+{
+	BUILD_BUG_ON(1);
+}
 #endif
 
 /*
Darrick J. Wong Aug. 21, 2024, 4:04 p.m. UTC | #2 
On Tue, Aug 20, 2024 at 07:04:56PM +0200, Christoph Hellwig wrote:
> xfs_bmap_exact_minlen_extent_alloc duplicates the args setup in
> xfs_bmap_btalloc.  Switch to call it from xfs_bmap_btalloc after
> doing the basic setup.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

AFAICT this looks good, though it might be useful to get an ack from
Chandan since he wrote the minlen allocator and might know about any
weird subtleties.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 61 +++++++++-------------------------------
>  1 file changed, 13 insertions(+), 48 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 1db9d084a44c47..b5eeaea164ee46 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3471,28 +3471,17 @@ xfs_bmap_process_allocated_extent(
>  #ifdef DEBUG
>  static int
>  xfs_bmap_exact_minlen_extent_alloc(
> -	struct xfs_bmalloca	*ap)
> +	struct xfs_bmalloca	*ap,
> +	struct xfs_alloc_arg	*args)
>  {
> -	struct xfs_mount	*mp = ap->ip->i_mount;
> -	struct xfs_alloc_arg	args = { .tp = ap->tp, .mp = mp };
> -	xfs_fileoff_t		orig_offset;
> -	xfs_extlen_t		orig_length;
> -	int			error;
> -
> -	ASSERT(ap->length);
> -
>  	if (ap->minlen != 1) {
> -		ap->blkno = NULLFSBLOCK;
> -		ap->length = 0;
> +		args->fsbno = NULLFSBLOCK;
>  		return 0;
>  	}
>  
> -	orig_offset = ap->offset;
> -	orig_length = ap->length;
> -
> -	args.alloc_minlen_only = 1;
> -
> -	xfs_bmap_compute_alignments(ap, &args);
> +	args->alloc_minlen_only = 1;
> +	args->minlen = args->maxlen = ap->minlen;
> +	args->total = ap->total;
>  
>  	/*
>  	 * Unlike the longest extent available in an AG, we don't track
> @@ -3502,33 +3491,9 @@ xfs_bmap_exact_minlen_extent_alloc(
>  	 * we need not be concerned about a drop in performance in
>  	 * "debug only" code paths.
>  	 */
> -	ap->blkno = XFS_AGB_TO_FSB(mp, 0, 0);
> +	ap->blkno = XFS_AGB_TO_FSB(ap->ip->i_mount, 0, 0);
>  
> -	args.oinfo = XFS_RMAP_OINFO_SKIP_UPDATE;
> -	args.minlen = args.maxlen = ap->minlen;
> -	args.total = ap->total;
> -
> -	args.alignment = 1;
> -	args.minalignslop = 0;
> -
> -	args.minleft = ap->minleft;
> -	args.wasdel = ap->wasdel;
> -	args.resv = XFS_AG_RESV_NONE;
> -	args.datatype = ap->datatype;
> -
> -	error = xfs_alloc_vextent_first_ag(&args, ap->blkno);
> -	if (error)
> -		return error;
> -
> -	if (args.fsbno != NULLFSBLOCK) {
> -		xfs_bmap_process_allocated_extent(ap, &args, orig_offset,
> -			orig_length);
> -	} else {
> -		ap->blkno = NULLFSBLOCK;
> -		ap->length = 0;
> -	}
> -
> -	return 0;
> +	return xfs_alloc_vextent_first_ag(args, ap->blkno);
>  }
>  #else
>  
> @@ -3792,8 +3757,11 @@ xfs_bmap_btalloc(
>  	/* Trim the allocation back to the maximum an AG can fit. */
>  	args.maxlen = min(ap->length, mp->m_ag_max_usable);
>  
> -	if ((ap->datatype & XFS_ALLOC_USERDATA) &&
> -	    xfs_inode_is_filestream(ap->ip))
> +	if (unlikely(XFS_TEST_ERROR(false, mp,
> +			XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT)))
> +		error = xfs_bmap_exact_minlen_extent_alloc(ap, &args);
> +	else if ((ap->datatype & XFS_ALLOC_USERDATA) &&
> +			xfs_inode_is_filestream(ap->ip))
>  		error = xfs_bmap_btalloc_filestreams(ap, &args, stripe_align);
>  	else
>  		error = xfs_bmap_btalloc_best_length(ap, &args, stripe_align);
> @@ -4208,9 +4176,6 @@ xfs_bmapi_allocate(
>  	if ((bma->datatype & XFS_ALLOC_USERDATA) &&
>  	    XFS_IS_REALTIME_INODE(bma->ip))
>  		error = xfs_bmap_rtalloc(bma);
> -	else if (unlikely(XFS_TEST_ERROR(false, mp,
> -			XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT)))
> -		error = xfs_bmap_exact_minlen_extent_alloc(bma);
>  	else
>  		error = xfs_bmap_btalloc(bma);
>  	if (error)
> -- 
> 2.43.0
> 
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 1db9d084a44c47..b5eeaea164ee46 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3471,28 +3471,17 @@  xfs_bmap_process_allocated_extent(
 #ifdef DEBUG
 static int
 xfs_bmap_exact_minlen_extent_alloc(
-	struct xfs_bmalloca	*ap)
+	struct xfs_bmalloca	*ap,
+	struct xfs_alloc_arg	*args)
 {
-	struct xfs_mount	*mp = ap->ip->i_mount;
-	struct xfs_alloc_arg	args = { .tp = ap->tp, .mp = mp };
-	xfs_fileoff_t		orig_offset;
-	xfs_extlen_t		orig_length;
-	int			error;
-
-	ASSERT(ap->length);
-
 	if (ap->minlen != 1) {
-		ap->blkno = NULLFSBLOCK;
-		ap->length = 0;
+		args->fsbno = NULLFSBLOCK;
 		return 0;
 	}
 
-	orig_offset = ap->offset;
-	orig_length = ap->length;
-
-	args.alloc_minlen_only = 1;
-
-	xfs_bmap_compute_alignments(ap, &args);
+	args->alloc_minlen_only = 1;
+	args->minlen = args->maxlen = ap->minlen;
+	args->total = ap->total;
 
 	/*
 	 * Unlike the longest extent available in an AG, we don't track
@@ -3502,33 +3491,9 @@  xfs_bmap_exact_minlen_extent_alloc(
 	 * we need not be concerned about a drop in performance in
 	 * "debug only" code paths.
 	 */
-	ap->blkno = XFS_AGB_TO_FSB(mp, 0, 0);
+	ap->blkno = XFS_AGB_TO_FSB(ap->ip->i_mount, 0, 0);
 
-	args.oinfo = XFS_RMAP_OINFO_SKIP_UPDATE;
-	args.minlen = args.maxlen = ap->minlen;
-	args.total = ap->total;
-
-	args.alignment = 1;
-	args.minalignslop = 0;
-
-	args.minleft = ap->minleft;
-	args.wasdel = ap->wasdel;
-	args.resv = XFS_AG_RESV_NONE;
-	args.datatype = ap->datatype;
-
-	error = xfs_alloc_vextent_first_ag(&args, ap->blkno);
-	if (error)
-		return error;
-
-	if (args.fsbno != NULLFSBLOCK) {
-		xfs_bmap_process_allocated_extent(ap, &args, orig_offset,
-			orig_length);
-	} else {
-		ap->blkno = NULLFSBLOCK;
-		ap->length = 0;
-	}
-
-	return 0;
+	return xfs_alloc_vextent_first_ag(args, ap->blkno);
 }
 #else
 
@@ -3792,8 +3757,11 @@  xfs_bmap_btalloc(
 	/* Trim the allocation back to the maximum an AG can fit. */
 	args.maxlen = min(ap->length, mp->m_ag_max_usable);
 
-	if ((ap->datatype & XFS_ALLOC_USERDATA) &&
-	    xfs_inode_is_filestream(ap->ip))
+	if (unlikely(XFS_TEST_ERROR(false, mp,
+			XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT)))
+		error = xfs_bmap_exact_minlen_extent_alloc(ap, &args);
+	else if ((ap->datatype & XFS_ALLOC_USERDATA) &&
+			xfs_inode_is_filestream(ap->ip))
 		error = xfs_bmap_btalloc_filestreams(ap, &args, stripe_align);
 	else
 		error = xfs_bmap_btalloc_best_length(ap, &args, stripe_align);
@@ -4208,9 +4176,6 @@  xfs_bmapi_allocate(
 	if ((bma->datatype & XFS_ALLOC_USERDATA) &&
 	    XFS_IS_REALTIME_INODE(bma->ip))
 		error = xfs_bmap_rtalloc(bma);
-	else if (unlikely(XFS_TEST_ERROR(false, mp,
-			XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT)))
-		error = xfs_bmap_exact_minlen_extent_alloc(bma);
 	else
 		error = xfs_bmap_btalloc(bma);
 	if (error)