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

[PATCH] 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 collectivly 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.

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

Darrick: Maybe this is the bug you saw [1] when writers got stuck
waitig for gc? (generic/476)

[1] https://lore.kernel.org/linux-xfs/20260925193722.GP2705364@frogsfrogsfrogs/

I have only been able to reproduce this issue in combination with other
patches, but the issue has been there since the start.

 fs/xfs/libxfs/xfs_zones.h |  6 ++++++
 fs/xfs/xfs_zone_alloc.c   | 29 +++++++++++++++++++++++++----
 2 files changed, 31 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..45bc77b1677b 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,12 @@ xfs_open_zone(
 		if (atomic_inc_not_zero(&xg->xg_active_ref))
 			goto found;
 	xas_unlock(&xas);
+
+	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 +507,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] xfs: don't let racing writers consume the free zones reserved for GC

[Christoph Hellwig]

On Sun, Sep 27, 2026 at 02:48:41PM +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 collectivly 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.

Ouch.   Yes, this looks good.  One nit:

> @@ -460,11 +483,12 @@ xfs_open_zone(
>  		if (atomic_inc_not_zero(&xg->xg_active_ref))
>  			goto found;
>  	xas_unlock(&xas);
> +
> +	atomic_inc(&zi->zi_nr_free_zones);
>  	return NULL;

This NULL return case now is a bug as the earlier atomic decrement
of zi_nr_free_zones should protect against it ever happening.

So this should have a comment and some kind of warning (WARN_ON maybe,
the usual XFS_IS_CORRUPT doesn't really fit for a pure in-core issue).