[PATCH v2] xfs: don't let racing writers consume the free zones reserved for GC

[PATCH v2] xfs: don't let racing writers consume the free zones reserved for GC

[Hans Holmberg]

xfs_try_open_zone() checks the free zone count to avoid grabbing zones
required for GC forward progress, but does so without decreasing the
counter, letting multiple user writers through to call xfs_open_zone()
and collectively gobble up all free zones, stalling gc and eventually
leading to user threads getting stuck waiting for free space.

Fold the check and the accounting into a single atomic claim in
xfs_open_zone() instead.  The free zone count is decremented before the
zone is looked up and given back if none could be grabbed, so the reserve
can no longer be raced away.  User allocations only claim a zone if that
leaves the reserve behind, while GC just needs one free zone and thus can
still use the zones that are kept for it. Also add a warning if we end
up not finding a free zone in the xarray, as this should never happen.

Fixes: 4e4d52075577 ("xfs: add the zoned space allocator")
Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
---

Changes since v1:
- Add a WARN_ON_ONCE when no free zone is found despite a successful claim

 fs/xfs/libxfs/xfs_zones.h |  6 ++++++
 fs/xfs/xfs_zone_alloc.c   | 36 ++++++++++++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_zones.h b/fs/xfs/libxfs/xfs_zones.h
index c16089c9a652..1391d99b31de 100644
--- a/fs/xfs/libxfs/xfs_zones.h
+++ b/fs/xfs/libxfs/xfs_zones.h
@@ -30,6 +30,12 @@ struct blk_zone;
 #define XFS_OPEN_GC_ZONES	1U
 #define XFS_MIN_OPEN_ZONES	(XFS_OPEN_GC_ZONES + 1U)
 
+/*
+ * Never open a zone for user data unless this many zones are free, so that GC
+ * can always open the target zone it needs to make forward progress.
+ */
+#define XFS_MIN_FREE_GC_ZONES	(XFS_GC_ZONES - XFS_OPEN_GC_ZONES)
+
 /*
  * For zoned devices that do not have a limit on the number of open zones, and
  * for regular devices using the zoned allocator, use the most common SMR disks
diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 28c1e48909fa..be434eeccab2 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -437,6 +437,26 @@ xfs_init_open_zone(
 	return oz;
 }
 
+/*
+ * Claim one of the free zones. User allocations may not dip into the pool
+ * reserved for guaranteeing GC forward progress.
+ */
+static bool
+xfs_claim_free_zone(
+	struct xfs_zone_info	*zi,
+	bool			is_gc)
+{
+	int			min_free = is_gc ? 1 : XFS_MIN_FREE_GC_ZONES;
+	int			free = atomic_read(&zi->zi_nr_free_zones);
+
+	do {
+		if (free < min_free)
+			return false;
+	} while (!atomic_try_cmpxchg(&zi->zi_nr_free_zones, &free, free - 1));
+
+	return true;
+}
+
 /*
  * Find a completely free zone, open it, and return a reference.
  */
