[PATCH v3 0/4] btrfs: zoned: fix active-zone accounting and transitions

[PATCH v3 0/4] btrfs: zoned: fix active-zone accounting and transitions

[Dongjiang Zhu]

After systems using HC620 host-managed zoned devices were upgraded to
Linux 6.18, multiple users reported transaction writeback failures with
-EAGAIN that forced filesystems read-only, as well as hung tasks.

Enforcing the device's max_active_zones=128 limit exposed existing
active-zone handling problems.  We reproduced representative cases
locally, including an -EAGAIN transaction abort that forced the filesystem
read-only and a balance self-deadlock that stalled transaction commit and
data writers.

Further investigation traced these failures to several related gaps in
Btrfs active-zone handling around mount recovery, non-data block group
activation and zone finishing:

- Conventional zones participate in runtime active-zone accounting, but
  that state cannot be reconstructed from device zone conditions at mount.
- Active metadata and system block groups are reconstructed at mount, but
  their runtime write roles are not.
- An obsolete allocation-time activation helper bypasses the write-time
  role pivot and can recursively enter zone finishing.
- do_zone_finish() clears the active state before the device operation has
  released the active-zone slot.

Together, these bugs can leave no active-zone slot available to metadata
or system writeback, or recursively enter zone finishing while holding
ro_block_group_mutex, resulting in the two failure modes described above.

Fix the active-zone model and its state transitions as follows:

1. Track only sequential zones as active.
2. Recover the normal metadata and system write roles on mount, and finish
   extra active block groups left by the old behavior.
3. Remove the obsolete allocation-time non-data activation helper so
   metadata and system activation goes through the write-time role pivot.
4. Serialize concurrent finish operations on the same block group and keep
   it active until the device operation and software cleanup complete.

The individual commits contain the corresponding reproduction conditions,
kernel stacks and state-transition traces.

Changes in v3:
- Dropped the block-group flag comment changes from patch 1; no functional
  changes.

Changes in v2:
- Removed the [BUG], [CAUSE] and [FIX] headings from the commit messages.
- Added a helper for identifying zones that need active-zone tracking.
- Added block group locking around alloc_offset and zone_unusable accesses
  during mount recovery.
- Explained why moving the active-bit clear alone does not serialize
  concurrent zone finish operations.

Previous versions:
v2:
https://lore.kernel.org/linux-btrfs/cover.1787717573.git.zhudongjiang@fygo.io/
v1:
https://lore.kernel.org/linux-btrfs/cover.1787551793.git.zhudongjiang@fygo.io/

Dongjiang Zhu (4):
  btrfs: zoned: track only sequential zones as active
  btrfs: zoned: recover active non-data block group roles on mount
  btrfs: zoned: remove obsolete non-data block group activation helper
  btrfs: zoned: serialize zone finishing per block group

 fs/btrfs/block-group.c |  38 ++------
 fs/btrfs/block-group.h |   1 +
 fs/btrfs/disk-io.c     |   7 +-
 fs/btrfs/zoned.c       | 211 +++++++++++++++++++++++++++++------------
 fs/btrfs/zoned.h       |   9 +-
 5 files changed, 167 insertions(+), 99 deletions(-)

-- 
2.39.5

[PATCH v3 1/4] btrfs: zoned: track only sequential zones as active

[Dongjiang Zhu]

The runtime activation and finish paths apply active-zone accounting to
both conventional and sequential stripes.

At mount, however, active_zones_left is rebuilt from device zone
conditions.  Conventional zones report NOT_WP and are therefore not
included.  Meanwhile, all-conventional block groups are marked active
and added to zone_active_bgs, so mount recovery still decreases
reserved_active_zones for their metadata and system stripes.

The two counters can therefore diverge across a remount.

Each unmatched reservation decrement lets data consume one more
sequential active-zone slot.  Once the reservation becomes negative,
data can exhaust all such slots.

A later transition from a conventional metadata or system target to a
sequential one can then fail, because finishing the conventional target
does not release a sequential active-zone slot.

If this happens during synchronous transaction writeback,
btrfs_check_meta_write_pointer() returns -EAGAIN.  The transaction is
then aborted and the filesystem is forced read-only.

The reservation mismatch was reproduced by creating small files with
large xattrs in batches, syncing them and remounting an HC620 filesystem
using SINGLE profiles:

  active_zones_left:     126
  reserved_active_zones: 3 -> -8

The active list contained one partially used conventional system block
group, nine full conventional metadata block groups and one partially
used conventional metadata block group.

To keep both counters based on the same physical resource, use
sequential stripes as the unit of active-zone tracking.  Accordingly, do
not mark all-conventional block groups active, and update the active-zone
bitmap and non-data reservation only for sequential stripes during
activation, finish and mount recovery.

Assisted-by: LLM
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Dongjiang Zhu <zhudongjiang@fygo.io>
---
 fs/btrfs/zoned.c | 59 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 9d448cdd60c4..0d964ada3ad4 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1969,7 +1969,6 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
 		} else if (map->num_stripes == num_conventional) {
 			cache->alloc_offset = last_alloc;
 			cache->zone_capacity = cache->length;
-			set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &cache->runtime_flags);
 			goto out;
 		}
 	}
