[PATCH] xfs: fix number of GC bvecs

[PATCH] xfs: fix number of GC bvecs

[Christoph Hellwig]

GC scratch allocations can wrap around and use the same buffer twice, and
the current code fails to account for that.  So far this worked due to
rounding in the block layer, but changes to the bio allocator drop the
over-provisioning and generic/256 or generic/361 will not usually fail
when running against the current block tree.

Simply the allocation to always pass the maximum value that is easier to
verify, as a saving of up to one bvec per allocation isn't worth the
effort to verify a complicated calculated value.

Fixes: 102f444b57b3 ("xfs: rework zone GC buffer management")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_zone_gc.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 5fe84e82ff1e..b2626a482563 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -670,7 +670,6 @@ xfs_zone_gc_start_chunk(
 	struct xfs_inode	*ip;
 	struct bio		*bio;
 	xfs_daddr_t		daddr;
-	unsigned int		len;
 	bool			is_seq;
 
 	if (xfs_is_shutdown(mp))
@@ -685,15 +684,16 @@ xfs_zone_gc_start_chunk(
 		return false;
 	}
 
-	len = XFS_FSB_TO_B(mp, irec.rm_blockcount);
-	bio = bio_alloc_bioset(bdev,
-			min(howmany(len, XFS_GC_BUF_SIZE) + 1, XFS_GC_NR_BUFS),
-			REQ_OP_READ, GFP_NOFS, &data->bio_set);
-
+	/*
+	 * Scratch allocation can wrap around to the same buffer again,
+	 * provision an extra bvec for that case.
+	 */
+	bio = bio_alloc_bioset(bdev, XFS_GC_NR_BUFS + 1, REQ_OP_READ, GFP_NOFS,
+			&data->bio_set);
 	chunk = container_of(bio, struct xfs_gc_bio, bio);
 	chunk->ip = ip;
 	chunk->offset = XFS_FSB_TO_B(mp, irec.rm_offset);
-	chunk->len = len;
+	chunk->len = XFS_FSB_TO_B(mp, irec.rm_blockcount);
 	chunk->old_startblock =
 		xfs_rgbno_to_rtb(iter->victim_rtg, irec.rm_startblock);
 	chunk->new_daddr = daddr;
@@ -707,8 +707,9 @@ xfs_zone_gc_start_chunk(
 	bio->bi_iter.bi_sector = xfs_rtb_to_daddr(mp, chunk->old_startblock);
 	bio->bi_end_io = xfs_zone_gc_end_io;
 	xfs_zone_gc_add_data(chunk);
-	data->scratch_head = (data->scratch_head + len) % data->scratch_size;
-	data->scratch_available -= len;
+	data->scratch_head =
+		(data->scratch_head + chunk->len) % data->scratch_size;
+	data->scratch_available -= chunk->len;
 
 	XFS_STATS_INC(mp, xs_gc_read_calls);
 
-- 
2.47.3

Re: [PATCH] xfs: fix number of GC bvecs

[Damien Le Moal]

On 4/1/26 22:58, Christoph Hellwig wrote:
> GC scratch allocations can wrap around and use the same buffer twice, and
> the current code fails to account for that.  So far this worked due to
> rounding in the block layer, but changes to the bio allocator drop the
> over-provisioning and generic/256 or generic/361 will not usually fail
> when running against the current block tree.
> 
> Simply the allocation to always pass the maximum value that is easier to

s/Simply/Simplify

> verify, as a saving of up to one bvec per allocation isn't worth the
> effort to verify a complicated calculated value.
> 
> Fixes: 102f444b57b3 ("xfs: rework zone GC buffer management")
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] xfs: fix number of GC bvecs

[Hans Holmberg]

On 01/04/2026 15:58, Christoph Hellwig wrote:
> GC scratch allocations can wrap around and use the same buffer twice, and
> the current code fails to account for that.  So far this worked due to
> rounding in the block layer, but changes to the bio allocator drop the
> over-provisioning and generic/256 or generic/361 will not usually fail
> when running against the current block tree.

I have a hard time understanding the "will not usually fail" part,
but apart from that, the patch looks good:

Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>


> 
> Simply the allocation to always pass the maximum value that is easier to
> verify, as a saving of up to one bvec per allocation isn't worth the
> effort to verify a complicated calculated value.
> 
> Fixes: 102f444b57b3 ("xfs: rework zone GC buffer management")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_zone_gc.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 5fe84e82ff1e..b2626a482563 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -670,7 +670,6 @@ xfs_zone_gc_start_chunk(
>  	struct xfs_inode	*ip;
>  	struct bio		*bio;
>  	xfs_daddr_t		daddr;
> -	unsigned int		len;
>  	bool			is_seq;
>  
>  	if (xfs_is_shutdown(mp))
> @@ -685,15 +684,16 @@ xfs_zone_gc_start_chunk(
>  		return false;
>  	}
>  
> -	len = XFS_FSB_TO_B(mp, irec.rm_blockcount);
> -	bio = bio_alloc_bioset(bdev,
> -			min(howmany(len, XFS_GC_BUF_SIZE) + 1, XFS_GC_NR_BUFS),
> -			REQ_OP_READ, GFP_NOFS, &data->bio_set);
> -
> +	/*
> +	 * Scratch allocation can wrap around to the same buffer again,
> +	 * provision an extra bvec for that case.
> +	 */
> +	bio = bio_alloc_bioset(bdev, XFS_GC_NR_BUFS + 1, REQ_OP_READ, GFP_NOFS,
> +			&data->bio_set);
>  	chunk = container_of(bio, struct xfs_gc_bio, bio);
>  	chunk->ip = ip;
>  	chunk->offset = XFS_FSB_TO_B(mp, irec.rm_offset);
> -	chunk->len = len;
> +	chunk->len = XFS_FSB_TO_B(mp, irec.rm_blockcount);
>  	chunk->old_startblock =
>  		xfs_rgbno_to_rtb(iter->victim_rtg, irec.rm_startblock);
>  	chunk->new_daddr = daddr;
> @@ -707,8 +707,9 @@ xfs_zone_gc_start_chunk(
>  	bio->bi_iter.bi_sector = xfs_rtb_to_daddr(mp, chunk->old_startblock);
>  	bio->bi_end_io = xfs_zone_gc_end_io;
>  	xfs_zone_gc_add_data(chunk);
> -	data->scratch_head = (data->scratch_head + len) % data->scratch_size;
> -	data->scratch_available -= len;
> +	data->scratch_head =
> +		(data->scratch_head + chunk->len) % data->scratch_size;
> +	data->scratch_available -= chunk->len;
>  
>  	XFS_STATS_INC(mp, xs_gc_read_calls);
>  

Re: [PATCH] xfs: fix number of GC bvecs

[Niklas Cassel]

Hello Christoph,

On Wed, Apr 01, 2026 at 03:58:01PM +0200, Christoph Hellwig wrote:
> GC scratch allocations can wrap around and use the same buffer twice, and
> the current code fails to account for that.  So far this worked due to
> rounding in the block layer, but changes to the bio allocator drop the

Which change in the bio allocator?

If it is a commit that has landed already, I think it would be nice to
mention the commit in the usual "commit + 12 character SHA1" format.

If it is a change in a series that has not landed yet, it would be nice
if you could provide a link to the series, e.g. after the commit log
(after the "---").


Kind regards,
Niklas

Re: [PATCH] xfs: fix number of GC bvecs

[Christoph Hellwig]

On Thu, Apr 02, 2026 at 07:25:44AM +0000, Hans Holmberg wrote:
> On 01/04/2026 15:58, Christoph Hellwig wrote:
> > GC scratch allocations can wrap around and use the same buffer twice, and
> > the current code fails to account for that.  So far this worked due to
> > rounding in the block layer, but changes to the bio allocator drop the
> > over-provisioning and generic/256 or generic/361 will not usually fail
> > when running against the current block tree.
> 
> I have a hard time understanding the "will not usually fail" part,
> but apart from that, the patch looks good:

The not should be a now.

Re: [PATCH] xfs: fix number of GC bvecs

[Christoph Hellwig]

On Thu, Apr 02, 2026 at 08:49:58PM +0200, Niklas Cassel wrote:
> Hello Christoph,
> 
> On Wed, Apr 01, 2026 at 03:58:01PM +0200, Christoph Hellwig wrote:
> > GC scratch allocations can wrap around and use the same buffer twice, and
> > the current code fails to account for that.  So far this worked due to
> > rounding in the block layer, but changes to the bio allocator drop the
> 
> Which change in the bio allocator?
> 
> If it is a commit that has landed already, I think it would be nice to
> mention the commit in the usual "commit + 12 character SHA1" format.

We can't reference the other trees as we can't guarantee a stable commit
ID before it hits mainline.