Re: [PATCH v2 4/4] btrfs: zoned: don't clobber the extent buffer when zeroing it out

On Thu, Jul 23, 2026 at 05:54:28PM +0200, Johannes Thumshirn wrote:
> On a zoned filesystem a freed-but-still-dirty tree block is written out
> as zeros (EXTENT_BUFFER_ZONED_ZEROOUT) only to keep the zone write
> pointer advancing. btree_csum_one_bio() implemented this by memzeroing
> the extent buffer's own folios before submission.
> 
> That destroys the in-memory buffer while it may still be referenced. In
> particular btrfs_free_tree_block() can run on it afterwards and reads
> the header to add a delayed reference; once the header has been zeroed
> it frees bytenr 0 and corrupts the extent tree (the
> btrfs_header_bytenr(buf) != 0 ASSERT in btrfs_free_tree_block(), or an
> "unable to find ref" abort). It is flaky and reproduces under fsstress,
> e.g. generic/461 and generic/013.
> 
> Write the zeros to disk from the shared zero page instead and leave the
> extent buffer content untouched, so any later reference - including the
> delayed reference from btrfs_free_tree_block() - still sees a valid
> header. end_bbio_meta_write() now clears writeback on the buffer's own
> folios, as the bio no longer carries them.
> 
> Fixes: aa6313e6ff2b ("btrfs: zoned: don't clear dirty flag of extent buffer")
> Assisted-by: LLM (debugging, commit message)

One question inline, but
Reviewed-by: Boris Burkov <[email protected]>

> Signed-off-by: Johannes Thumshirn <[email protected]>
> ---
>  fs/btrfs/disk-io.c   | 13 +++++++------
>  fs/btrfs/extent_io.c | 31 ++++++++++++++++++++++++-------
>  2 files changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 01d01e10d000..a1277c8f2871 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -271,14 +271,15 @@ int btree_csum_one_bio(struct btrfs_bio *bbio)
>  		return -EIO;
>  
>  	/*
> -	 * If an extent_buffer is marked as EXTENT_BUFFER_ZONED_ZEROOUT, don't
> -	 * checksum it but zero-out its content. This is done to preserve
> -	 * ordering of I/O without unnecessarily writing out data.
> +	 * An extent_buffer marked EXTENT_BUFFER_ZONED_ZEROOUT is written out as
> +	 * zeros to preserve ordering of I/O without persisting the now
> +	 * unnecessary block. The bio is fed from the shared zero page (see
> +	 * write_one_eb()), so there is nothing to checksum here. Crucially, the
> +	 * buffer's own content is left intact: it may still be referenced, e.g.
> +	 * btrfs_free_tree_block() reads its header to add a delayed reference.
>  	 */
> -	if (test_bit(EXTENT_BUFFER_ZONED_ZEROOUT, &eb->bflags)) {
> -		memzero_extent_buffer(eb, 0, eb->len);
> +	if (test_bit(EXTENT_BUFFER_ZONED_ZEROOUT, &eb->bflags))
>  		return 0;
> -	}
>  
>  	if (WARN_ON_ONCE(found_start != eb->start))
>  		return -EIO;
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index fa473476b7db..e88381a40600 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2266,14 +2266,17 @@ static struct extent_buffer *find_extent_buffer_nolock(
>  static void end_bbio_meta_write(struct btrfs_bio *bbio)
>  {
>  	struct extent_buffer *eb = bbio->private;
> -	struct folio_iter fi;
>  
>  	if (bbio->bio.bi_status != BLK_STS_OK)
>  		set_btree_ioerr(eb);
>  
> -	bio_for_each_folio_all(fi, &bbio->bio) {
> -		btrfs_meta_folio_clear_writeback(fi.folio, eb);
> -	}
> +	/*
> +	 * Clear writeback on the buffer's own folios. The bio may carry the
> +	 * shared zero page instead (EXTENT_BUFFER_ZONED_ZEROOUT), so iterate
> +	 * the extent buffer folios rather than the bio folios.
> +	 */
> +	for (int i = 0; i < num_extent_folios(eb); i++)
> +		btrfs_meta_folio_clear_writeback(eb->folios[i], eb);
>  
>  	buffer_tree_clear_mark(eb, PAGECACHE_TAG_WRITEBACK);
>  	clear_and_wake_up_bit(EXTENT_BUFFER_WRITEBACK, &eb->bflags);
> @@ -2314,7 +2317,8 @@ static noinline_for_stack void write_one_eb(struct extent_buffer *eb,
>  	struct btrfs_fs_info *fs_info = eb->fs_info;
>  	struct btrfs_bio *bbio;
>  
> -	prepare_eb_write(eb);
> +	if (!test_bit(EXTENT_BUFFER_ZONED_ZEROOUT, &eb->bflags))
> +		prepare_eb_write(eb);
>  
>  	bbio = btrfs_bio_alloc(INLINE_EXTENT_BUFFER_PAGES,
>  			       REQ_OP_WRITE | REQ_META | wbc_to_write_flags(wbc),
> @@ -2334,8 +2338,21 @@ static noinline_for_stack void write_one_eb(struct extent_buffer *eb,
>  		btrfs_meta_folio_set_writeback(folio, eb);
>  		if (!folio_test_dirty(folio))
>  			wbc->nr_to_write -= folio_nr_pages(folio);
> -		bio_add_folio_nofail(&bbio->bio, folio, range_len,
> -				     offset_in_folio(folio, range_start));
> +		if (test_bit(EXTENT_BUFFER_ZONED_ZEROOUT, &eb->bflags)) {
> +			u32 off = 0;
> +
> +			while (off < range_len) {
> +				u32 add = min_t(u32, PAGE_SIZE, range_len - off);
> +
> +				bio_add_folio_nofail(&bbio->bio,
> +						     page_folio(ZERO_PAGE(0)),
> +						     add, 0);

In your opinion would it be better to hang the zero_page off eb->folios[]?

Gets rid of this special case but might do something surprising /
unexpected (or just be a dumb idea and not work at all, lol)

> +				off += add;
> +			}
> +		} else {
> +			bio_add_folio_nofail(&bbio->bio, folio, range_len,
> +					     offset_in_folio(folio, range_start));
> +		}
>  		wbc_account_cgroup_owner(wbc, folio, range_len);
>  		folio_unlock(folio);
>  	}
> -- 
> 2.55.0
>