@@ -2005,6 +2004,8 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
 	if (!ret) {
 		cache->meta_write_pointer = cache->alloc_offset + cache->start;
 		if (test_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &cache->runtime_flags)) {
+			ASSERT(test_bit(BLOCK_GROUP_FLAG_SEQUENTIAL_ZONE,
+					&cache->runtime_flags));
 			btrfs_get_block_group(cache);
 			spin_lock(&fs_info->zone_active_bgs_lock);
 			list_add_tail(&cache->active_bg_list,
@@ -2198,6 +2199,9 @@ static bool check_bg_is_active(struct btrfs_eb_write_context *ctx,
 	if (test_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &block_group->runtime_flags))
 		return true;
 
+	if (!test_bit(BLOCK_GROUP_FLAG_SEQUENTIAL_ZONE, &block_group->runtime_flags))
+		return true;
+
 	if (fs_info->treelog_bg == block_group->start) {
 		if (!btrfs_zone_activate(block_group)) {
 			int ret_fin;
@@ -2394,6 +2398,18 @@ int btrfs_sync_zone_write_pointer(struct btrfs_device *tgt_dev, u64 logical,
 	return btrfs_zoned_issue_zeroout(tgt_dev, physical_pos, length);
 }
 
+static inline bool btrfs_needs_active_zone_tracking(struct btrfs_device *dev,
+						    u64 physical)
+{
+	if (!btrfs_dev_is_sequential(dev, physical))
+		return false;
+
+	if (dev->zone_info->max_active_zones == 0)
+		return false;
+
+	return true;
+}
+
 /*
  * Activate block group and underlying device zones
  *
@@ -2417,6 +2433,9 @@ bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 	if (unlikely(btrfs_is_testing(fs_info)))
 		return true;
 
+	if (!test_bit(BLOCK_GROUP_FLAG_SEQUENTIAL_ZONE, &block_group->runtime_flags))
+		return true;
+
 	map = block_group->physical_map;
 
 	spin_lock(&fs_info->zone_active_bgs_lock);
@@ -2448,7 +2467,7 @@ bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 		if (!device->bdev)
 			continue;
 
-		if (zinfo->max_active_zones == 0)
+		if (!btrfs_needs_active_zone_tracking(device, physical))
 			continue;
 
 		if (is_data)
@@ -2514,26 +2533,22 @@ static int call_zone_finish(struct btrfs_block_group *block_group,
 	struct btrfs_device *device = stripe->dev;
 	const u64 physical = stripe->physical;
 	struct btrfs_zoned_device_info *zinfo = device->zone_info;
+	unsigned int nofs_flags;
 	int ret;
 
 	if (!device->bdev)
 		return 0;
 
-	if (zinfo->max_active_zones == 0)
+	if (!btrfs_needs_active_zone_tracking(device, physical))
 		return 0;
 
-	if (btrfs_dev_is_sequential(device, physical)) {
-		unsigned int nofs_flags;
-
-		nofs_flags = memalloc_nofs_save();
-		ret = blkdev_zone_mgmt(device->bdev, REQ_OP_ZONE_FINISH,
-				       physical >> SECTOR_SHIFT,
-				       zinfo->zone_size >> SECTOR_SHIFT);
-		memalloc_nofs_restore(nofs_flags);
-
-		if (ret)
-			return ret;
-	}
+	nofs_flags = memalloc_nofs_save();
+	ret = blkdev_zone_mgmt(device->bdev, REQ_OP_ZONE_FINISH,
+			       physical >> SECTOR_SHIFT,
+			       zinfo->zone_size >> SECTOR_SHIFT);
+	memalloc_nofs_restore(nofs_flags);
+	if (ret)
+		return ret;
 
 	if (!(block_group->flags & BTRFS_BLOCK_GROUP_DATA))
 		zinfo->reserved_active_zones++;
@@ -3087,8 +3102,18 @@ void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info)
 		      (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_SYSTEM)))
 			continue;
 
-		for (int i = 0; i < map->num_stripes; i++)
-			map->stripes[i].dev->zone_info->reserved_active_zones--;
+		for (int i = 0; i < map->num_stripes; i++) {
+			struct btrfs_device *device = map->stripes[i].dev;
+			u64 physical = map->stripes[i].physical;
+
+			if (!device->bdev)
+				continue;
+
+			if (!btrfs_needs_active_zone_tracking(device, physical))
+				continue;
+
+			device->zone_info->reserved_active_zones--;
+		}
 	}
 	spin_unlock(&fs_info->zone_active_bgs_lock);
 }
-- 
2.39.5

Re: [PATCH v3 1/4] btrfs: zoned: track only sequential zones as active

[Naohiro Aota]

On Fri Aug 28, 2026 at 12:51 PM JST, Dongjiang Zhu wrote:
> The runtime activation and finish paths apply active-zone accounting to
> both conventional and sequential stripes.
>
> At mount, however, active_zones_left is rebuilt from device zone
> conditions.  Conventional zones report NOT_WP and are therefore not
> included.  Meanwhile, all-conventional block groups are marked active
> and added to zone_active_bgs, so mount recovery still decreases
> reserved_active_zones for their metadata and system stripes.
>
> The two counters can therefore diverge across a remount.
>
> Each unmatched reservation decrement lets data consume one more
> sequential active-zone slot.  Once the reservation becomes negative,
> data can exhaust all such slots.
>
> A later transition from a conventional metadata or system target to a
> sequential one can then fail, because finishing the conventional target
> does not release a sequential active-zone slot.
>
> If this happens during synchronous transaction writeback,
> btrfs_check_meta_write_pointer() returns -EAGAIN.  The transaction is
> then aborted and the filesystem is forced read-only.
>
> The reservation mismatch was reproduced by creating small files with
> large xattrs in batches, syncing them and remounting an HC620 filesystem
> using SINGLE profiles:
>
>   active_zones_left:     126
>   reserved_active_zones: 3 -> -8
>
> The active list contained one partially used conventional system block
> group, nine full conventional metadata block groups and one partially
> used conventional metadata block group.
>
> To keep both counters based on the same physical resource, use
> sequential stripes as the unit of active-zone tracking.  Accordingly, do
> not mark all-conventional block groups active, and update the active-zone
> bitmap and non-data reservation only for sequential stripes during
> activation, finish and mount recovery.
>
> Assisted-by: LLM
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Signed-off-by: Dongjiang Zhu <zhudongjiang@fygo.io>
> ---
>  fs/btrfs/zoned.c | 59 ++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 42 insertions(+), 17 deletions(-)

I'm sorry to miss this version, but the same comment applies on this
version too.

https://lore.kernel.org/linux-btrfs/DL0AVDP5O9C2.1VT2TYQXCL4B4@wdc.com/

[PATCH v3 2/4] btrfs: zoned: recover active non-data block group roles on mount

[Dongjiang Zhu]

After a remount, a later write can activate a new metadata or system
block group without finishing the recovered one.  Repeating this across
remounts consumes additional active-zone slots and reservations, and can
eventually exhaust all active-zone slots.

Once no slot is available, activation of a metadata or system block group
fails.  During synchronous transaction writeback this propagates as
-EAGAIN, aborting the transaction and forcing the filesystem read-only.

A remount/fsync/remount reproducer on an HC620 left two partially used
sequential metadata block groups active while active_meta_bg remained
NULL:

  bg=343865819136 alloc=176095232 cap=268435456
  bg=344402690048 alloc=3260416   cap=268435456

active_meta_bg and active_system_bg assign active block groups to the
normal metadata and system write roles.  These runtime-only pointers are
reset to NULL on every mount.

Mount reconstructs partially used sequential block groups in
zone_active_bgs and adjusts their reservations, but does not recover the
role assignments.

The write-time pivot can finish the previous block group only when the
corresponding role pointer is set.  If it is NULL, activation proceeds
without releasing an already active block group of the same role.

Restore the role assignments by selecting one active block group for the
normal metadata role and one for the system role from zone_active_bgs.
For each role, keep the block group with the largest writable tail and
finish the others.

The tree-log block group role is runtime-only and cannot be recovered
after a remount.  Use the selected metadata block group for the normal
metadata role and leave one metadata reservation for a new tree-log block
group.

Run the recovery on every mount.  ZONE_FINISH preserves existing extents
but abandons the unused tail, so account that tail as zone unusable
immediately.

Assisted-by: LLM
Signed-off-by: Dongjiang Zhu <zhudongjiang@fygo.io>
---
 fs/btrfs/disk-io.c |  7 +++-
 fs/btrfs/zoned.c   | 97 ++++++++++++++++++++++++++++++++++++++++++----
 fs/btrfs/zoned.h   |  7 +++-
 3 files changed, 101 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index e6bbb0b1b38c..cd1f46ec62f0 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3713,7 +3713,12 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 
 	btrfs_free_zone_cache(fs_info);
 
-	btrfs_check_active_zone_reservation(fs_info);
+	ret = btrfs_restore_active_nondata_bgs(fs_info);
+	if (ret) {
+		btrfs_err(fs_info, "failed to restore active non-data block groups: %pe",
+			  ERR_PTR(ret));
+		goto fail_sysfs;
+	}
 
 	if (!sb_rdonly(sb) && fs_info->fs_devices->missing_devices &&
 	    !btrfs_check_rw_degradable(fs_info, NULL)) {
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 0d964ada3ad4..ee7d518bec15 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -3056,22 +3056,77 @@ int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_info, bool do_fin
 	return 0;
 }
 
+static int finish_extra_active_nondata_bgs(struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_block_group *block_group;
+	struct btrfs_block_group *next;
+	u64 tail_unusable;
+	int ret;
+
+	list_for_each_entry_safe(block_group, next, &fs_info->zone_active_bgs,
+				 active_bg_list) {
+		if (!(block_group->flags &
+		      (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_SYSTEM)))
+			continue;
+
+		if (block_group == fs_info->active_meta_bg ||
+		    block_group == fs_info->active_system_bg)
+			continue;
+
+		btrfs_get_block_group(block_group);
+		spin_lock(&block_group->lock);
+		tail_unusable = block_group->zone_capacity - block_group->alloc_offset;
+		spin_unlock(&block_group->lock);
+
+		ret = do_zone_finish(block_group, true);
+		if (!ret) {
+			struct btrfs_space_info *space_info = block_group->space_info;
+
+			/* Account for the unused tail abandoned by ZONE_FINISH. */
+			spin_lock(&space_info->lock);
+			spin_lock(&block_group->lock);
+			block_group->zone_unusable += tail_unusable;
+			btrfs_space_info_update_bytes_zone_unusable(space_info,
+								    tail_unusable);
+			spin_unlock(&block_group->lock);
+			spin_unlock(&space_info->lock);
+		}
+		btrfs_put_block_group(block_group);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 /*
- * Reserve zones for one metadata block group, one tree-log block group, and one
- * system block group.
+ * Restore one active block group for normal metadata and system writes.
+ *
+ * The tree-log block group identity is not persisted, so recovered active
+ * metadata block groups cannot be distinguished by their previous role. Keep
+ * one for the normal metadata role and leave one reservation for a new
+ * tree-log block group.
+ *
+ * Older kernels rebuilt zone_active_bgs at mount without recovering the role
+ * assignments, which could result in multiple active metadata or system block
+ * groups. Finish all but the selected block group for each role.
  */
-void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info)
+int btrfs_restore_active_nondata_bgs(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
 	struct btrfs_block_group *block_group;
+	struct btrfs_block_group *active_meta_bg = NULL;
+	struct btrfs_block_group *active_system_bg = NULL;
 	struct btrfs_device *device;
+	u64 active_meta_avail = 0;
+	u64 active_system_avail = 0;
 	/* Reserve zones for normal SINGLE metadata and tree-log block group. */
 	unsigned int metadata_reserve = 2;
 	/* Reserve a zone for SINGLE system block group. */
 	unsigned int system_reserve = 1;
 
 	if (!test_bit(BTRFS_FS_ACTIVE_ZONE_TRACKING, &fs_info->flags))
-		return;
+		return 0;
 
 	/*
 	 * This function is called from the mount context. So, there is no
@@ -3093,14 +3148,38 @@ void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info)
 	}
 	mutex_unlock(&fs_devices->device_list_mutex);
 
-	/* Release reservation for currently active block groups. */
+	/*
+	 * Account all active non-data block groups before selecting one for each role.
+	 * Finishing the extra block groups releases their reservations again.
+	 */
 	spin_lock(&fs_info->zone_active_bgs_lock);
 	list_for_each_entry(block_group, &fs_info->zone_active_bgs, active_bg_list) {
 		struct btrfs_chunk_map *map = block_group->physical_map;
+		struct btrfs_block_group **active_bg;
+		u64 *active_avail;
+		u64 avail;
 
-		if (!(block_group->flags &
-		      (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_SYSTEM)))
+		if (block_group->flags & BTRFS_BLOCK_GROUP_METADATA) {
+			active_bg = &active_meta_bg;
+			active_avail = &active_meta_avail;
+		} else if (block_group->flags & BTRFS_BLOCK_GROUP_SYSTEM) {
+			active_bg = &active_system_bg;
+			active_avail = &active_system_avail;
+		} else {
 			continue;
+		}
+
+		spin_lock(&block_group->lock);
+		avail = block_group->zone_capacity - block_group->alloc_offset;
+		spin_unlock(&block_group->lock);
+
+		if (!*active_bg || avail > *active_avail) {
+			if (*active_bg)
+				btrfs_put_block_group(*active_bg);
+			*active_bg = block_group;
+			*active_avail = avail;
+			btrfs_get_block_group(*active_bg);
+		}
 
 		for (int i = 0; i < map->num_stripes; i++) {
 			struct btrfs_device *device = map->stripes[i].dev;
@@ -3115,7 +3194,11 @@ void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info)
 			device->zone_info->reserved_active_zones--;
 		}
 	}
+	fs_info->active_meta_bg = active_meta_bg;
+	fs_info->active_system_bg = active_system_bg;
 	spin_unlock(&fs_info->zone_active_bgs_lock);
+
+	return finish_extra_active_nondata_bgs(fs_info);
 }
 
 /*
diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h
index 8e21a836f858..934a9368ed15 100644
--- a/fs/btrfs/zoned.h
+++ b/fs/btrfs/zoned.h
@@ -95,7 +95,7 @@ void btrfs_zoned_release_data_reloc_bg(struct btrfs_fs_info *fs_info, u64 logica
 				       u64 length);
 int btrfs_zone_finish_one_bg(struct btrfs_fs_info *fs_info);
 int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_info, bool do_finish);
-void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info);
+int btrfs_restore_active_nondata_bgs(struct btrfs_fs_info *fs_info);
 int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num_bytes);
 void btrfs_show_zoned_stats(struct btrfs_fs_info *fs_info, struct seq_file *seq);
 
@@ -279,7 +279,10 @@ static inline int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_inf
 	return 0;
 }
 
-static inline void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info) { }
+static inline int btrfs_restore_active_nondata_bgs(struct btrfs_fs_info *fs_info)
+{
+	return 0;
+}
 
 static inline int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info,
 						  u64 num_bytes)
-- 
2.39.5

Re: [PATCH v3 2/4] btrfs: zoned: recover active non-data block group roles on mount

[Naohiro Aota]

On Fri Aug 28, 2026 at 12:51 PM JST, Dongjiang Zhu wrote:
> After a remount, a later write can activate a new metadata or system
> block group without finishing the recovered one.  Repeating this across
> remounts consumes additional active-zone slots and reservations, and can
> eventually exhaust all active-zone slots.
>
> Once no slot is available, activation of a metadata or system block group
> fails.  During synchronous transaction writeback this propagates as
> -EAGAIN, aborting the transaction and forcing the filesystem read-only.
>
> A remount/fsync/remount reproducer on an HC620 left two partially used
> sequential metadata block groups active while active_meta_bg remained
> NULL:
>
>   bg=343865819136 alloc=176095232 cap=268435456
>   bg=344402690048 alloc=3260416   cap=268435456

Please show the reproducer.

>
> active_meta_bg and active_system_bg assign active block groups to the
> normal metadata and system write roles.  These runtime-only pointers are
> reset to NULL on every mount.
>
> Mount reconstructs partially used sequential block groups in
> zone_active_bgs and adjusts their reservations, but does not recover the
> role assignments.
>
> The write-time pivot can finish the previous block group only when the
> corresponding role pointer is set.  If it is NULL, activation proceeds
> without releasing an already active block group of the same role.

True. But, it is set to the written block group if e.g, active_meta_bg
== NULL. Given that metadata is properly written to partially used block
group, isn't it no problem with that? Apparently, this write-time
setting can be considered as the role recovery.

If it is writing into another block group leaving partially used one
intact, then that is an issue on metadata allocation. 

>
> Restore the role assignments by selecting one active block group for the
> normal metadata role and one for the system role from zone_active_bgs.
> For each role, keep the block group with the largest writable tail and
> finish the others.
>
> The tree-log block group role is runtime-only and cannot be recovered
> after a remount.  Use the selected metadata block group for the normal
> metadata role and leave one metadata reservation for a new tree-log block
> group.
>
> Run the recovery on every mount.  ZONE_FINISH preserves existing extents
> but abandons the unused tail, so account that tail as zone unusable
> immediately.
>
> Assisted-by: LLM
> Signed-off-by: Dongjiang Zhu <zhudongjiang@fygo.io>
> ---
>  fs/btrfs/disk-io.c |  7 +++-
>  fs/btrfs/zoned.c   | 97 ++++++++++++++++++++++++++++++++++++++++++----
>  fs/btrfs/zoned.h   |  7 +++-
>  3 files changed, 101 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index e6bbb0b1b38c..cd1f46ec62f0 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3713,7 +3713,12 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>  
>  	btrfs_free_zone_cache(fs_info);
>  
> -	btrfs_check_active_zone_reservation(fs_info);
> +	ret = btrfs_restore_active_nondata_bgs(fs_info);
> +	if (ret) {
> +		btrfs_err(fs_info, "failed to restore active non-data block groups: %pe",
> +			  ERR_PTR(ret));
> +		goto fail_sysfs;
> +	}
>  
>  	if (!sb_rdonly(sb) && fs_info->fs_devices->missing_devices &&
>  	    !btrfs_check_rw_degradable(fs_info, NULL)) {
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index 0d964ada3ad4..ee7d518bec15 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -3056,22 +3056,77 @@ int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_info, bool do_fin
>  	return 0;
>  }
>  
> +static int finish_extra_active_nondata_bgs(struct btrfs_fs_info *fs_info)
> +{
> +	struct btrfs_block_group *block_group;
> +	struct btrfs_block_group *next;
> +	u64 tail_unusable;
> +	int ret;
> +
> +	list_for_each_entry_safe(block_group, next, &fs_info->zone_active_bgs,
> +				 active_bg_list) {
> +		if (!(block_group->flags &
> +		      (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_SYSTEM)))
> +			continue;
> +
> +		if (block_group == fs_info->active_meta_bg ||
> +		    block_group == fs_info->active_system_bg)
> +			continue;
> +
> +		btrfs_get_block_group(block_group);

Why do we need to do this?

> +		spin_lock(&block_group->lock);
> +		tail_unusable = block_group->zone_capacity - block_group->alloc_offset;
> +		spin_unlock(&block_group->lock);
> +
> +		ret = do_zone_finish(block_group, true);
> +		if (!ret) {
> +			struct btrfs_space_info *space_info = block_group->space_info;
> +
> +			/* Account for the unused tail abandoned by ZONE_FINISH. */
> +			spin_lock(&space_info->lock);
> +			spin_lock(&block_group->lock);
> +			block_group->zone_unusable += tail_unusable;
> +			btrfs_space_info_update_bytes_zone_unusable(space_info,
> +								    tail_unusable);
> +			spin_unlock(&block_group->lock);
> +			spin_unlock(&space_info->lock);
> +		}

