Re: [PATCH 1/4] btrfs: zoned: flush active metadata block group at btree_writepages() start

On Wed, Jul 22, 2026 at 01:30:13PM +0200, Johannes Thumshirn wrote:
> On a zoned filesystem btree_writepages() writes metadata in ascending

wording nit: isn't this true on non-zoned too? Maybe something like:
btree_writepages() writes metadata in ascending logical order. On a
zoned filsystem, which keeps a single .... <something terrible happens>

> logical address order and keeps a single active metadata/system block group,
> pivoting it via btrfs_check_meta_write_pointer() -> check_bg_is_active() as
> writeback moves between block groups.
> 
> If the active block group is at a higher address than another block group
> that also has dirty metadata, the walk reaches the lower one first and the
> pivot must finish the active block group. It cannot finish one that still has
> unsent IO, and refuses to wait for it during WB_SYNC_ALL commit writeback, so

Can you add some more detail on why it refuses? Or why the pivot can't
be the one to do these submissions? I think that would help fully
motivate this change.

> btrfs_check_meta_write_pointer() returns -EAGAIN and the transaction is
> aborted, forcing the filesystem read-only. This happens intermittently under
> metadata-heavy relocation (e.g. fstests btrfs/187).
> 
> Flush the active metadata and system block groups at the start of
> btree_writepages(), under the fs_info->zoned_meta_io_lock it already holds, so
> they have no unsent IO and the later pivot can finish them.
> 
> Fixes: 13bb483d32ab ("btrfs: zoned: activate metadata block group on write time")
> Assisted-by: LLM (debugging, commit message)
> Signed-off-by: Johannes Thumshirn <[email protected]>
> ---
>  fs/btrfs/extent_io.c | 62 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 97bd18d515af..bac3edabe7c9 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2383,6 +2383,52 @@ void btrfs_btree_wait_writeback_range(struct btrfs_fs_info *fs_info, u64 start,
>  	}
>  }
>  
> +/* Write out the dirty metadata extent buffers of a single block group. */

This comment is quite generic, and not zoned specific, but the function
name is much more zoned focused (and we assert zoned)

I am also not a huge fan of this code mostly duplicating the loop in
btree_writepages while just leaving that spot open coded. Is that an
intentional decision because of important distinctions between the two?
Is there a variant we could lift to "write a range" and make
flush_active_meta_bg() and btree_writepages share it? (like the while
loop basically? or maybe the per-eb bit?)

note the missing call to btrfs_schedule_zone_finish_bg() I noted inline
as a concrete reason it's helpful to not duplicate.

> +static void flush_active_meta_bg(struct address_space *mapping,
> +				 struct writeback_control *wbc,
> +				 struct btrfs_eb_write_context *ctx,
> +				 struct btrfs_block_group *bg)
> +{
> +	struct btrfs_fs_info *fs_info = inode_to_fs_info(mapping->host);
> +	unsigned long index = bg->start >> fs_info->nodesize_bits;
> +	unsigned long end = (btrfs_block_group_end(bg) - 1) >> fs_info->nodesize_bits;
> +	struct eb_batch batch;
> +	unsigned int nr_ebs;
> +
> +	ASSERT(btrfs_is_zoned(fs_info));
> +	lockdep_assert_held(&fs_info->zoned_meta_io_lock);
> +
> +	eb_batch_init(&batch);
> +	while (index <= end &&
> +	       (nr_ebs = buffer_tree_get_ebs_tag(fs_info, &index, end,
> +						 PAGECACHE_TAG_DIRTY, &batch))) {
> +		struct extent_buffer *eb;
> +
> +		while ((eb = eb_batch_next(&batch)) != NULL) {
> +			ctx->eb = eb;
> +
> +			/*
> +			 * Best effort: if the eb is not writable at the write
> +			 * pointer (e.g. a hole), stop flushing this bg and let
> +			 * the main walk deal with it.
> +			 */
> +			if (btrfs_check_meta_write_pointer(eb->fs_info, ctx)) {
> +				eb_batch_release(&batch);
> +				return;
> +			}
> +
> +			if (!lock_extent_buffer_for_io(eb, wbc))
> +				continue;
> +
> +			if (ctx->zoned_bg)

the btree_writepages code also calls btrfs_schedule_zone_finish_bg()
here, is that intentionally omitted?

> +				ctx->zoned_bg->meta_write_pointer += eb->len;
> +			write_one_eb(eb, wbc);
> +		}
> +		eb_batch_release(&batch);
> +		cond_resched();
> +	}
> +}
> +
>  int btree_writepages(struct address_space *mapping, struct writeback_control *wbc)
>  {
>  	struct btrfs_eb_write_context ctx = { .wbc = wbc };
> @@ -2418,6 +2464,22 @@ int btree_writepages(struct address_space *mapping, struct writeback_control *wb
>  	else
>  		tag = PAGECACHE_TAG_DIRTY;
>  	btrfs_zoned_meta_io_lock(fs_info);
> +
> +	/*
> +	 * On a zoned filesystem, flush the currently active metadata/system
> +	 * block group(s) first, under this same lock, so the ascending-address
> +	 * walk below can pivot the active block group instead of aborting the
> +	 * commit with -EAGAIN.
> +	 */
> +	if (btrfs_is_zoned(fs_info) && wbc->sync_mode == WB_SYNC_ALL &&
> +	    !wbc->for_sync) {
> +		if (fs_info->active_meta_bg)
> +			flush_active_meta_bg(mapping, wbc, &ctx,
> +					     fs_info->active_meta_bg);
> +		if (fs_info->active_system_bg)
> +			flush_active_meta_bg(mapping, wbc, &ctx,
> +					     fs_info->active_system_bg);
> +	}

I think this placement makes sense, but I also wouldn't mind just having
a more generic "eb ordering" concept that zoned could use to plug the
active ebs to the front of the list then fallback to the normal order,
or something. Not a deal breaker for me, just food for thought.

I think fewer "tricks" for zoned writeback and more "it falls out
correctly from a generic form" would make it easier to maintain in the
long run. Obviously there are real, important differences so this kind
of thing is never free...

Thanks,
Boris

>  retry:
>  	if (wbc->sync_mode == WB_SYNC_ALL)
>  		buffer_tree_tag_for_writeback(fs_info, index, end);
> -- 
> 2.54.0
>