cleanup open GC zone handling v2

cleanup open GC zone handling v2

[Christoph Hellwig]

Hi all,

this series cleans up the handling of open GC zones so that the
reference counting works more similar to normal open GC zones.
This cleans up the code in general, and also helps with experiments
to support multiple open zones for GC I've started.

Changes since v1:
 - improve various commit messages
 - move over two patches to heree from a different series
 - use READ_ONCE in nr_open_zones_show
 - fix the nr_open_zones_show documentation

Diffstat:
 Documentation/admin-guide/xfs.rst |    4 
 fs/xfs/xfs_sysfs.c                |   13 ++
 fs/xfs/xfs_trace.h                |    1 
 fs/xfs/xfs_zone_alloc.c           |   21 ++-
 fs/xfs/xfs_zone_gc.c              |  230 +++++++++++++++++---------------------
 fs/xfs/xfs_zone_info.c            |   20 +--
 fs/xfs/xfs_zone_priv.h            |   15 --
 7 files changed, 152 insertions(+), 152 deletions(-)

[PATCH 1/9] xfs: delay initial open of the GC zone

[Christoph Hellwig]

The code currently used to select the new GC target zone when the
previous one is full also handles the case where there is no current GC
target zone at all.  Make use of that to simplify the logic in
xfs_zone_gc_mount.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_zone_gc.c | 45 ++++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index a8f71231f351..00a996c13d8a 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -527,7 +527,7 @@ xfs_zone_gc_select_victim(
 	return true;
 }
 