So, basically, do_zone_finish() should have handled this...

> +		btrfs_put_block_group(block_group);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
> - * Reserve zones for one metadata block group, one tree-log block group, and one
> - * system block group.
> + * Restore one active block group for normal metadata and system writes.
> + *
> + * The tree-log block group identity is not persisted, so recovered active
> + * metadata block groups cannot be distinguished by their previous role. Keep
> + * one for the normal metadata role and leave one reservation for a new
> + * tree-log block group.
> + *
> + * Older kernels rebuilt zone_active_bgs at mount without recovering the role
> + * assignments, which could result in multiple active metadata or system block
> + * groups. Finish all but the selected block group for each role.
>   */
> -void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info)
> +int btrfs_restore_active_nondata_bgs(struct btrfs_fs_info *fs_info)
>  {
>  	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
>  	struct btrfs_block_group *block_group;
> +	struct btrfs_block_group *active_meta_bg = NULL;
> +	struct btrfs_block_group *active_system_bg = NULL;
>  	struct btrfs_device *device;
> +	u64 active_meta_avail = 0;
> +	u64 active_system_avail = 0;
>  	/* Reserve zones for normal SINGLE metadata and tree-log block group. */
>  	unsigned int metadata_reserve = 2;
>  	/* Reserve a zone for SINGLE system block group. */
>  	unsigned int system_reserve = 1;
>  
>  	if (!test_bit(BTRFS_FS_ACTIVE_ZONE_TRACKING, &fs_info->flags))
> -		return;
> +		return 0;
>  
>  	/*
>  	 * This function is called from the mount context. So, there is no
> @@ -3093,14 +3148,38 @@ void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info)
>  	}
>  	mutex_unlock(&fs_devices->device_list_mutex);
>  
> -	/* Release reservation for currently active block groups. */
> +	/*
> +	 * Account all active non-data block groups before selecting one for each role.
> +	 * Finishing the extra block groups releases their reservations again.
> +	 */
>  	spin_lock(&fs_info->zone_active_bgs_lock);
>  	list_for_each_entry(block_group, &fs_info->zone_active_bgs, active_bg_list) {
>  		struct btrfs_chunk_map *map = block_group->physical_map;
> +		struct btrfs_block_group **active_bg;
> +		u64 *active_avail;
> +		u64 avail;
>  
> -		if (!(block_group->flags &
> -		      (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_SYSTEM)))
> +		if (block_group->flags & BTRFS_BLOCK_GROUP_METADATA) {
> +			active_bg = &active_meta_bg;
> +			active_avail = &active_meta_avail;
> +		} else if (block_group->flags & BTRFS_BLOCK_GROUP_SYSTEM) {
> +			active_bg = &active_system_bg;
> +			active_avail = &active_system_avail;
> +		} else {
>  			continue;
> +		}
> +
> +		spin_lock(&block_group->lock);
> +		avail = block_group->zone_capacity - block_group->alloc_offset;
> +		spin_unlock(&block_group->lock);
> +
> +		if (!*active_bg || avail > *active_avail) {
> +			if (*active_bg)
> +				btrfs_put_block_group(*active_bg);
> +			*active_bg = block_group;
> +			*active_avail = avail;
> +			btrfs_get_block_group(*active_bg);
> +		}
>  
>  		for (int i = 0; i < map->num_stripes; i++) {
>  			struct btrfs_device *device = map->stripes[i].dev;
> @@ -3115,7 +3194,11 @@ void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info)
>  			device->zone_info->reserved_active_zones--;
>  		}
>  	}
> +	fs_info->active_meta_bg = active_meta_bg;
> +	fs_info->active_system_bg = active_system_bg;
>  	spin_unlock(&fs_info->zone_active_bgs_lock);
> +
> +	return finish_extra_active_nondata_bgs(fs_info);
>  }
>  
>  /*
> diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h
> index 8e21a836f858..934a9368ed15 100644
> --- a/fs/btrfs/zoned.h
> +++ b/fs/btrfs/zoned.h
> @@ -95,7 +95,7 @@ void btrfs_zoned_release_data_reloc_bg(struct btrfs_fs_info *fs_info, u64 logica
>  				       u64 length);
>  int btrfs_zone_finish_one_bg(struct btrfs_fs_info *fs_info);
>  int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_info, bool do_finish);
> -void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info);
> +int btrfs_restore_active_nondata_bgs(struct btrfs_fs_info *fs_info);
>  int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num_bytes);
>  void btrfs_show_zoned_stats(struct btrfs_fs_info *fs_info, struct seq_file *seq);
>  
> @@ -279,7 +279,10 @@ static inline int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_inf
>  	return 0;
>  }
>  
> -static inline void btrfs_check_active_zone_reservation(struct btrfs_fs_info *fs_info) { }
> +static inline int btrfs_restore_active_nondata_bgs(struct btrfs_fs_info *fs_info)
> +{
> +	return 0;
> +}
>  
>  static inline int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info,
>  						  u64 num_bytes)

Re: [PATCH v3 2/4] btrfs: zoned: recover active non-data block group roles on mount

[Dongjiang Zhu]

在 2026/8/28 13:43, Naohiro Aota 写道:
> On Fri Aug 28, 2026 at 12:51 PM JST, Dongjiang Zhu wrote:
>> After a remount, a later write can activate a new metadata or system
>> block group without finishing the recovered one.  Repeating this across
>> remounts consumes additional active-zone slots and reservations, and can
>> eventually exhaust all active-zone slots.
>>
>> Once no slot is available, activation of a metadata or system block group
>> fails.  During synchronous transaction writeback this propagates as
>> -EAGAIN, aborting the transaction and forcing the filesystem read-only.
>>
>> A remount/fsync/remount reproducer on an HC620 left two partially used
>> sequential metadata block groups active while active_meta_bg remained
>> NULL:
>>
>>    bg=343865819136 alloc=176095232 cap=268435456
>>    bg=344402690048 alloc=3260416   cap=268435456
> 
> Please show the reproducer.
> 

Sure.  The two block groups were not both for normal metadata.  One was
the existing partially used normal metadata block group, while the other
was newly allocated for the tree log, as confirmed by the tree-log
allocation trace.

The original reproducer contains a fairly large setup stage.  I will
simplify it and include the exact reproducer in the next revision of the
commit message.

>>
>> active_meta_bg and active_system_bg assign active block groups to the
>> normal metadata and system write roles.  These runtime-only pointers are
>> reset to NULL on every mount.
>>
>> Mount reconstructs partially used sequential block groups in
>> zone_active_bgs and adjusts their reservations, but does not recover the
>> role assignments.
>>
>> The write-time pivot can finish the previous block group only when the
>> corresponding role pointer is set.  If it is NULL, activation proceeds
>> without releasing an already active block group of the same role.
> 
> True. But, it is set to the written block group if e.g, active_meta_bg
> == NULL. Given that metadata is properly written to partially used block
> group, isn't it no problem with that? Apparently, this write-time
> setting can be considered as the role recovery.
> 
> If it is writing into another block group leaving partially used one
> intact, then that is an issue on metadata allocation.
> 

Thanks for pointing this out.  You are right that the commit message
overstates the problem.  With the current allocation behavior, normal
metadata continues to use the existing partially used block group, so
the sequence described in the commit message should not occur for normal
metadata.

A lazy recovery model can therefore work with the current allocator.
However, I still think restoring the role at mount is preferable, as it
keeps the write-time pivot state independent of allocator behavior.

Also, since the tree-log role is not persisted, mount must select one
metadata block group and finish the extras.  Restoring the normal
metadata role at the same time is straightforward.

I will correct the commit message accordingly.

>>
>> Restore the role assignments by selecting one active block group for the
>> normal metadata role and one for the system role from zone_active_bgs.
>> For each role, keep the block group with the largest writable tail and
>> finish the others.
>>
>> The tree-log block group role is runtime-only and cannot be recovered
>> after a remount.  Use the selected metadata block group for the normal
>> metadata role and leave one metadata reservation for a new tree-log block
>> group.
>>
>> Run the recovery on every mount.  ZONE_FINISH preserves existing extents
>> but abandons the unused tail, so account that tail as zone unusable
>> immediately.
>>
>> Assisted-by: LLM
>> Signed-off-by: Dongjiang Zhu <zhudongjiang@fygo.io>
>> ---
>>   fs/btrfs/disk-io.c |  7 +++-
>>   fs/btrfs/zoned.c   | 97 ++++++++++++++++++++++++++++++++++++++++++----
>>   fs/btrfs/zoned.h   |  7 +++-
>>   3 files changed, 101 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index e6bbb0b1b38c..cd1f46ec62f0 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -3713,7 +3713,12 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>>   
>>   	btrfs_free_zone_cache(fs_info);
>>   
>> -	btrfs_check_active_zone_reservation(fs_info);
>> +	ret = btrfs_restore_active_nondata_bgs(fs_info);
>> +	if (ret) {
>> +		btrfs_err(fs_info, "failed to restore active non-data block groups: %pe",
>> +			  ERR_PTR(ret));
>> +		goto fail_sysfs;
>> +	}
>>   
>>   	if (!sb_rdonly(sb) && fs_info->fs_devices->missing_devices &&
>>   	    !btrfs_check_rw_degradable(fs_info, NULL)) {
>> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>> index 0d964ada3ad4..ee7d518bec15 100644
>> --- a/fs/btrfs/zoned.c
>> +++ b/fs/btrfs/zoned.c
>> @@ -3056,22 +3056,77 @@ int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_info, bool do_fin
>>   	return 0;
>>   }
>>   
>> +static int finish_extra_active_nondata_bgs(struct btrfs_fs_info *fs_info)
>> +{
>> +	struct btrfs_block_group *block_group;
>> +	struct btrfs_block_group *next;
>> +	u64 tail_unusable;
>> +	int ret;
>> +
>> +	list_for_each_entry_safe(block_group, next, &fs_info->zone_active_bgs,
>> +				 active_bg_list) {
>> +		if (!(block_group->flags &
>> +		      (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_SYSTEM)))
>> +			continue;
>> +
>> +		if (block_group == fs_info->active_meta_bg ||
>> +		    block_group == fs_info->active_system_bg)
>> +			continue;
>> +
>> +		btrfs_get_block_group(block_group);
> 
> Why do we need to do this?
> 

I added the extra reference because do_zone_finish() removes the block
group from zone_active_bgs and drops the active-list reference before the
caller-side accounting.

However, the block group remains in the block-group cache and cannot be
removed concurrently in this mount-time context, so the extra reference
is not necessary.  Moving the tail accounting into do_zone_finish() also
removes all post-finish accesses here.  I will remove the get/put pair.

>> +		spin_lock(&block_group->lock);
>> +		tail_unusable = block_group->zone_capacity - block_group->alloc_offset;
>> +		spin_unlock(&block_group->lock);
>> +
>> +		ret = do_zone_finish(block_group, true);
>> +		if (!ret) {
>> +			struct btrfs_space_info *space_info = block_group->space_info;
>> +
>> +			/* Account for the unused tail abandoned by ZONE_FINISH. */
>> +			spin_lock(&space_info->lock);
>> +			spin_lock(&block_group->lock);
>> +			block_group->zone_unusable += tail_unusable;
>> +			btrfs_space_info_update_bytes_zone_unusable(space_info,
>> +								    tail_unusable);
>> +			spin_unlock(&block_group->lock);
>> +			spin_unlock(&space_info->lock);
>> +		}
> 
> So, basically, do_zone_finish() should have handled this...
> 

