zoned GC fix

zoned GC fix

[Christoph Hellwig]

Hi Carlos,

this series fixes a potential deadlock in the zoned GC code, and
also adds a comment in the same area while looking at it.

Diffstat:
 xfs_zone_gc.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

[PATCH 1/2] xfs: add a separate bio_set for spliting GC writes

[Christoph Hellwig]

Allocating the new bio for a split from the same pool as the original
one can deadlock under memory pressure.  Add a separate pool for
splitting GC write bios to avoid this.

Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_zone_gc.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index f76a09130852..e4f70e024632 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -130,6 +130,9 @@ struct xfs_zone_gc_data {
 	/* bioset used to allocate the gc_bios */
 	struct bio_set			bio_set;
 
+	/* bioset used when writes need to be split to hardware limits */
+	struct bio_set			split_bio_set;
+
 	/*
 	 * Scratchpad to buffer GC data, organized as a ring buffer over
 	 * discontiguous folios.  scratch_head is where the buffer is filled,
@@ -221,6 +224,9 @@ xfs_zone_gc_data_alloc(
 	if (bioset_init(&data->bio_set, 16, offsetof(struct xfs_gc_bio, bio),
 			BIOSET_NEED_BVECS))
 		goto out_free_recs;
+	if (bioset_init(&data->split_bio_set, 16,
+			offsetof(struct xfs_gc_bio, bio), 0))
+		goto out_exit_bio_set;
 	for (i = 0; i < XFS_GC_NR_BUFS; i++) {
 		data->scratch_folios[i] =
 			folio_alloc(GFP_KERNEL, get_order(XFS_GC_BUF_SIZE));
@@ -238,6 +244,8 @@ xfs_zone_gc_data_alloc(
 out_free_scratch:
 	while (--i >= 0)
 		folio_put(data->scratch_folios[i]);
+	bioset_exit(&data->split_bio_set);
+out_exit_bio_set:
 	bioset_exit(&data->bio_set);
 out_free_recs:
 	kfree(data->iter.recs);
@@ -254,6 +262,7 @@ xfs_zone_gc_data_free(
 
 	for (i = 0; i < XFS_GC_NR_BUFS; i++)
 		folio_put(data->scratch_folios[i]);
+	bioset_exit(&data->split_bio_set);
 	bioset_exit(&data->bio_set);
 	kfree(data->iter.recs);
 	kfree(data);
@@ -810,7 +819,8 @@ xfs_zone_gc_split_write(
 			data->mp->m_sb.sb_blocksize) >> SECTOR_SHIFT;
 	split_len = split_sectors << SECTOR_SHIFT;
 
-	split = bio_split(&chunk->bio, split_sectors, GFP_NOFS, &data->bio_set);
+	split = bio_split(&chunk->bio, split_sectors, GFP_NOFS,
+			&data->split_bio_set);
 	split_chunk = container_of(split, struct xfs_gc_bio, bio);
 	split_chunk->data = data;
 	ihold(VFS_I(chunk->ip));
-- 
2.53.0

[PATCH 2/2] xfs: add a comment to describe xfs_gc_bio.victim_rtg

[Christoph Hellwig]

All other fields have comments describing them, add one for this field
as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_zone_gc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index e4f70e024632..e2bad83b10f6 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -103,6 +103,7 @@ struct xfs_gc_bio {
 	/* Open Zone being written to */
 	struct xfs_open_zone		*oz;
 
+	/* Realtime group currently evacuated */
 	struct xfs_rtgroup		*victim_rtg;
 
 	/* Bio used for reads and writes, including the bvec used by it */
-- 
2.53.0

Re: [PATCH 1/2] xfs: add a separate bio_set for spliting GC writes

[Darrick J. Wong]

On Tue, Jul 28, 2026 at 10:06:44AM +0200, Christoph Hellwig wrote:
> Allocating the new bio for a split from the same pool as the original
> one can deadlock under memory pressure.  Add a separate pool for
> splitting GC write bios to avoid this.
> 
> Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Seems fine to me...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_zone_gc.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index f76a09130852..e4f70e024632 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -130,6 +130,9 @@ struct xfs_zone_gc_data {
>  	/* bioset used to allocate the gc_bios */
>  	struct bio_set			bio_set;
>  
> +	/* bioset used when writes need to be split to hardware limits */
> +	struct bio_set			split_bio_set;
> +
>  	/*
>  	 * Scratchpad to buffer GC data, organized as a ring buffer over
>  	 * discontiguous folios.  scratch_head is where the buffer is filled,
> @@ -221,6 +224,9 @@ xfs_zone_gc_data_alloc(
>  	if (bioset_init(&data->bio_set, 16, offsetof(struct xfs_gc_bio, bio),
>  			BIOSET_NEED_BVECS))
>  		goto out_free_recs;
> +	if (bioset_init(&data->split_bio_set, 16,
> +			offsetof(struct xfs_gc_bio, bio), 0))
> +		goto out_exit_bio_set;
>  	for (i = 0; i < XFS_GC_NR_BUFS; i++) {
>  		data->scratch_folios[i] =
>  			folio_alloc(GFP_KERNEL, get_order(XFS_GC_BUF_SIZE));
> @@ -238,6 +244,8 @@ xfs_zone_gc_data_alloc(
>  out_free_scratch:
>  	while (--i >= 0)
>  		folio_put(data->scratch_folios[i]);
> +	bioset_exit(&data->split_bio_set);
> +out_exit_bio_set:
>  	bioset_exit(&data->bio_set);
>  out_free_recs:
>  	kfree(data->iter.recs);
> @@ -254,6 +262,7 @@ xfs_zone_gc_data_free(
>  
>  	for (i = 0; i < XFS_GC_NR_BUFS; i++)
>  		folio_put(data->scratch_folios[i]);
> +	bioset_exit(&data->split_bio_set);
>  	bioset_exit(&data->bio_set);
>  	kfree(data->iter.recs);
>  	kfree(data);
> @@ -810,7 +819,8 @@ xfs_zone_gc_split_write(
>  			data->mp->m_sb.sb_blocksize) >> SECTOR_SHIFT;
>  	split_len = split_sectors << SECTOR_SHIFT;
>  
> -	split = bio_split(&chunk->bio, split_sectors, GFP_NOFS, &data->bio_set);
> +	split = bio_split(&chunk->bio, split_sectors, GFP_NOFS,
> +			&data->split_bio_set);
>  	split_chunk = container_of(split, struct xfs_gc_bio, bio);
>  	split_chunk->data = data;
>  	ihold(VFS_I(chunk->ip));
> -- 
> 2.53.0
> 
> 

Re: [PATCH 2/2] xfs: add a comment to describe xfs_gc_bio.victim_rtg

[Darrick J. Wong]

On Tue, Jul 28, 2026 at 10:06:45AM +0200, Christoph Hellwig wrote:
> All other fields have comments describing them, add one for this field
> as well.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

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

--D

> ---
>  fs/xfs/xfs_zone_gc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index e4f70e024632..e2bad83b10f6 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -103,6 +103,7 @@ struct xfs_gc_bio {
>  	/* Open Zone being written to */
>  	struct xfs_open_zone		*oz;
>  
> +	/* Realtime group currently evacuated */
>  	struct xfs_rtgroup		*victim_rtg;
>  
>  	/* Bio used for reads and writes, including the bvec used by it */
> -- 
> 2.53.0
> 
> 

Re: [PATCH 2/2] xfs: add a comment to describe xfs_gc_bio.victim_rtg

[Damien Le Moal]

On 7/28/26 17:06, Christoph Hellwig wrote:
> All other fields have comments describing them, add one for this field
> as well.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me (see nit below).

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

> ---
>  fs/xfs/xfs_zone_gc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index e4f70e024632..e2bad83b10f6 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -103,6 +103,7 @@ struct xfs_gc_bio {
>  	/* Open Zone being written to */
>  	struct xfs_open_zone		*oz;
>  
> +	/* Realtime group currently evacuated */

May be: /* Realtime group currently being reclaimed */ ?

>  	struct xfs_rtgroup		*victim_rtg;
>  
>  	/* Bio used for reads and writes, including the bvec used by it */


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/2] xfs: add a separate bio_set for spliting GC writes

[Damien Le Moal]

On 7/28/26 17:06, Christoph Hellwig wrote:
> Allocating the new bio for a split from the same pool as the original
> one can deadlock under memory pressure.  Add a separate pool for
> splitting GC write bios to avoid this.
> 
> Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me, but the commit message would be better with an explanation of
how the deadlock can happen, as I do not think it is that obvious.

Regardless,

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


-- 
Damien Le Moal
Western Digital Research