-static struct xfs_open_zone *
+static int
 xfs_zone_gc_steal_open(
 	struct xfs_zone_info	*zi)
 {
@@ -538,15 +538,18 @@ xfs_zone_gc_steal_open(
 		if (!found || oz->oz_allocated < found->oz_allocated)
 			found = oz;
 	}
-
-	if (found) {
-		found->oz_is_gc = true;
-		list_del_init(&found->oz_entry);
-		zi->zi_nr_open_zones--;
+	if (!found) {
+		spin_unlock(&zi->zi_open_zones_lock);
+		return -EIO;
 	}
 
+	trace_xfs_zone_gc_target_opened(found->oz_rtg);
+	found->oz_is_gc = true;
+	list_del_init(&found->oz_entry);
+	zi->zi_nr_open_zones--;
+	zi->zi_open_gc_zone = found;
 	spin_unlock(&zi->zi_open_zones_lock);
-	return found;
+	return 0;
 }
 
 static struct xfs_open_zone *
@@ -1194,31 +1197,24 @@ xfs_zone_gc_mount(
 {
 	struct xfs_zone_info	*zi = mp->m_zone_info;
 	struct xfs_zone_gc_data	*data;
-	struct xfs_open_zone	*oz;
 	int			error;
 
 	/*
-	 * If there are no free zones available for GC, pick the open zone with
+	 * If there are no free zones available for GC, or the number of open
+	 * zones has reached the open zone limit, pick the open zone with
 	 * the least used space to GC into.  This should only happen after an
-	 * unclean shutdown near ENOSPC while GC was ongoing.
-	 *
-	 * We also need to do this for the first gc zone allocation if we
-	 * unmounted while at the open limit.
+	 * unclean shutdown while GC was ongoing.  Otherwise a GC zone will
+	 * be selected from the free zone pool on demand.
 	 */
 	if (!xfs_group_marked(mp, XG_TYPE_RTG, XFS_RTG_FREE) ||
-	    zi->zi_nr_open_zones == mp->m_max_open_zones)
-		oz = xfs_zone_gc_steal_open(zi);
-	else
-		oz = xfs_open_zone(mp, WRITE_LIFE_NOT_SET, true);
-	if (!oz) {
-		xfs_warn(mp, "unable to allocate a zone for gc");
-		error = -EIO;
-		goto out;
+	    zi->zi_nr_open_zones >= mp->m_max_open_zones) {
+		error = xfs_zone_gc_steal_open(zi);
+		if (error) {
+			xfs_warn(mp, "unable to steal an open zone for gc");
+			return error;
+		}
 	}
 
-	trace_xfs_zone_gc_target_opened(oz->oz_rtg);
-	zi->zi_open_gc_zone = oz;
-
 	data = xfs_zone_gc_data_alloc(mp);
 	if (!data) {
 		error = -ENOMEM;
@@ -1241,7 +1237,6 @@ xfs_zone_gc_mount(
 	kfree(data);
 out_put_gc_zone:
 	xfs_open_zone_put(zi->zi_open_gc_zone);
-out:
 	return error;
 }
 
-- 
2.47.3

[PATCH 2/9] xfs: add a separate tracepoint for stealing an open zone for GC

[Christoph Hellwig]

The case where we have to reuse an already open zone warrants a different
trace point vs the normal opening of a GC zone.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_trace.h   | 1 +
 fs/xfs/xfs_zone_gc.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index c5ad26a1d7bb..1c098cfc5c00 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -394,6 +394,7 @@ DEFINE_ZONE_EVENT(xfs_zone_full);
 DEFINE_ZONE_EVENT(xfs_zone_opened);
 DEFINE_ZONE_EVENT(xfs_zone_reset);
 DEFINE_ZONE_EVENT(xfs_zone_gc_target_opened);
+DEFINE_ZONE_EVENT(xfs_zone_gc_target_stolen);
 
 TRACE_EVENT(xfs_zone_free_blocks,
 	TP_PROTO(struct xfs_rtgroup *rtg, xfs_rgblock_t rgbno,
diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 00a996c13d8a..0c1e22409ef1 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -543,7 +543,7 @@ xfs_zone_gc_steal_open(
 		return -EIO;
 	}
 
-	trace_xfs_zone_gc_target_opened(found->oz_rtg);
+	trace_xfs_zone_gc_target_stolen(found->oz_rtg);
 	found->oz_is_gc = true;
 	list_del_init(&found->oz_entry);
 	zi->zi_nr_open_zones--;
-- 
2.47.3

[PATCH 3/9] xfs: put the open zone later xfs_open_zone_put

[Christoph Hellwig]

The open zone is what holds the rtg reference for us.  This doesn't
matter until we support shrinking, and even then is rather theoretical
because we can't shrink away a just filled zone in a tiny race window,
but let's play safe here.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_zone_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 5f8b6cbeebfd..17a3762aa951 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -182,11 +182,11 @@ xfs_open_zone_mark_full(
 		list_del_init(&oz->oz_entry);
 	}
 	spin_unlock(&zi->zi_open_zones_lock);
-	xfs_open_zone_put(oz);
 
 	wake_up_all(&zi->zi_zone_wait);
 	if (used < rtg_blocks(rtg))
 		xfs_zone_account_reclaimable(rtg, rtg_blocks(rtg) - used);
+	xfs_open_zone_put(oz);
 }
 
 static inline void
-- 
2.47.3

[PATCH 4/9] xfs: rename xfs_zone_gc_iter_next to xfs_zone_gc_iter_irec

[Christoph Hellwig]

This function returns the current iterator position, which makes the
_next postfix a bit misleading.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_zone_gc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 0c1e22409ef1..cd4f52af295e 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -374,7 +374,7 @@ xfs_zone_gc_query(
 }
 
 static bool
-xfs_zone_gc_iter_next(
+xfs_zone_gc_iter_irec(
 	struct xfs_mount	*mp,
 	struct xfs_zone_gc_iter	*iter,
 	struct xfs_rmap_irec	*chunk_rec,
@@ -691,7 +691,7 @@ xfs_zone_gc_start_chunk(
 	if (xfs_is_shutdown(mp))
 		return false;
 
-	if (!xfs_zone_gc_iter_next(mp, iter, &irec, &ip))
+	if (!xfs_zone_gc_iter_irec(mp, iter, &irec, &ip))
 		return false;
 	oz = xfs_zone_gc_alloc_blocks(data, &irec.rm_blockcount, &daddr,
 			&is_seq);
-- 
2.47.3

[PATCH 5/9] xfs: refactor GC zone selection helpers

[Christoph Hellwig]

Merge xfs_zone_gc_ensure_target into xfs_zone_gc_select_target
to keep all zone selection code together.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_zone_gc.c | 45 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index cd4f52af295e..0f62051b0acc 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -552,6 +552,9 @@ xfs_zone_gc_steal_open(
 	return 0;
 }
 
+/*
+ * Ensure we have a valid open zone to write to.
+ */
 static struct xfs_open_zone *
 xfs_zone_gc_select_target(
 	struct xfs_mount	*mp)
@@ -559,12 +562,25 @@ xfs_zone_gc_select_target(
 	struct xfs_zone_info	*zi = mp->m_zone_info;
 	struct xfs_open_zone	*oz = zi->zi_open_gc_zone;
 
+	if (oz) {
+		/*
+		 * If we have space available, just keep using the existing
+		 * zone.
+		 */
+		if (oz->oz_allocated < rtg_blocks(oz->oz_rtg))
+			return oz;
+
+		/*
+		 * Wait for all writes to the current zone to finish before
+		 * picking a new one.
+		 */
+		if (oz->oz_written < rtg_blocks(oz->oz_rtg))
+			return NULL;
+	}
+
 	/*
-	 * We need to wait for pending writes to finish.
+	 * Open a new zone when there is none currently in use.
 	 */
-	if (oz && oz->oz_written < rtg_blocks(oz->oz_rtg))
-		return NULL;
-
 	ASSERT(zi->zi_nr_open_zones <=
 		mp->m_max_open_zones - XFS_OPEN_GC_ZONES);
 	oz = xfs_open_zone(mp, WRITE_LIFE_NOT_SET, true);
@@ -576,23 +592,6 @@ xfs_zone_gc_select_target(
 	return oz;
 }
 
-/*
- * Ensure we have a valid open zone to write the GC data to.
- *
- * If the current target zone has space keep writing to it, else first wait for
- * all pending writes and then pick a new one.
- */
-static struct xfs_open_zone *
-xfs_zone_gc_ensure_target(
-	struct xfs_mount	*mp)
-{
-	struct xfs_open_zone	*oz = mp->m_zone_info->zi_open_gc_zone;
-
-	if (!oz || oz->oz_allocated == rtg_blocks(oz->oz_rtg))
-		return xfs_zone_gc_select_target(mp);
-	return oz;
-}
-
 static void
 xfs_zone_gc_end_io(
 	struct bio		*bio)
@@ -615,7 +614,7 @@ xfs_zone_gc_alloc_blocks(
 	struct xfs_mount	*mp = data->mp;
 	struct xfs_open_zone	*oz;
 
-	oz = xfs_zone_gc_ensure_target(mp);
+	oz = xfs_zone_gc_select_target(mp);
 	if (!oz)
 		return NULL;
 
@@ -1019,7 +1018,7 @@ xfs_zone_gc_should_start_new_work(
 	if (!data->scratch_available)
 		return false;
 
-	oz = xfs_zone_gc_ensure_target(data->mp);
+	oz = xfs_zone_gc_select_target(data->mp);
 	if (!oz || oz->oz_allocated == rtg_blocks(oz->oz_rtg))
 		return false;
 
-- 
2.47.3

[PATCH 6/9] xfs: streamline GC zone selection

[Christoph Hellwig]

Currently picking of the GC target zone is a bit odd as it is done both
in the main "can we start new GC cycles" routine and in the low-level
block allocator for GC.  This was mostly done to work around the rules
for when code in a waitqueue wait loop can sleep.

But with a trick to check if the process state has been set to running to
discover if the wait loop has to be retried, all this becomes much
simpler.  We can select a GC zone just before writing, and bail out of
starting new work if we can't find a usable zone.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_zone_gc.c | 95 +++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 55 deletions(-)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 0f62051b0acc..2c2fa924fecd 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -383,9 +383,6 @@ xfs_zone_gc_iter_irec(
 	struct xfs_rmap_irec	*irec;
 	int			error;
 
-	if (!iter->victim_rtg)
-		return false;
-
 retry:
 	if (iter->rec_idx == iter->rec_count) {
 		error = xfs_zone_gc_query(mp, iter);
@@ -555,7 +552,7 @@ xfs_zone_gc_steal_open(
 /*
  * Ensure we have a valid open zone to write to.
  */
-static struct xfs_open_zone *
+static bool
 xfs_zone_gc_select_target(
 	struct xfs_mount	*mp)
 {
@@ -568,14 +565,14 @@ xfs_zone_gc_select_target(
 		 * zone.
 		 */
 		if (oz->oz_allocated < rtg_blocks(oz->oz_rtg))
-			return oz;
+			return true;
 
 		/*
 		 * Wait for all writes to the current zone to finish before
 		 * picking a new one.
 		 */
 		if (oz->oz_written < rtg_blocks(oz->oz_rtg))
-			return NULL;
+			return false;
 	}
 
 	/*
@@ -589,7 +586,7 @@ xfs_zone_gc_select_target(
 	spin_lock(&zi->zi_open_zones_lock);
 	zi->zi_open_gc_zone = oz;
 	spin_unlock(&zi->zi_open_zones_lock);
-	return oz;
+	return !!oz;
 }
 
 static void
@@ -604,7 +601,7 @@ xfs_zone_gc_end_io(
 	wake_up_process(data->mp->m_zone_info->zi_gc_thread);
 }
 
-static struct xfs_open_zone *
+static bool
 xfs_zone_gc_alloc_blocks(
 	struct xfs_zone_gc_data	*data,
 	xfs_extlen_t		*count_fsb,
@@ -612,11 +609,7 @@ xfs_zone_gc_alloc_blocks(
 	bool			*is_seq)
 {
 	struct xfs_mount	*mp = data->mp;
-	struct xfs_open_zone	*oz;
-
-	oz = xfs_zone_gc_select_target(mp);
-	if (!oz)
-		return NULL;
+	struct xfs_open_zone	*oz = mp->m_zone_info->zi_open_gc_zone;
 
 	*count_fsb = min(*count_fsb, XFS_B_TO_FSB(mp, data->scratch_available));
 
@@ -638,7 +631,7 @@ xfs_zone_gc_alloc_blocks(
 	spin_unlock(&mp->m_sb_lock);
 
 	if (!*count_fsb)
-		return NULL;
+		return false;
 
 	*daddr = xfs_gbno_to_daddr(rtg_group(oz->oz_rtg), 0);
 	*is_seq = bdev_zone_is_seq(mp->m_rtdev_targp->bt_bdev, *daddr);
@@ -646,7 +639,7 @@ xfs_zone_gc_alloc_blocks(
 		*daddr += XFS_FSB_TO_BB(mp, oz->oz_allocated);
 	oz->oz_allocated += *count_fsb;
 	atomic_inc(&oz->oz_ref);
-	return oz;
+	return true;
 }
 
 static void
@@ -671,6 +664,28 @@ xfs_zone_gc_add_data(
 	} while (len);
 }
 
+static bool
+xfs_zone_gc_can_start_chunk(
+	struct xfs_zone_gc_data	*data)
+{
+
+	if (xfs_is_shutdown(data->mp))
+		return false;
+	if (!data->scratch_available)
+		return false;
+
+	if (!data->iter.victim_rtg) {
+		if (kthread_should_stop() || kthread_should_park())
+			return false;
+		if (!xfs_zoned_need_gc(data->mp))
+			return false;
+		if (!xfs_zone_gc_select_victim(data))
+			return false;
+	}
+
+	return xfs_zone_gc_select_target(data->mp);
+}
+
 static bool
 xfs_zone_gc_start_chunk(
 	struct xfs_zone_gc_data	*data)
@@ -678,7 +693,6 @@ xfs_zone_gc_start_chunk(
 	struct xfs_zone_gc_iter	*iter = &data->iter;
 	struct xfs_mount	*mp = data->mp;
 	struct block_device	*bdev = mp->m_rtdev_targp->bt_bdev;
-	struct xfs_open_zone	*oz;
 	struct xfs_rmap_irec	irec;
 	struct xfs_gc_bio	*chunk;
 	struct xfs_inode	*ip;
@@ -687,14 +701,15 @@ xfs_zone_gc_start_chunk(
 	unsigned int		len;
 	bool			is_seq;
 
-	if (xfs_is_shutdown(mp))
+	if (!xfs_zone_gc_can_start_chunk(data))
 		return false;
 
+	set_current_state(TASK_RUNNING);
 	if (!xfs_zone_gc_iter_irec(mp, iter, &irec, &ip))
 		return false;
-	oz = xfs_zone_gc_alloc_blocks(data, &irec.rm_blockcount, &daddr,
-			&is_seq);
-	if (!oz) {
+
+	if (!xfs_zone_gc_alloc_blocks(data, &irec.rm_blockcount, &daddr,
+			&is_seq)) {
 		xfs_irele(ip);
 		return false;
 	}
@@ -713,7 +728,7 @@ xfs_zone_gc_start_chunk(
 	chunk->new_daddr = daddr;
 	chunk->is_seq = is_seq;
 	chunk->data = data;
-	chunk->oz = oz;
+	chunk->oz = mp->m_zone_info->zi_open_gc_zone;
 	chunk->victim_rtg = iter->victim_rtg;
 	atomic_inc(&rtg_group(chunk->victim_rtg)->xg_active_ref);
 	atomic_inc(&chunk->victim_rtg->rtg_gccount);
@@ -1007,33 +1022,6 @@ xfs_zone_gc_reset_zones(
 	} while (next);
 }
 
-static bool
-xfs_zone_gc_should_start_new_work(
-	struct xfs_zone_gc_data	*data)
-{
-	struct xfs_open_zone	*oz;
-
-	if (xfs_is_shutdown(data->mp))
-		return false;
-	if (!data->scratch_available)
-		return false;
-
-	oz = xfs_zone_gc_select_target(data->mp);
-	if (!oz || oz->oz_allocated == rtg_blocks(oz->oz_rtg))
-		return false;
-
-	if (!data->iter.victim_rtg) {
-		if (kthread_should_stop() || kthread_should_park())
-			return false;
-		if (!xfs_zoned_need_gc(data->mp))
-			return false;
-		if (!xfs_zone_gc_select_victim(data))
-			return false;
-	}
-
-	return true;
-}
-
 /*
  * Handle the work to read and write data for GC and to reset the zones,
  * including handling all completions.
@@ -1083,13 +1071,10 @@ xfs_zone_gc_handle_work(
 	}
 	blk_finish_plug(&plug);
 
-	if (xfs_zone_gc_should_start_new_work(data)) {
-		set_current_state(TASK_RUNNING);
-		blk_start_plug(&plug);
-		while (xfs_zone_gc_start_chunk(data))
-			;
-		blk_finish_plug(&plug);
-	}
+	blk_start_plug(&plug);
+	while (xfs_zone_gc_start_chunk(data))
+		;
+	blk_finish_plug(&plug);
 }
 
 /*
-- 
2.47.3

[PATCH 7/9] xfs: reduce special casing for the open GC zone

[Christoph Hellwig]

Currently the open zone used for garbage collection is a special snow
flake, and it has been a bit annoying for some further zoned XFS work
I've been doing.

Remove the zi_open_gc_field and instead track the open GC zone in the
zi_open_zones list together with the normal open zones, and keep an extra
pointer and a reference of in the GC thread's data structure.  This means
anything iterating over open zones just has to look at zi_open_zones, and
the life time rules are consistent.  It also helps to add support for
multiple open GC zones if we ever need them, and removes a bit of code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
---
 fs/xfs/xfs_zone_alloc.c | 19 +++++++----
 fs/xfs/xfs_zone_gc.c    | 71 ++++++++++++++++++++++-------------------
 fs/xfs/xfs_zone_info.c  | 12 +++----
 fs/xfs/xfs_zone_priv.h  | 15 ++-------
 4 files changed, 58 insertions(+), 59 deletions(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 17a3762aa951..a851b98143c0 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -174,16 +174,18 @@ xfs_open_zone_mark_full(
 	WRITE_ONCE(rtg->rtg_open_zone, NULL);
 
 	spin_lock(&zi->zi_open_zones_lock);
-	if (oz->oz_is_gc) {
-		ASSERT(current == zi->zi_gc_thread);
-		zi->zi_open_gc_zone = NULL;
-	} else {
+	if (oz->oz_is_gc)
+		zi->zi_nr_open_gc_zones--;
+	else
 		zi->zi_nr_open_zones--;
-		list_del_init(&oz->oz_entry);
-	}
+	list_del_init(&oz->oz_entry);
 	spin_unlock(&zi->zi_open_zones_lock);
 
-	wake_up_all(&zi->zi_zone_wait);
+	if (oz->oz_is_gc)
+		wake_up_process(zi->zi_gc_thread);
+	else
+		wake_up_all(&zi->zi_zone_wait);
+
 	if (used < rtg_blocks(rtg))
 		xfs_zone_account_reclaimable(rtg, rtg_blocks(rtg) - used);
 	xfs_open_zone_put(oz);
@@ -557,6 +559,9 @@ xfs_try_use_zone(
 	struct xfs_open_zone	*oz,
 	unsigned int		goodness)
 {
+	if (oz->oz_is_gc)
+		return false;
+
 	if (oz->oz_allocated == rtg_blocks(oz->oz_rtg))
 		return false;
 
diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 2c2fa924fecd..30bcc415eaeb 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -125,6 +125,7 @@ struct xfs_zone_gc_iter {
  */
 struct xfs_zone_gc_data {
 	struct xfs_mount		*mp;
+	struct xfs_open_zone		*oz;
 
 	/* bioset used to allocate the gc_bios */
 	struct bio_set			bio_set;
@@ -525,9 +526,10 @@ xfs_zone_gc_select_victim(
 }
 
 static int
-xfs_zone_gc_steal_open(
-	struct xfs_zone_info	*zi)
+xfs_zone_gc_steal_open_zone(
+	struct xfs_zone_gc_data	*data)
 {
+	struct xfs_zone_info	*zi = data->mp->m_zone_info;
 	struct xfs_open_zone	*oz, *found = NULL;
 
 	spin_lock(&zi->zi_open_zones_lock);
@@ -542,10 +544,12 @@ xfs_zone_gc_steal_open(
 
 	trace_xfs_zone_gc_target_stolen(found->oz_rtg);
 	found->oz_is_gc = true;
-	list_del_init(&found->oz_entry);
 	zi->zi_nr_open_zones--;
-	zi->zi_open_gc_zone = found;
+	zi->zi_nr_open_gc_zones++;
 	spin_unlock(&zi->zi_open_zones_lock);
+
+	atomic_inc(&found->oz_ref);
+	data->oz = found;
 	return 0;
 }
 
@@ -554,39 +558,43 @@ xfs_zone_gc_steal_open(
  */
 static bool
 xfs_zone_gc_select_target(
-	struct xfs_mount	*mp)
+	struct xfs_zone_gc_data	*data)
 {
-	struct xfs_zone_info	*zi = mp->m_zone_info;
-	struct xfs_open_zone	*oz = zi->zi_open_gc_zone;
+	struct xfs_zone_info	*zi = data->mp->m_zone_info;
 
-	if (oz) {
+	if (data->oz) {
 		/*
 		 * If we have space available, just keep using the existing
 		 * zone.
 		 */
-		if (oz->oz_allocated < rtg_blocks(oz->oz_rtg))
+		if (data->oz->oz_allocated < rtg_blocks(data->oz->oz_rtg))
 			return true;
 
 		/*
 		 * Wait for all writes to the current zone to finish before
 		 * picking a new one.
 		 */
-		if (oz->oz_written < rtg_blocks(oz->oz_rtg))
+		if (data->oz->oz_written < rtg_blocks(data->oz->oz_rtg))
 			return false;
+
+		xfs_open_zone_put(data->oz);
 	}
 
 	/*
 	 * Open a new zone when there is none currently in use.
 	 */
 	ASSERT(zi->zi_nr_open_zones <=
-		mp->m_max_open_zones - XFS_OPEN_GC_ZONES);
-	oz = xfs_open_zone(mp, WRITE_LIFE_NOT_SET, true);
-	if (oz)
-		trace_xfs_zone_gc_target_opened(oz->oz_rtg);
+		data->mp->m_max_open_zones - XFS_OPEN_GC_ZONES);
+	data->oz = xfs_open_zone(data->mp, WRITE_LIFE_NOT_SET, true);
+	if (!data->oz)
+		return false;
+	trace_xfs_zone_gc_target_opened(data->oz->oz_rtg);
+	atomic_inc(&data->oz->oz_ref);
 	spin_lock(&zi->zi_open_zones_lock);
-	zi->zi_open_gc_zone = oz;
+	zi->zi_nr_open_gc_zones++;
+	list_add_tail(&data->oz->oz_entry, &zi->zi_open_zones);
 	spin_unlock(&zi->zi_open_zones_lock);
-	return !!oz;
+	return true;
 }
 
 static void
@@ -609,7 +617,7 @@ xfs_zone_gc_alloc_blocks(
 	bool			*is_seq)
 {
 	struct xfs_mount	*mp = data->mp;
-	struct xfs_open_zone	*oz = mp->m_zone_info->zi_open_gc_zone;
+	struct xfs_open_zone	*oz = data->oz;
 
 	*count_fsb = min(*count_fsb, XFS_B_TO_FSB(mp, data->scratch_available));
 
@@ -683,7 +691,7 @@ xfs_zone_gc_can_start_chunk(
 			return false;
 	}
 
-	return xfs_zone_gc_select_target(data->mp);
+	return xfs_zone_gc_select_target(data);
 }
 
 static bool
@@ -728,7 +736,7 @@ xfs_zone_gc_start_chunk(
 	chunk->new_daddr = daddr;
 	chunk->is_seq = is_seq;
 	chunk->data = data;
-	chunk->oz = mp->m_zone_info->zi_open_gc_zone;
+	chunk->oz = data->oz;
 	chunk->victim_rtg = iter->victim_rtg;
 	atomic_inc(&rtg_group(chunk->victim_rtg)->xg_active_ref);
 	atomic_inc(&chunk->victim_rtg->rtg_gccount);
@@ -1134,6 +1142,8 @@ xfs_zoned_gcd(
 	}
 	xfs_clear_zonegc_running(mp);
 
+	if (data->oz)
+		xfs_open_zone_put(data->oz);
 	if (data->iter.victim_rtg)
 		xfs_rtgroup_rele(data->iter.victim_rtg);
 
@@ -1183,6 +1193,10 @@ xfs_zone_gc_mount(
 	struct xfs_zone_gc_data	*data;
 	int			error;
 
+	data = xfs_zone_gc_data_alloc(mp);
+	if (!data)
+		return -ENOMEM;
+
 	/*
 	 * If there are no free zones available for GC, or the number of open
 	 * zones has reached the open zone limit, pick the open zone with
@@ -1192,35 +1206,30 @@ xfs_zone_gc_mount(
 	 */
 	if (!xfs_group_marked(mp, XG_TYPE_RTG, XFS_RTG_FREE) ||
 	    zi->zi_nr_open_zones >= mp->m_max_open_zones) {
-		error = xfs_zone_gc_steal_open(zi);
+		error = xfs_zone_gc_steal_open_zone(data);
 		if (error) {
 			xfs_warn(mp, "unable to steal an open zone for gc");
-			return error;
+			goto out_free_gc_data;
 		}
 	}
 
-	data = xfs_zone_gc_data_alloc(mp);
-	if (!data) {
-		error = -ENOMEM;
-		goto out_put_gc_zone;
-	}
-
 	zi->zi_gc_thread = kthread_create(xfs_zoned_gcd, data,
 			"xfs-zone-gc/%s", mp->m_super->s_id);
 	if (IS_ERR(zi->zi_gc_thread)) {
 		xfs_warn(mp, "unable to create zone gc thread");
 		error = PTR_ERR(zi->zi_gc_thread);
-		goto out_free_gc_data;
+		goto out_put_oz;
 	}
 
 	/* xfs_zone_gc_start will unpark for rw mounts */
 	kthread_park(zi->zi_gc_thread);
 	return 0;
 
+out_put_oz:
+	if (data->oz)
+		xfs_open_zone_put(data->oz);
 out_free_gc_data:
 	kfree(data);
-out_put_gc_zone:
-	xfs_open_zone_put(zi->zi_open_gc_zone);
 	return error;
 }
 
@@ -1231,6 +1240,4 @@ xfs_zone_gc_unmount(
 	struct xfs_zone_info	*zi = mp->m_zone_info;
 
 	kthread_stop(zi->zi_gc_thread);
-	if (zi->zi_open_gc_zone)
-		xfs_open_zone_put(zi->zi_open_gc_zone);
 }
diff --git a/fs/xfs/xfs_zone_info.c b/fs/xfs/xfs_zone_info.c
index a2af44011654..dcdc1dd206b2 100644
--- a/fs/xfs/xfs_zone_info.c
+++ b/fs/xfs/xfs_zone_info.c
@@ -30,11 +30,12 @@ xfs_show_open_zone(
 	struct seq_file		*m,
 	struct xfs_open_zone	*oz)
 {
-	seq_printf(m, "\t  zone %d, wp %u, written %u, used %u, hint %s\n",
+	seq_printf(m, "\t  zone %d, wp %u, written %u, used %u, hint %s %s\n",
 		rtg_rgno(oz->oz_rtg),
 		oz->oz_allocated, oz->oz_written,
 		rtg_rmap(oz->oz_rtg)->i_used_blocks,
-		xfs_write_hint_to_str(oz->oz_write_hint));
+		xfs_write_hint_to_str(oz->oz_write_hint),
+		oz->oz_is_gc ? "(GC)" : "");
 }
 
 static void
@@ -58,9 +59,8 @@ xfs_show_full_zone_used_distribution(
 	spin_unlock(&zi->zi_used_buckets_lock);
 
 	full = mp->m_sb.sb_rgcount;
-	if (zi->zi_open_gc_zone)
-		full--;
 	full -= zi->zi_nr_open_zones;
+	full -= zi->zi_nr_open_gc_zones;
 	full -= atomic_read(&zi->zi_nr_free_zones);
 	full -= reclaimable;
 
@@ -100,10 +100,6 @@ xfs_zoned_show_stats(
 	seq_puts(m, "\topen zones:\n");
 	list_for_each_entry(oz, &zi->zi_open_zones, oz_entry)
 		xfs_show_open_zone(m, oz);
-	if (zi->zi_open_gc_zone) {
-		seq_puts(m, "\topen gc zone:\n");
-		xfs_show_open_zone(m, zi->zi_open_gc_zone);
-	}
 	spin_unlock(&zi->zi_open_zones_lock);
 	seq_puts(m, "\tused blocks distribution (fully written zones):\n");
 	xfs_show_full_zone_used_distribution(m, mp);
diff --git a/fs/xfs/xfs_zone_priv.h b/fs/xfs/xfs_zone_priv.h
index 8fbf9a52964e..fcb57506d8e6 100644
--- a/fs/xfs/xfs_zone_priv.h
+++ b/fs/xfs/xfs_zone_priv.h
@@ -32,11 +32,7 @@ struct xfs_open_zone {
 	 */
 	enum rw_hint		oz_write_hint;
 
-	/*
-	 * Is this open zone used for garbage collection?  There can only be a
-	 * single open GC zone, which is pointed to by zi_open_gc_zone in
-	 * struct xfs_zone_info.  Constant over the life time of an open zone.
-	 */
+	/* Is this open zone used for garbage collection? */
 	bool			oz_is_gc;
 
 	/*
@@ -68,6 +64,7 @@ struct xfs_zone_info {
 	spinlock_t		zi_open_zones_lock;
 	struct list_head	zi_open_zones;
 	unsigned int		zi_nr_open_zones;
+	unsigned int		zi_nr_open_gc_zones;
 
 	/*
 	 * Free zone search cursor and number of free zones:
@@ -81,15 +78,9 @@ struct xfs_zone_info {
 	wait_queue_head_t	zi_zone_wait;
 
 	/*
-	 * Pointer to the GC thread, and the current open zone used by GC
-	 * (if any).
-	 *
-	 * zi_open_gc_zone is mostly private to the GC thread, but can be read
-	 * for debugging from other threads, in which case zi_open_zones_lock
-	 * must be taken to access it.
+	 * Pointer to the GC thread.
 	 */
 	struct task_struct      *zi_gc_thread;
-	struct xfs_open_zone	*zi_open_gc_zone;
 
 	/*
 	 * List of zones that need a reset:
-- 
2.47.3

[PATCH 8/9] xfs: expose the number of open zones in sysfs

[Christoph Hellwig]

Add a sysfs attribute for the current number of open zones so that it
can be trivially read from userspace in monitoring or testing software.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/admin-guide/xfs.rst |  4 ++++
 fs/xfs/xfs_sysfs.c                | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
index 746ea60eed3f..acdd4b65964c 100644
--- a/Documentation/admin-guide/xfs.rst
+++ b/Documentation/admin-guide/xfs.rst
@@ -550,6 +550,10 @@ For zoned file systems, the following attributes are exposed in:
 	is limited by the capabilities of the backing zoned device, file system
 	size and the max_open_zones mount option.
 
+  nr_open_zones			(Min:  0  Default:  Varies  Max:  UINTMAX)
+	This read-only attribute exposes the current number of open zones
+	used by the file system.
+
   zonegc_low_space		(Min:  0  Default:  0  Max:  100)
 	Define a percentage for how much of the unused space that GC should keep
 	available for writing. A high value will reclaim more of the space
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 4527119b2961..676777064c2d 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -13,6 +13,7 @@
 #include "xfs_log.h"
 #include "xfs_log_priv.h"
 #include "xfs_mount.h"
+#include "xfs_zone_priv.h"
 #include "xfs_zones.h"
 #include "xfs_zone_alloc.h"
 
@@ -719,6 +720,17 @@ max_open_zones_show(
 }
 XFS_SYSFS_ATTR_RO(max_open_zones);
 
+static ssize_t
+nr_open_zones_show(
+	struct kobject		*kobj,
+	char			*buf)
+{
+	struct xfs_zone_info	*zi = zoned_to_mp(kobj)->m_zone_info;
+
+	return sysfs_emit(buf, "%u\n", READ_ONCE(zi->zi_nr_open_zones));
+}
+XFS_SYSFS_ATTR_RO(nr_open_zones);
+
 static ssize_t
 zonegc_low_space_store(
 	struct kobject		*kobj,
@@ -756,6 +768,7 @@ XFS_SYSFS_ATTR_RW(zonegc_low_space);
 
 static struct attribute *xfs_zoned_attrs[] = {
 	ATTR_LIST(max_open_zones),
+	ATTR_LIST(nr_open_zones),
 	ATTR_LIST(zonegc_low_space),
 	NULL,
 };
-- 
2.47.3

Re: [PATCH 8/9] xfs: expose the number of open zones in sysfs

[Damien Le Moal]

On 4/1/26 00:27, Christoph Hellwig wrote:
> Add a sysfs attribute for the current number of open zones so that it
> can be trivially read from userspace in monitoring or testing software.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I am not sure if the READ_ONCE() not being paired with a WRITE_ONCE() really
helps here.

Nevertheless,

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

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 8/9] xfs: expose the number of open zones in sysfs

[Hans Holmberg]

On 31/03/2026 17:28, Christoph Hellwig wrote:
> Add a sysfs attribute for the current number of open zones so that it
> can be trivially read from userspace in monitoring or testing software.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  Documentation/admin-guide/xfs.rst |  4 ++++
>  fs/xfs/xfs_sysfs.c                | 13 +++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
> index 746ea60eed3f..acdd4b65964c 100644
> --- a/Documentation/admin-guide/xfs.rst
> +++ b/Documentation/admin-guide/xfs.rst
> @@ -550,6 +550,10 @@ For zoned file systems, the following attributes are exposed in:
>  	is limited by the capabilities of the backing zoned device, file system
>  	size and the max_open_zones mount option.
>  
> +  nr_open_zones			(Min:  0  Default:  Varies  Max:  UINTMAX)
> +	This read-only attribute exposes the current number of open zones
> +	used by the file system.
> +
>    zonegc_low_space		(Min:  0  Default:  0  Max:  100)
>  	Define a percentage for how much of the unused space that GC should keep
>  	available for writing. A high value will reclaim more of the space
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index 4527119b2961..676777064c2d 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -13,6 +13,7 @@
>  #include "xfs_log.h"
>  #include "xfs_log_priv.h"
>  #include "xfs_mount.h"
> +#include "xfs_zone_priv.h"
>  #include "xfs_zones.h"
>  #include "xfs_zone_alloc.h"
>  
> @@ -719,6 +720,17 @@ max_open_zones_show(
>  }
>  XFS_SYSFS_ATTR_RO(max_open_zones);
>  
> +static ssize_t
> +nr_open_zones_show(
> +	struct kobject		*kobj,
> +	char			*buf)
> +{
> +	struct xfs_zone_info	*zi = zoned_to_mp(kobj)->m_zone_info;
> +
> +	return sysfs_emit(buf, "%u\n", READ_ONCE(zi->zi_nr_open_zones));
> +}
> +XFS_SYSFS_ATTR_RO(nr_open_zones);
> +
>  static ssize_t
>  zonegc_low_space_store(
>  	struct kobject		*kobj,
> @@ -756,6 +768,7 @@ XFS_SYSFS_ATTR_RW(zonegc_low_space);
>  
>  static struct attribute *xfs_zoned_attrs[] = {
>  	ATTR_LIST(max_open_zones),
> +	ATTR_LIST(nr_open_zones),
>  	ATTR_LIST(zonegc_low_space),
>  	NULL,
>  };


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

[PATCH 9/9] xfs: untangle the open zones reporting in mountinfo

[Christoph Hellwig]

Keeping a value per line makes parsing much easier, so move the maximum
number of open zones into a separate line, and also add a new line for
the number of open open GC zones.  While that has to be either 0 or 1
currently having a value future-proofs the interface for adding more open
GC zones if needed.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hans Holmberg <hans.holmberg@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
---
 fs/xfs/xfs_zone_info.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_zone_info.c b/fs/xfs/xfs_zone_info.c
index dcdc1dd206b2..47b475e21af8 100644
--- a/fs/xfs/xfs_zone_info.c
+++ b/fs/xfs/xfs_zone_info.c
@@ -95,8 +95,12 @@ xfs_zoned_show_stats(
 	seq_printf(m, "\tfree zones: %d\n", atomic_read(&zi->zi_nr_free_zones));
 
 	spin_lock(&zi->zi_open_zones_lock);
-	seq_printf(m, "\tnumber of open zones: %u / %u\n",
-		zi->zi_nr_open_zones, mp->m_max_open_zones);
+	seq_printf(m, "\tmax open zones: %u\n",
+		mp->m_max_open_zones);
+	seq_printf(m, "\tnr open zones: %u\n",
+		zi->zi_nr_open_zones);
+	seq_printf(m, "\tnr open GC zones: %u\n",
+		zi->zi_nr_open_gc_zones);
 	seq_puts(m, "\topen zones:\n");
 	list_for_each_entry(oz, &zi->zi_open_zones, oz_entry)
 		xfs_show_open_zone(m, oz);
-- 
2.47.3

Re: cleanup open GC zone handling v2

[Carlos Maiolino]

On Tue, 31 Mar 2026 17:27:22 +0200, Christoph Hellwig wrote:
> this series cleans up the handling of open GC zones so that the
> reference counting works more similar to normal open GC zones.
> This cleans up the code in general, and also helps with experiments
> to support multiple open zones for GC I've started.
> 
> Changes since v1:
>  - improve various commit messages
>  - move over two patches to heree from a different series
>  - use READ_ONCE in nr_open_zones_show
>  - fix the nr_open_zones_show documentation
> 
> [...]

Applied to for-next, thanks!

[1/9] xfs: delay initial open of the GC zone
      commit: e771da0727c13ab2cb89ee47f2edcda8b87b2c73
[2/9] xfs: add a separate tracepoint for stealing an open zone for GC
      commit: c2257d9f63bbf7e1f39fb2b5585b21ea7445e18f
[3/9] xfs: put the open zone later xfs_open_zone_put
      commit: a99ed5dbae018627faf4ac275cb322e94606a6f9
[4/9] xfs: rename xfs_zone_gc_iter_next to xfs_zone_gc_iter_irec
      commit: ca0170a7fa41fa4ef5e31b0baf2225d84a385bfc
[5/9] xfs: refactor GC zone selection helpers
      commit: 53c1c822908d9804504596486b96d8b887b0bacd
[6/9] xfs: streamline GC zone selection
      commit: 4c1b6e03e31c5933355f25fe7fa564be3a0f931d
[7/9] xfs: reduce special casing for the open GC zone
      commit: 1d0d9e9459c053fc1700739e267271adccdcaadc
[8/9] xfs: expose the number of open zones in sysfs
      commit: 62c89988dc198efc17be0119a43ad21cf32334d6
[9/9] xfs: untangle the open zones reporting in mountinfo
      commit: 9de45faed34d11f1821c386ea306d9788e9a6448

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