Yes.  The fully_written=false path already updates zone_unusable through
the temporary read-only transition, while the fully_written=true path
assumes that no allocatable tail remains.

Mount recovery intentionally finishes partially used block groups, so
that assumption no longer holds for this caller.  I will move the unused
tail accounting into do_zone_finish() and remove the caller-side
accounting.

Thanks,
Dongjiang Zhu

[PATCH v3 3/4] btrfs: zoned: remove obsolete non-data block group activation helper

[Dongjiang Zhu]

On an HC620 with max_active_zones=128, file writes occupied all 128
active-zone slots.  With four direct-I/O writers still running, starting
a balance with

  btrfs balance start -f -dusage=0 -musage=0 -susage=100 <mountpoint>

left the balance task blocked in D state.  SysRq-w reported:

  task:btrfs           state:D stack:0     pid:8145
  Call Trace:
   <TASK>
   __schedule+0x50e/0xfe0
   schedule+0x22/0xa0
   schedule_preempt_disabled+0x10/0x20
   __mutex_lock.constprop.0+0x380/0x920
   btrfs_inc_block_group_ro+0x6a/0x270 [btrfs]
   do_zone_finish+0x2b6/0x4c0 [btrfs]
   btrfs_zone_finish_one_bg+0x184/0x1f0 [btrfs]
   btrfs_zoned_activate_one_bg+0x189/0x1d0 [btrfs]
   reserve_chunk_space+0xdb/0x180 [btrfs]
   btrfs_inc_block_group_ro+0x252/0x270 [btrfs]
   btrfs_relocate_block_group+0xc2/0x2260 [btrfs]
   btrfs_relocate_chunk+0x39/0x1c0 [btrfs]
   btrfs_balance+0x911/0x1810 [btrfs]
   </TASK>

