Re: [RFC] btrfs: zoned: reset stale zones in unallocated device space
From: Марк Коренберг
Date: Tue Oct 27 2026 - 10:59:16 EST
Hi Johannes,
I rebased your patch onto current master and found a few problems in
review, so I reworked it. v2 is attached. Main issues:
1) Data loss during device replace: the replace target has no chunk
stripes until the replace finishes, so all data already copied to
it was reset, and the resumed replace never copies it again.
Trigger: start a replace, umount or remount,ro, then mount rw.
v2 skips the pass while a replace is ongoing.
2) Races: chunks can be allocated (data reloc bg, transaction kthread)
and removed (relocating repair after a read error) while the pass
runs, and the removal is not committed yet. v2 takes
reclaim_bgs_lock, commits, and holds chunk_mutex.
3) Used zones: v1 marked only the zone where each stripe starts. Today
stripes are exactly one zone long, but mount does not enforce that,
so a longer stripe would get its other zones reset. v2 checks
device->alloc_state instead, the allocator's own record of which
ranges belong to chunks, over the full stripe length. This also
drops the bitmap allocation that could fail the mount.
4) Zones that must not be reset: zones beyond total_bytes, read-only
devices, offline and read-only zones. v2 skips them.
5) v2 resets adjacent zones in one call and logs each reset range.
The commit message also now says why the existing code does not help:
btrfs_find_allocatable_zones() skips non-empty zones, so leaked zones
are never reused. And the typo in your Signed-off-by is fixed.
I used an AI assistant (Claude) for the review and for v2. v2 is build
tested and checkpatch clean, not runtime tested yet.
Please check that it still covers the case you hit, and tell me if you
prefer to fold it into your patch or keep it as a fix on top.
Thanks,
Mark
вт, 27 окт. 2026 г. в 15:27, Johannes Thumshirn <johannes.thumshirn@xxxxxxx>:
>
> On 10/27/26 9:36 AM, Марк Коренберг wrote:
> > Hi,
> >
> > I'd like to propose a feature for zoned btrfs. I'm not planning to
> > implement it myself, but I'd like to get feedback and, if the idea is
> > considered reasonable, have it tracked as a TODO / project idea.
> >
> > Background
> > ----------
> > I maintain a host-managed zoned block device implemented with ublk,
> > where each zone is backed by S3 objects. Storage cost is proportional
> > to the amount of data actually held in zones, so zones that contain
> > data but are not referenced by the filesystem are a real cost, not
> > just lost capacity.
> >
> > Problem
> > -------
> > With a long commit interval (e.g. commit=300) and heavy buffered
> > writes, btrfs may allocate new block groups and write a large amount
> > of data into their zones before the transaction commits. If the
> > filesystem then goes down uncleanly before the commit, this is fine
> > from a consistency point of view: nobody called fsync(), and after
> > the next mount the filesystem is consistent at the last committed
> > transaction.
> >
> > For a virtual device like mine, "goes down uncleanly" is more likely
> > the ublk server crashing or losing connectivity to S3 than an actual
> > power loss, but the effect on btrfs is the same: the running
> > transaction never commits.
> >
> > The device extents of those new block groups were never persisted, so
> > after the next mount the corresponding zones belong to unallocated
> > device space from btrfs' point of view, while on the device they are
> > still non-empty (write pointer > zone start) and hold data.
>
> I have a patch lying around doing this in an old stale branch. I've
> attached it, but not tried rebasing it onto latest for-next. If you're
> brave enough to test it I'd be happy.
>
>
> > As far as I can tell, btrfs currently handles this lazily: such zones
> > are reset only when the allocator picks that range for a new chunk
> > (btrfs_ensure_empty_zones()). Until then they hold stale data
> > indefinitely. On SMR HDDs this does not matter much, but on ZNS SSDs
> > (where a reset helps the device's internal GC) and especially on
> > virtual/cloud-backed zoned devices it does.
> >
> > (Stale data past the committed alloc_offset inside *existing* block
> > groups is accounted as zone_unusable and can only be reclaimed via
> > relocation; this proposal does not try to address that case.)
> >
> > Current workaround
> > ------------------
> > The only safe way I see today is offline: unmount, walk the dev tree
> > to collect allocated dev extents, and issue BLKRESETZONE for every
> > non-empty sequential zone not covered by a dev extent. Doing this on
> > a mounted filesystem from userspace is unsafe, because it races with
> > chunk allocation (a dev extent may be allocated in the running
> > transaction but not yet visible in the committed dev tree).
> >
> > Proposal
> > --------
> > Let the kernel do it, since it can serialize against chunk allocation
> > (chunk_mutex / device->alloc_state). The operation itself: reset
> > every sequential zone that is fully contained in unallocated device
> > space and whose write pointer is not at the zone start.
> >
> > Possible interfaces, in order of my preference:
> >
> > 1. Automatic background cleanup, e.g. once after mount (after the
> > dev extents are loaded) and/or as part of the existing
> > reclaim/discard workers, possibly behind a mount option or sysfs
> > knob. This requires no administrator action and covers the
> > problem completely.
> >
> > 2. FITRIM: on zoned filesystems, apply the reset to unallocated
> > device space as part of fstrim. Many distributions already run
> > fstrim periodically (fstrim.timer), so this would also work
> > without extra configuration in most setups, and it fits the
> > "release unused space to the device" semantics.
> >
> > 3. A dedicated trigger: a new ioctl (exposed as a btrfs-progs
> > subcommand) or a sysfs knob under /sys/fs/btrfs/<UUID>/ that
> > starts the cleanup on demand. Least convenient, but still much
> > better than the current offline-only workaround.
> >
> > Risk
> > ----
> > With corrupted metadata (a dev extent missing from the dev tree),
> > this could reset a zone that actually holds live data. But the
> > allocator already makes exactly the same assumption today when it
> > resets non-empty zones before reusing them, so this does not add a
> > new class of risk; it only performs the reset earlier. If automatic
> > cleanup is considered too aggressive, options 2 and 3 are explicit
> > and opt-in.
> >
> > Questions
> > ---------
> > - Does this sound like something you would accept upstream?
> > - Which interface would you prefer?
> > - Am I missing a reason why the reset is intentionally deferred to
> > allocation time?
> >
> > If the idea is acceptable, I'd appreciate it being added to the btrfs
> > TODO list so that someone interested could pick it up.
> >
> > Thanks,
> > Korenberg Mark
>
--
Segmentation fault
From cc092992fe050b4edcde691829435a1e09b4fb47 Mon Sep 17 00:00:00 2001
From: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
Date: Tue, 8 Sep 2026 11:42:05 +0200
Subject: [PATCH v2] btrfs: zoned: reset leaked zones on read-write mount
If a transaction commit is aborted after the zones of a new block group
were written but before the superblock was, the chunk item and the dev
extent are never persisted and the zones leak. The chunk allocator skips
non-empty zones, so they are never reused, open ones keep counting
against the active zone limit, and btrfs check reports them as
unreferenced.
On a read-write mount, reset every non-empty sequential zone that is
neither a superblock log zone nor allocated to a chunk. Commit the
running transaction under reclaim_bgs_lock first, so that a chunk
removal is not mistaken for a leak, hold chunk_mutex so that no chunk is
allocated meanwhile, and skip it all while a device replace is ongoing,
as no chunk references the replace target yet.
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
---
Notes:
Changes in v2:
- Skip while a device replace is ongoing; v1 reset the data already
copied to the replace target.
- Hold chunk_mutex; v1 raced with chunk allocation in open_ctree().
- Hold reclaim_bgs_lock and commit first; v1 could reset the zones of a
chunk removed by a relocating repair before the removal was committed.
- Check coverage in device->alloc_state: covers whole stripes, no
bitmap allocation, no NULL physical_map case.
- Walk alloc_list (writeable devices) under chunk_mutex instead of the
device list under device_list_mutex, only up to total_bytes.
- Skip offline and read-only zones, they cannot be reset.
- Reset adjacent zones in one call and log each reset range.
Reviewed with the help of an AI assistant (Claude), which also drafted
the v2 changes. Build tested only.
fs/btrfs/disk-io.c | 2 +
fs/btrfs/zoned.c | 159 +++++++++++++++++++++++++++++++++++++++++++++
fs/btrfs/zoned.h | 3 +
3 files changed, 164 insertions(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c45cda4290d5..f3e15c575e8a 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3059,6 +3059,8 @@ int btrfs_start_pre_rw_mount(struct btrfs_fs_info *fs_info)
int ret;
bool rebuild_free_space_tree = false;
+ btrfs_reset_unreferenced_zones(fs_info);
+
if (btrfs_test_opt(fs_info, CLEAR_CACHE) &&
btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) {
if (btrfs_fs_incompat(fs_info, EXTENT_TREE_V2))
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 1cda88332284..8b0331bc8fab 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -3226,6 +3226,165 @@ int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num
return 0;
}
+/*
+ * A zone is leaked if it is a non-empty sequential zone that is neither a
+ * superblock log zone nor part of any stripe of any chunk. Chunk stripes are
+ * looked up in the device's alloc_state, which covers every chunk in the
+ * mapping tree over the whole stripe length.
+ */
+static bool is_leaked_zone(struct btrfs_device *device, u32 zno)
+{
+ struct btrfs_zoned_device_info *zinfo = device->zone_info;
+ u64 start = (u64)zno << zinfo->zone_size_shift;
+ struct blk_zone zone;
+ unsigned int nofs_flags;
+ int ret;
+
+ if (!btrfs_dev_is_sequential(device, start))
+ return false;
+
+ if (btrfs_dev_is_empty_zone(device, start))
+ return false;
+
+ if (btrfs_test_range_bit_exists(&device->alloc_state, start,
+ start + zinfo->zone_size - 1,
+ CHUNK_ALLOCATED))
+ return false;
+
+ for (int mirror = 0; mirror < BTRFS_SUPER_MIRROR_MAX; mirror++) {
+ u32 sb_zno = sb_zone_number(zinfo->zone_size_shift, mirror);
+
+ if (sb_zno + BTRFS_NR_SB_LOG_ZONES > zinfo->nr_zones)
+ continue;
+ if (zno >= sb_zno && zno < sb_zno + BTRFS_NR_SB_LOG_ZONES)
+ return false;
+ }
+
+ /*
+ * Offline and read-only zones are never marked empty either, but they
+ * cannot be reset. Their condition is not cached, so ask the device.
+ * We hold chunk_mutex here, so don't recurse into the filesystem.
+ */
+ nofs_flags = memalloc_nofs_save();
+ ret = btrfs_get_dev_zone(device, start, &zone);
+ memalloc_nofs_restore(nofs_flags);
+ if (ret)
+ return false;
+
+ if (zone.cond == BLK_ZONE_COND_OFFLINE)
+ return false;
+
+ if (zone.cond == BLK_ZONE_COND_READONLY)
+ return false;
+
+ return true;
+}
+
+void btrfs_reset_unreferenced_zones(struct btrfs_fs_info *fs_info)
+{
+ struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
+ struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace;
+ struct btrfs_device *device;
+ bool replace_ongoing;
+ int ret;
+
+ if (!btrfs_is_zoned(fs_info))
+ return;
+
+ /*
+ * No chunk stripe points to the target of an interrupted device replace
+ * until the replace finishes, so the data already copied to it would
+ * look leaked. The replace resumes from the last committed cursor and
+ * would never copy that data again.
+ *
+ * The target is not on alloc_list, which is walked below, so strictly
+ * speaking it would be enough to skip only the device that has
+ * BTRFS_DEV_STATE_REPLACE_TGT set. Still, do not reset anything at all
+ * while a replace is ongoing, the leaked zones can be reclaimed on the
+ * next mount after the replace has finished or was canceled.
+ *
+ * Do not hold the rwsem across the pass, the replace code takes it
+ * after chunk_mutex. A replace cannot be started while we are in the
+ * (re)mount path.
+ */
+ down_read(&dev_replace->rwsem);
+ replace_ongoing = btrfs_dev_replace_is_ongoing(dev_replace);
+ up_read(&dev_replace->rwsem);
+ if (replace_ongoing) {
+ btrfs_info(fs_info,
+ "zoned: device replace is ongoing, not resetting leaked zones");
+ return;
+ }
+
+ /*
+ * Chunks are only removed under reclaim_bgs_lock, and their zones must
+ * not be reset before the removal is committed. A relocating repair,
+ * started by a read error during mount, may already have removed one in
+ * the running transaction, so commit it before looking for leaked zones.
+ */
+ mutex_lock(&fs_info->reclaim_bgs_lock);
+ ret = btrfs_commit_current_transaction(fs_info->tree_root);
+ if (ret) {
+ btrfs_warn(fs_info,
+ "zoned: failed to commit transaction, not resetting leaked zones: %pe",
+ ERR_PTR(ret));
+ goto out;
+ }
+
+ /*
+ * Hold chunk_mutex, which also protects alloc_list, so that no chunk can
+ * be allocated between checking a zone and resetting it.
+ */
+ mutex_lock(&fs_info->chunk_mutex);
+ list_for_each_entry(device, &fs_devices->alloc_list, dev_alloc_list) {
+ struct btrfs_zoned_device_info *zinfo = device->zone_info;
+ u32 nr_zones;
+
+ if (!device->bdev || !zinfo || !bdev_is_zoned(device->bdev))
+ continue;
+
+ /* Nothing is ever allocated beyond total_bytes, leave it alone. */
+ nr_zones = min_t(u64, zinfo->nr_zones,
+ device->total_bytes >> zinfo->zone_size_shift);
+
+ for (u32 zno = 0; zno < nr_zones; zno++) {
+ u32 first = zno;
+ u64 reset_bytes;
+
+ cond_resched();
+
+ while (zno < nr_zones && is_leaked_zone(device, zno))
+ zno++;
+ if (zno == first)
+ continue;
+
+ ret = btrfs_reset_device_zone(device,
+ (u64)first << zinfo->zone_size_shift,
+ (u64)(zno - first) << zinfo->zone_size_shift,
+ &reset_bytes);
+ /*
+ * On failure the run could be retried zone by zone, so that
+ * a single zone failing to reset does not keep the other
+ * zones of the run from being reclaimed. A failed reset is
+ * likely slow already, so just log it and move on, the run
+ * is retried on the next mount.
+ */
+ if (ret)
+ btrfs_warn(fs_info,
+ "zoned: failed to reset leaked zones %u-%u on device %s (devid %llu): %pe",
+ first, zno - 1, btrfs_dev_name(device),
+ device->devid, ERR_PTR(ret));
+ else
+ btrfs_info(fs_info,
+ "zoned: reset leaked zones %u-%u on device %s (devid %llu)",
+ first, zno - 1, btrfs_dev_name(device), device->devid);
+ }
+ }
+ mutex_unlock(&fs_info->chunk_mutex);
+out:
+ mutex_unlock(&fs_info->reclaim_bgs_lock);
+}
+
void btrfs_show_zoned_stats(struct btrfs_fs_info *fs_info, struct seq_file *seq)
{
struct btrfs_block_group *bg;
diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h
index 8e21a836f858..76b40e81ca32 100644
--- a/fs/btrfs/zoned.h
+++ b/fs/btrfs/zoned.h
@@ -97,6 +97,7 @@ 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_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num_bytes);
+void btrfs_reset_unreferenced_zones(struct btrfs_fs_info *fs_info);
void btrfs_show_zoned_stats(struct btrfs_fs_info *fs_info, struct seq_file *seq);
#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
@@ -287,6 +288,8 @@ static inline int btrfs_reset_unused_block_groups(struct btrfs_space_info *space
return 0;
}
+static inline void btrfs_reset_unreferenced_zones(struct btrfs_fs_info *fs_info) { }
+
static inline int btrfs_show_zoned_stats(struct btrfs_fs_info *fs_info, struct seq_file *seq)
{
return 0;
base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
--
2.56.0