@@ -450,6 +470,9 @@ xfs_open_zone(
 	XA_STATE		(xas, &mp->m_groups[XG_TYPE_RTG].xa, 0);
 	struct xfs_group	*xg;
 
+	if (!xfs_claim_free_zone(zi, is_gc))
+		return NULL;
+
 	/*
 	 * Pick the free zone with lowest index. Zones in the beginning of the
 	 * address space typically provides higher bandwidth than those at the
@@ -460,11 +483,19 @@ xfs_open_zone(
 		if (atomic_inc_not_zero(&xg->xg_active_ref))
 			goto found;
 	xas_unlock(&xas);
+
+	/*
+	 * We should not end up here as we claimed a free zone before looking
+	 * through the free zone list. The free zone counter and the number
+	 * of zones marked as free must be out of sync.
+	 */
+	WARN_ON_ONCE(1);
+
+	atomic_inc(&zi->zi_nr_free_zones);
 	return NULL;
 
 found:
 	xas_clear_mark(&xas, XFS_RTG_FREE);
-	atomic_dec(&zi->zi_nr_free_zones);
 	xas_unlock(&xas);
 
 	set_current_state(TASK_RUNNING);
@@ -483,9 +514,6 @@ xfs_try_open_zone(
 
 	if (zi->zi_nr_open_zones >= mp->m_max_open_zones - XFS_OPEN_GC_ZONES)
 		return NULL;
-	if (atomic_read(&zi->zi_nr_free_zones) <
-	    XFS_GC_ZONES - XFS_OPEN_GC_ZONES)
-		return NULL;
 
 	/*
 	 * Increment the open zone count to reserve our slot before dropping
-- 
2.43.0

Re: [PATCH v2] xfs: don't let racing writers consume the free zones reserved for GC

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2] xfs: don't let racing writers consume the free zones reserved for GC

[Carlos Maiolino]

On Mon, Oct 05, 2026 at 01:59:17PM +0200, Hans Holmberg wrote:
> xfs_try_open_zone() checks the free zone count to avoid grabbing zones
> required for GC forward progress, but does so without decreasing the
> counter, letting multiple user writers through to call xfs_open_zone()
> and collectively gobble up all free zones, stalling gc and eventually
> leading to user threads getting stuck waiting for free space.
> 
> Fold the check and the accounting into a single atomic claim in
> xfs_open_zone() instead.  The free zone count is decremented before the
> zone is looked up and given back if none could be grabbed, so the reserve
> can no longer be raced away.  User allocations only claim a zone if that
> leaves the reserve behind, while GC just needs one free zone and thus can
> still use the zones that are kept for it. Also add a warning if we end
> up not finding a free zone in the xarray, as this should never happen.
> 
> Fixes: 4e4d52075577 ("xfs: add the zoned space allocator")
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>

Looks fine to me:

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
> 
> Changes since v1:
> - Add a WARN_ON_ONCE when no free zone is found despite a successful claim
> 
>  fs/xfs/libxfs/xfs_zones.h |  6 ++++++
>  fs/xfs/xfs_zone_alloc.c   | 36 ++++++++++++++++++++++++++++++++----
>  2 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_zones.h b/fs/xfs/libxfs/xfs_zones.h
> index c16089c9a652..1391d99b31de 100644
> --- a/fs/xfs/libxfs/xfs_zones.h
> +++ b/fs/xfs/libxfs/xfs_zones.h
> @@ -30,6 +30,12 @@ struct blk_zone;
>  #define XFS_OPEN_GC_ZONES	1U
>  #define XFS_MIN_OPEN_ZONES	(XFS_OPEN_GC_ZONES + 1U)
>  
> +/*
> + * Never open a zone for user data unless this many zones are free, so that GC
> + * can always open the target zone it needs to make forward progress.
> + */
> +#define XFS_MIN_FREE_GC_ZONES	(XFS_GC_ZONES - XFS_OPEN_GC_ZONES)
> +
>  /*
>   * For zoned devices that do not have a limit on the number of open zones, and
>   * for regular devices using the zoned allocator, use the most common SMR disks
> diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
> index 28c1e48909fa..be434eeccab2 100644
> --- a/fs/xfs/xfs_zone_alloc.c
> +++ b/fs/xfs/xfs_zone_alloc.c
> @@ -437,6 +437,26 @@ xfs_init_open_zone(
>  	return oz;
>  }
>  
> +/*
> + * Claim one of the free zones. User allocations may not dip into the pool
> + * reserved for guaranteeing GC forward progress.
> + */
> +static bool
> +xfs_claim_free_zone(
> +	struct xfs_zone_info	*zi,
> +	bool			is_gc)
> +{
> +	int			min_free = is_gc ? 1 : XFS_MIN_FREE_GC_ZONES;
> +	int			free = atomic_read(&zi->zi_nr_free_zones);
> +
> +	do {
> +		if (free < min_free)
> +			return false;
> +	} while (!atomic_try_cmpxchg(&zi->zi_nr_free_zones, &free, free - 1));
> +
> +	return true;
> +}
> +
>  /*
>   * Find a completely free zone, open it, and return a reference.
>   */
> @@ -450,6 +470,9 @@ xfs_open_zone(
>  	XA_STATE		(xas, &mp->m_groups[XG_TYPE_RTG].xa, 0);
>  	struct xfs_group	*xg;
>  
> +	if (!xfs_claim_free_zone(zi, is_gc))
> +		return NULL;
> +
>  	/*
>  	 * Pick the free zone with lowest index. Zones in the beginning of the
>  	 * address space typically provides higher bandwidth than those at the
> @@ -460,11 +483,19 @@ xfs_open_zone(
>  		if (atomic_inc_not_zero(&xg->xg_active_ref))
>  			goto found;
>  	xas_unlock(&xas);
> +
> +	/*
> +	 * We should not end up here as we claimed a free zone before looking
> +	 * through the free zone list. The free zone counter and the number
> +	 * of zones marked as free must be out of sync.
> +	 */
> +	WARN_ON_ONCE(1);
> +
> +	atomic_inc(&zi->zi_nr_free_zones);
>  	return NULL;
>  
>  found:
>  	xas_clear_mark(&xas, XFS_RTG_FREE);
> -	atomic_dec(&zi->zi_nr_free_zones);
>  	xas_unlock(&xas);
>  
>  	set_current_state(TASK_RUNNING);
> @@ -483,9 +514,6 @@ xfs_try_open_zone(
>  
>  	if (zi->zi_nr_open_zones >= mp->m_max_open_zones - XFS_OPEN_GC_ZONES)
>  		return NULL;
> -	if (atomic_read(&zi->zi_nr_free_zones) <
> -	    XFS_GC_ZONES - XFS_OPEN_GC_ZONES)
> -		return NULL;
>  
>  	/*
>  	 * Increment the open zone count to reserve our slot before dropping
> -- 
> 2.43.0
> 
> 

Re: [PATCH v2] xfs: don't let racing writers consume the free zones reserved for GC

[Carlos Maiolino]

On Mon, 05 Oct 2026 13:59:17 +0200, Hans Holmberg wrote:
> xfs_try_open_zone() checks the free zone count to avoid grabbing zones
> required for GC forward progress, but does so without decreasing the
> counter, letting multiple user writers through to call xfs_open_zone()
> and collectively gobble up all free zones, stalling gc and eventually
> leading to user threads getting stuck waiting for free space.
> 
> Fold the check and the accounting into a single atomic claim in
> xfs_open_zone() instead.  The free zone count is decremented before the
> zone is looked up and given back if none could be grabbed, so the reserve
> can no longer be raced away.  User allocations only claim a zone if that
> leaves the reserve behind, while GC just needs one free zone and thus can
> still use the zones that are kept for it. Also add a warning if we end
> up not finding a free zone in the xarray, as this should never happen.
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: don't let racing writers consume the free zones reserved for GC
      commit: 197f678cccbaaf8a5f52a91e6fc914d4c57be7f7

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>