The blocked balance task also stalls transaction commit and data writers,
so the filesystem can no longer make write progress or unmount cleanly.

This happens because the outer btrfs_inc_block_group_ro() holds
ro_block_group_mutex while reserve_chunk_space() tries to activate a
system block group.  When no active-zone slot is available,
btrfs_zoned_activate_one_bg() starts finishing a partially written data
block group.

do_zone_finish() then enters btrfs_inc_block_group_ro() again.  The same
task waits for ro_block_group_mutex that it already holds and
self-deadlocks.

Remove the two allocation-time activation calls and their helper for two
reasons:

1. They were added for active_total_bytes accounting, which no longer
   exists.  Metadata and system block groups are now activated at write
   time, and both callers already handle no block group being activated.

2. Allocation-time activation bypasses the role pivot.  If A is the
   current system block group, activating B consumes another active-zone
   slot instead of finishing A.  If no slot is available, the helper
   tries to finish an unrelated data block group, creating the recursive
   path above.

With these activation paths removed, the first write to a sequential
metadata or system block group enters check_bg_is_active(), which finishes
the previous block group for that role before activating the new one.

Assisted-by: LLM
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Dongjiang Zhu <zhudongjiang@fygo.io>
---
 fs/btrfs/block-group.c | 38 +++++++++-----------------------
 fs/btrfs/zoned.c       | 50 ------------------------------------------
 fs/btrfs/zoned.h       |  8 -------
 3 files changed, 10 insertions(+), 86 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 830460a40e86..c58bac5cb134 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -3219,13 +3219,6 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
 
 	alloc_flags = btrfs_get_alloc_profile(fs_info, space_info->flags);
 	ret = btrfs_chunk_alloc(trans, space_info, alloc_flags, CHUNK_ALLOC_FORCE);
-	if (ret < 0)
-		goto out;
-	/*
-	 * We have allocated a new chunk. We also need to activate that chunk to
-	 * grant metadata tickets for zoned filesystem.
-	 */
-	ret = btrfs_zoned_activate_one_bg(space_info, true);
 	if (ret < 0)
 		goto out;
 
@@ -4537,29 +4530,18 @@ static void reserve_chunk_space(struct btrfs_trans_handle *trans,
 		if (IS_ERR(bg)) {
 			ret = PTR_ERR(bg);
 		} else {
-			int activate_ret;
-
 			/*
-			 * We have a new chunk. We also need to activate it for
-			 * zoned filesystem.
+			 * If we fail to add the chunk item here, we end
+			 * up trying again at phase 2 of chunk allocation,
+			 * at btrfs_create_pending_block_groups(). So
+			 * ignore any error here. An ENOSPC here could
+			 * happen, due to the cases described at
+			 * do_chunk_alloc() - the system block group we
+			 * just created was just turned into RO mode by a
+			 * scrub for example, or a running discard
+			 * temporarily removed its free space entries, etc.
 			 */
-			activate_ret = btrfs_zoned_activate_one_bg(info, true);
-			if (activate_ret < 0) {
-				ret = activate_ret;
-			} else {
-				/*
-				 * If we fail to add the chunk item here, we end
-				 * up trying again at phase 2 of chunk allocation,
-				 * at btrfs_create_pending_block_groups(). So
-				 * ignore any error here. An ENOSPC here could
-				 * happen, due to the cases described at
-				 * do_chunk_alloc() - the system block group we
-				 * just created was just turned into RO mode by a
-				 * scrub for example, or a running discard
-				 * temporarily removed its free space entries, etc.
-				 */
-				btrfs_chunk_alloc_add_chunk_item(trans, bg);
-			}
+			btrfs_chunk_alloc_add_chunk_item(trans, bg);
 		}
 	}
 
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index ee7d518bec15..63d59b015829 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -3006,56 +3006,6 @@ int btrfs_zone_finish_one_bg(struct btrfs_fs_info *fs_info)
 	return ret < 0 ? ret : 1;
 }
 
-int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_info, bool do_finish)
-{
-	struct btrfs_fs_info *fs_info = space_info->fs_info;
-	struct btrfs_block_group *bg;
-	int index;
-
-	if (!btrfs_is_zoned(fs_info) || (space_info->flags & BTRFS_BLOCK_GROUP_DATA))
-		return 0;
-
-	for (;;) {
-		int ret;
-		bool need_finish = false;
-
-		down_read(&space_info->groups_sem);
-		for (index = 0; index < BTRFS_NR_RAID_TYPES; index++) {
-			list_for_each_entry(bg, &space_info->block_groups[index],
-					    list) {
-				if (!spin_trylock(&bg->lock))
-					continue;
-				if (btrfs_zoned_bg_is_full(bg) ||
-				    test_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE,
-					     &bg->runtime_flags)) {
-					spin_unlock(&bg->lock);
-					continue;
-				}
-				spin_unlock(&bg->lock);
-
-				if (btrfs_zone_activate(bg)) {
-					up_read(&space_info->groups_sem);
-					return 1;
-				}
-
-				need_finish = true;
-			}
-		}
-		up_read(&space_info->groups_sem);
-
-		if (!do_finish || !need_finish)
-			break;
-
-		ret = btrfs_zone_finish_one_bg(fs_info);
-		if (ret == 0)
-			break;
-		if (ret < 0)
-			return ret;
-	}
-
-	return 0;
-}
-
 static int finish_extra_active_nondata_bgs(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_block_group *block_group;
diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h
index 934a9368ed15..dc7c453fc979 100644
--- a/fs/btrfs/zoned.h
+++ b/fs/btrfs/zoned.h
@@ -94,7 +94,6 @@ bool btrfs_zoned_should_reclaim(const struct btrfs_fs_info *fs_info);
 void btrfs_zoned_release_data_reloc_bg(struct btrfs_fs_info *fs_info, u64 logical,
 				       u64 length);
 int btrfs_zone_finish_one_bg(struct btrfs_fs_info *fs_info);
-int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_info, bool do_finish);
 int btrfs_restore_active_nondata_bgs(struct btrfs_fs_info *fs_info);
 int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num_bytes);
 void btrfs_show_zoned_stats(struct btrfs_fs_info *fs_info, struct seq_file *seq);
@@ -272,13 +271,6 @@ static inline int btrfs_zone_finish_one_bg(struct btrfs_fs_info *fs_info)
 	return 1;
 }
 
-static inline int btrfs_zoned_activate_one_bg(struct btrfs_space_info *space_info,
-					      bool do_finish)
-{
-	/* Consider all the block groups are active */
-	return 0;
-}
-
 static inline int btrfs_restore_active_nondata_bgs(struct btrfs_fs_info *fs_info)
 {
 	return 0;
-- 
2.39.5

[PATCH v3 4/4] btrfs: zoned: serialize zone finishing per block group

[Dongjiang Zhu]

On an HC620 with max_active_zones=128, buffered writes occupied all 128
active-zone slots.  A metadata-heavy create/delete workload followed by
sync then forced a metadata block group switch.

A kernel with additional zone finish tracing reported:

  zoned-debug: finish clear-runtime bg=140928614400 caller=btrfs_zone_finish_endio_workfn comm=kworker/u8:7
  zoned-debug: finish early-inactive bg=140928614400 caller=btrfs_check_meta_write_pointer comm=sync
  zoned-debug: pivot finish ret=0 old_bg=140928614400 new_bg=339839287296
  zoned-debug: activate fail slot bg=339839287296 type=metadata left=0 max_active=128 comm=sync
  zoned-debug: finish zone-mgmt bg=140928614400 ret=0 left=0 caller=btrfs_zone_finish_endio_workfn comm=kworker/u8:7
  BTRFS error (device sdb): error while writing out transaction: -EAGAIN
  BTRFS error (device sdb state A): Transaction 67 aborted (-EAGAIN)
  BTRFS info (device sdb state EA): forced readonly
  zoned-debug: clear-active dev=/dev/sdb pos=140928614400 left=1 max_active=128

The transaction was aborted before the old block group returned its
active-zone slot, forcing the filesystem read-only.

This happens because do_zone_finish() clears
BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE before issuing the zone finish operation.
This allows the following interleaving:

  Thread A, endio worker              Thread B, transaction writeback

  clear ZONE_IS_ACTIVE
  call_zone_finish(old)
                                        observe old as inactive
                                        return from do_zone_finish(old)
                                        activate(new)
                                        fail with left == 0
  clear the device active bit
  increment active_zones_left

Thread B treats the cleared active bit as a completed finish while thread
A is still performing the device operation.

Simply moving the active-bit clear after call_zone_finish() is not
sufficient because it does not serialize finishers.  Two callers can
still observe the block group as active and finish it concurrently.  For
a non-data block group, both increment reserved_active_zones, while
active_zones_left is incremented only once.  This breaks the invariant
that the two counters are restored together.

Therefore, add BLOCK_GROUP_FLAG_ZONE_FINISHING and keep ZONE_IS_ACTIVE set
until the zone finish operation and active-list cleanup are complete.

Concurrent finishers wait for ZONE_FINISHING to be cleared instead of
returning early.  Wake them after clearing ZONE_IS_ACTIVE and removing
the block group from the active list.  If a waiter acquired a temporary
read-only reference, release it after the wait.

Assisted-by: LLM
Signed-off-by: Dongjiang Zhu <zhudongjiang@fygo.io>
---
 fs/btrfs/block-group.h |  1 +
 fs/btrfs/zoned.c       | 29 ++++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
index 69d56864d4ba..7acd9bf9620d 100644
--- a/fs/btrfs/block-group.h
+++ b/fs/btrfs/block-group.h
@@ -95,6 +95,7 @@ enum btrfs_block_group_flags {
 	BLOCK_GROUP_FLAG_NEW,
 	BLOCK_GROUP_FLAG_FULLY_REMAPPED,
 	BLOCK_GROUP_FLAG_STRIPE_REMOVAL_PENDING,
+	BLOCK_GROUP_FLAG_ZONE_FINISHING,
 };
 
 enum btrfs_caching_type {
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 63d59b015829..bc09447caf10 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -2564,6 +2564,7 @@ static int do_zone_finish(struct btrfs_block_group *block_group, bool fully_writ
 	const bool is_metadata = (block_group->flags &
 			(BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_SYSTEM));
 	struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace;
+	bool bg_ro = false;
 	int ret = 0;
 	int i;
 
@@ -2573,6 +2574,11 @@ static int do_zone_finish(struct btrfs_block_group *block_group, bool fully_writ
 		return 0;
 	}
 
+	if (test_bit(BLOCK_GROUP_FLAG_ZONE_FINISHING, &block_group->runtime_flags)) {
+		spin_unlock(&block_group->lock);
+		goto wait_finish;
+	}
+
 	/* Check if we have unwritten allocated space */
 	if (is_metadata &&
 	    block_group->start + block_group->alloc_offset > block_group->meta_write_pointer) {
@@ -2597,6 +2603,7 @@ static int do_zone_finish(struct btrfs_block_group *block_group, bool fully_writ
 		ret = btrfs_inc_block_group_ro(block_group, false);
 		if (ret)
 			return ret;
+		bg_ro = true;
 
 		/* Ensure all writes in this block group finish */
 		btrfs_wait_block_group_reservations(block_group);
@@ -2619,6 +2626,12 @@ static int do_zone_finish(struct btrfs_block_group *block_group, bool fully_writ
 			return 0;
 		}
 
+		if (test_bit(BLOCK_GROUP_FLAG_ZONE_FINISHING,
+			     &block_group->runtime_flags)) {
+			spin_unlock(&block_group->lock);
+			goto wait_finish;
+		}
+
 		if (block_group->reserved ||
 		    test_bit(BLOCK_GROUP_FLAG_ZONED_DATA_RELOC,
 			     &block_group->runtime_flags)) {
@@ -2628,7 +2641,7 @@ static int do_zone_finish(struct btrfs_block_group *block_group, bool fully_writ
 		}
 	}
 
-	clear_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &block_group->runtime_flags);
+	set_bit(BLOCK_GROUP_FLAG_ZONE_FINISHING, &block_group->runtime_flags);
 	block_group->alloc_offset = block_group->zone_capacity;
 	if (block_group->flags & (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_SYSTEM))
 		block_group->meta_write_pointer = block_group->start +
@@ -2652,8 +2665,13 @@ static int do_zone_finish(struct btrfs_block_group *block_group, bool fully_writ
 		btrfs_dec_block_group_ro(block_group);
 
 	spin_lock(&fs_info->zone_active_bgs_lock);
+	spin_lock(&block_group->lock);
+	clear_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &block_group->runtime_flags);
 	ASSERT(!list_empty(&block_group->active_bg_list));
 	list_del_init(&block_group->active_bg_list);
+	clear_and_wake_up_bit(BLOCK_GROUP_FLAG_ZONE_FINISHING,
+			      &block_group->runtime_flags);
+	spin_unlock(&block_group->lock);
 	spin_unlock(&fs_info->zone_active_bgs_lock);
 
 	/* For active_bg_list */
@@ -2662,6 +2680,15 @@ static int do_zone_finish(struct btrfs_block_group *block_group, bool fully_writ
 	clear_and_wake_up_bit(BTRFS_FS_NEED_ZONE_FINISH, &fs_info->flags);
 
 	return ret;
+
+wait_finish:
+	wait_on_bit_io(&block_group->runtime_flags,
+		       BLOCK_GROUP_FLAG_ZONE_FINISHING,
+		       TASK_UNINTERRUPTIBLE);
+	if (bg_ro)
+		btrfs_dec_block_group_ro(block_group);
+
+	return 0;
 }
 
 int btrfs_zone_finish(struct btrfs_block_group *block_group)
-- 
2.39.5