[PATCH v3 0/5] btrfs: zoned: don't zone append to conventional zone

[PATCH v3 0/5] btrfs: zoned: don't zone append to conventional zone

[Johannes Thumshirn]

When in a zoned mirrored RAID setup a block-group is backed by a
conventional and a sequential write required zone, btrfs will write the
data using REQ_OP_ZONE_APPEND. As this is illegal an ASSERT() in
btrfs_submit_dev_bio() will catch it beforehand.

Fix it by only setting REQ_OP_ZONE_APPEND btrfs_submit_dev_bio() if the
actual zone is a sequential write required zone.

To avoid multiple block-group lookups, cache the ability to use zone
append in btrfs_bio.

Afterwards convert the sprinkled booleans in btrfs_bio into a 'flags'
member and set them accordingly. This is deliberately done after the fix
to ease potential backporting.

Changes to v2:
- Rename to can_use_append
- Add patches to move booleans into new flags member

Johannes Thumshirn (5):
  btrfs: zoned: don't zone append to conventional zone
  btrfs: move btrfs_bio::csum_search_commit_root into flags
  btrfs: move btrfs_bio::is_scrub into flags
  btrfs: move btrfs_bio::async_csum into flags
  btrfs: move btrfs_bio::can_use_append into flags

 fs/btrfs/bio.c         | 32 ++++++++++++++++++--------------
 fs/btrfs/bio.h         | 24 +++++++++++++-----------
 fs/btrfs/compression.c |  4 +++-
 fs/btrfs/extent_io.c   |  7 ++++---
 fs/btrfs/file-item.c   |  8 ++++----
 fs/btrfs/scrub.c       |  2 +-
 6 files changed, 43 insertions(+), 34 deletions(-)

-- 
2.52.0

[PATCH v3 1/5] btrfs: zoned: don't zone append to conventional zone

[Johannes Thumshirn]

In case of a zoned RAID, it can happen that a data write is targeting a
sequential write required zone and a conventional zone. In this case the
bio will be marked as REQ_OP_ZONE_APPEND but for the conventional zone,
this needs to be REQ_OP_WRITE.

The setting of REQ_OP_ZONE_APPEND is deferred to the last possible time in
btrfs_submit_dev_bio(), but the decision if we can use zone append is
cached in btrfs_bio.

Cc: Naohiro Aota <naohiro.aota@wdc.com>
Cc: Christoph Hellwig <hch@lst.de>
Fixes: e9b9b911e03c ("btrfs: add raid stripe tree to features enabled with debug config")
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/bio.c | 20 ++++++++++----------
 fs/btrfs/bio.h |  3 +++
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 4a7bef895b97..33149f07e62d 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -480,6 +480,8 @@ static void btrfs_clone_write_end_io(struct bio *bio)
 
 static void btrfs_submit_dev_bio(struct btrfs_device *dev, struct bio *bio)
 {
+	u64 physical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
+
 	if (!dev || !dev->bdev ||
 	    test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state) ||
 	    (btrfs_op(bio) == BTRFS_MAP_WRITE &&
@@ -494,12 +496,14 @@ static void btrfs_submit_dev_bio(struct btrfs_device *dev, struct bio *bio)
 	 * For zone append writing, bi_sector must point the beginning of the
 	 * zone
 	 */
-	if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
-		u64 physical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
+	if (btrfs_bio(bio)->can_use_append &&
+	    btrfs_dev_is_sequential(dev, physical)) {
 		u64 zone_start = round_down(physical, dev->fs_info->zone_size);
 
 		ASSERT(btrfs_dev_is_sequential(dev, physical));
 		bio->bi_iter.bi_sector = zone_start >> SECTOR_SHIFT;
+		bio->bi_opf &= ~REQ_OP_WRITE;
+		bio->bi_opf |= REQ_OP_ZONE_APPEND;
 	}
 	btrfs_debug(dev->fs_info,
 	"%s: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u",
@@ -747,7 +751,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 	u64 logical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
 	u64 length = bio->bi_iter.bi_size;
 	u64 map_length = length;
-	bool use_append = btrfs_use_zone_append(bbio);
 	struct btrfs_io_context *bioc = NULL;
 	struct btrfs_io_stripe smap;
 	blk_status_t status;
@@ -775,8 +778,10 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 	if (bio_op(bio) == REQ_OP_WRITE && is_data_bbio(bbio))
 		bbio->orig_logical = logical;
 
+	bbio->can_use_append = btrfs_use_zone_append(bbio);
+
 	map_length = min(map_length, length);
-	if (use_append)
+	if (bbio->can_use_append)
 		map_length = btrfs_append_map_length(bbio, map_length);
 
 	if (map_length < length) {
@@ -805,11 +810,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 	}
 
 	if (btrfs_op(bio) == BTRFS_MAP_WRITE) {
-		if (use_append) {
-			bio->bi_opf &= ~REQ_OP_WRITE;
-			bio->bi_opf |= REQ_OP_ZONE_APPEND;
-		}
-
 		if (is_data_bbio(bbio) && bioc && bioc->use_rst) {
 			/*
 			 * No locking for the list update, as we only add to
@@ -836,7 +836,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 			status = errno_to_blk_status(ret);
 			if (status)
 				goto fail;
-		} else if (use_append ||
+		} else if (bbio->can_use_append ||
 			   (btrfs_is_zoned(fs_info) && inode &&
 			    inode->flags & BTRFS_INODE_NODATASUM)) {
 			ret = btrfs_alloc_dummy_sum(bbio);
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index 56279b7f3b2a..d6da9ed08bfa 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -92,6 +92,9 @@ struct btrfs_bio {
 	/* Whether the csum generation for data write is async. */
 	bool async_csum;
 
+	/* Whether the bio is written using zone append. */
+	bool can_use_append;
+
 	/*
 	 * This member must come last, bio_alloc_bioset will allocate enough
 	 * bytes for entire btrfs_bio but relies on bio being last.
-- 
2.52.0

[PATCH v3 2/5] btrfs: move btrfs_bio::csum_search_commit_root into flags

[Johannes Thumshirn]

Remove struct btrfs_bio's csum_search_commit_root field and move it as a
flag into the newly introduced flags member of struct btrfs_bio.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/bio.c         | 5 ++++-
 fs/btrfs/bio.h         | 6 ++++--
 fs/btrfs/compression.c | 4 +++-
 fs/btrfs/extent_io.c   | 7 ++++---
 fs/btrfs/file-item.c   | 4 ++--
 5 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 33149f07e62d..2a9ab1275b7d 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -97,7 +97,10 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
 		bbio->orig_logical = orig_bbio->orig_logical;
 		orig_bbio->orig_logical += map_length;
 	}
-	bbio->csum_search_commit_root = orig_bbio->csum_search_commit_root;
+
+	if (orig_bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT)
+		bbio->flags |= BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT;
+
 	atomic_inc(&orig_bbio->pending_ios);
 	return bbio;
 }
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index d6da9ed08bfa..18d7d441c1ec 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -20,6 +20,9 @@ struct btrfs_inode;
 
 typedef void (*btrfs_bio_end_io_t)(struct btrfs_bio *bbio);
 
+/* Use the commit root to look up csums (data read bio only). */
+#define BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT	(1 << 0)
+
 /*
  * Highlevel btrfs I/O structure.  It is allocated by btrfs_bio_alloc and
  * passed to btrfs_submit_bbio() for mapping to the physical devices.
@@ -80,8 +83,7 @@ struct btrfs_bio {
 	/* Save the first error status of split bio. */
 	blk_status_t status;
 
-	/* Use the commit root to look up csums (data read bio only). */
-	bool csum_search_commit_root;
+	unsigned int flags;
 
 	/*
 	 * Since scrub will reuse btree inode, we need this flag to distinguish
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 913fddce356a..1823262fabbd 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -603,7 +603,9 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio)
 	cb->compressed_len = compressed_len;
 	cb->compress_type = btrfs_extent_map_compression(em);
 	cb->orig_bbio = bbio;
-	cb->bbio.csum_search_commit_root = bbio->csum_search_commit_root;
+
+	if (bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT)
+		cb->bbio.flags |= BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT;
 
 	btrfs_free_extent_map(em);
 
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 2d32dfc34ae3..d321f6897388 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -166,9 +166,10 @@ static void bio_set_csum_search_commit_root(struct btrfs_bio_ctrl *bio_ctrl)
 	if (!(btrfs_op(&bbio->bio) == BTRFS_MAP_READ && is_data_inode(bbio->inode)))
 		return;
 
-	bio_ctrl->bbio->csum_search_commit_root =
-		(bio_ctrl->generation &&
-		 bio_ctrl->generation < btrfs_get_fs_generation(bbio->inode->root->fs_info));
+
+	if (bio_ctrl->generation &&
+	    bio_ctrl->generation < btrfs_get_fs_generation(bbio->inode->root->fs_info))
+		bbio->flags |= BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT;
 }
 
 static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 14e5257f0f04..823c063bb4b7 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -422,7 +422,7 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 	 * while we are not holding the commit_root_sem, and we get csums
 	 * from across transactions.
 	 */
-	if (bbio->csum_search_commit_root) {
+	if (bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT) {
 		path->search_commit_root = true;
 		path->skip_locking = true;
 		down_read(&fs_info->commit_root_sem);
@@ -473,7 +473,7 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 		bio_offset += count * sectorsize;
 	}
 
-	if (bbio->csum_search_commit_root)
+	if (bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT)
 		up_read(&fs_info->commit_root_sem);
 	return ret;
 }
-- 
2.52.0

[PATCH v3 3/5] btrfs: move btrfs_bio::is_scrub into flags

[Johannes Thumshirn]

Remove struct btrfs_bio's is_scrub field and move it as a flag into the
newly introduced flags member of struct btrfs_bio.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/bio.c   |  4 ++--
 fs/btrfs/bio.h   | 11 +++++------
 fs/btrfs/scrub.c |  2 +-
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 2a9ab1275b7d..515b801a9c29 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -759,7 +759,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 	blk_status_t status;
 	int ret;
 
-	if (bbio->is_scrub || btrfs_is_data_reloc_root(inode->root))
+	if (bbio->flags & BTRFS_BIO_IS_SCRUB || btrfs_is_data_reloc_root(inode->root))
 		smap.rst_search_commit_root = true;
 	else
 		smap.rst_search_commit_root = false;
@@ -1011,7 +1011,7 @@ void btrfs_submit_repair_write(struct btrfs_bio *bbio, int mirror_num, bool dev_
 	ASSERT(mirror_num > 0);
 	ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
 	ASSERT(!is_data_inode(bbio->inode));
-	ASSERT(bbio->is_scrub);
+	ASSERT(bbio->flags & BTRFS_BIO_IS_SCRUB);
 
 	btrfs_bio_counter_inc_blocked(fs_info);
 	ret = btrfs_map_repair_block(fs_info, &smap, logical, length, mirror_num);
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index 18d7d441c1ec..8183e57725e1 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -22,6 +22,11 @@ typedef void (*btrfs_bio_end_io_t)(struct btrfs_bio *bbio);
 
 /* Use the commit root to look up csums (data read bio only). */
 #define BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT	(1 << 0)
+/*
+ * Since scrub will reuse btree inode, we need this flag to distinguish
+ * scrub bios.
+ */
+#define BTRFS_BIO_IS_SCRUB			(1 << 1)
 
 /*
  * Highlevel btrfs I/O structure.  It is allocated by btrfs_bio_alloc and
@@ -85,12 +90,6 @@ struct btrfs_bio {
 
 	unsigned int flags;
 
-	/*
-	 * Since scrub will reuse btree inode, we need this flag to distinguish
-	 * scrub bios.
-	 */
-	bool is_scrub;
-
 	/* Whether the csum generation for data write is async. */
 	bool async_csum;
 
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 41ead4a929b0..349d0bd25c7d 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -951,7 +951,7 @@ static struct btrfs_bio *alloc_scrub_bbio(struct btrfs_fs_info *fs_info,
 {
 	struct btrfs_bio *bbio = btrfs_bio_alloc(nr_vecs, opf, BTRFS_I(fs_info->btree_inode),
 						 logical, end_io, private);
-	bbio->is_scrub = true;
+	bbio->flags |= BTRFS_BIO_IS_SCRUB;
 	bbio->bio.bi_iter.bi_sector = logical >> SECTOR_SHIFT;
 	return bbio;
 }
-- 
2.52.0

[PATCH v3 4/5] btrfs: move btrfs_bio::async_csum into flags

[Johannes Thumshirn]

Remove struct btrfs_bio's async_csum field and move it as a flag into the
newly introduced flags member of struct btrfs_bio.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/bio.c       | 2 +-
 fs/btrfs/bio.h       | 5 ++---
 fs/btrfs/file-item.c | 4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 515b801a9c29..a1b0dd8b08f5 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -110,7 +110,7 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
 	/* Make sure we're already in task context. */
 	ASSERT(in_task());
 
-	if (bbio->async_csum)
+	if (bbio->flags & BTRFS_BIO_ASYNC_CSUM)
 		wait_for_completion(&bbio->csum_done);
 
 	bbio->bio.bi_status = status;
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index 8183e57725e1..d523929b4538 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -27,6 +27,8 @@ typedef void (*btrfs_bio_end_io_t)(struct btrfs_bio *bbio);
  * scrub bios.
  */
 #define BTRFS_BIO_IS_SCRUB			(1 << 1)
+/* Whether the csum generation for data write is async. */
+#define BTRFS_BIO_ASYNC_CSUM			(1 << 2)
 
 /*
  * Highlevel btrfs I/O structure.  It is allocated by btrfs_bio_alloc and
@@ -90,9 +92,6 @@ struct btrfs_bio {
 
 	unsigned int flags;
 
-	/* Whether the csum generation for data write is async. */
-	bool async_csum;
-
 	/* Whether the bio is written using zone append. */
 	bool can_use_append;
 
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 823c063bb4b7..8fa457e1ccd2 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -799,7 +799,7 @@ static void csum_one_bio_work(struct work_struct *work)
 	struct btrfs_bio *bbio = container_of(work, struct btrfs_bio, csum_work);
 
 	ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
-	ASSERT(bbio->async_csum == true);
+	ASSERT(bbio->flags & BTRFS_BIO_ASYNC_CSUM);
 	csum_one_bio(bbio, &bbio->csum_saved_iter);
 	complete(&bbio->csum_done);
 }
@@ -835,7 +835,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
 		return 0;
 	}
 	init_completion(&bbio->csum_done);
-	bbio->async_csum = true;
+	bbio->flags |= BTRFS_BIO_ASYNC_CSUM;
 	bbio->csum_saved_iter = bbio->bio.bi_iter;
 	INIT_WORK(&bbio->csum_work, csum_one_bio_work);
 	schedule_work(&bbio->csum_work);
-- 
2.52.0

[PATCH v3 5/5] btrfs: move btrfs_bio::can_use_append into flags

[Johannes Thumshirn]

Remove struct btrfs_bio's can_use_append field and move it as a flag into
the newly introduced flags member of struct btrfs_bio.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/bio.c | 9 +++++----
 fs/btrfs/bio.h | 5 ++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index a1b0dd8b08f5..ad7e930e4bd0 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -499,7 +499,7 @@ static void btrfs_submit_dev_bio(struct btrfs_device *dev, struct bio *bio)
 	 * For zone append writing, bi_sector must point the beginning of the
 	 * zone
 	 */
-	if (btrfs_bio(bio)->can_use_append &&
+	if (btrfs_bio(bio)->flags & BTRFS_BIO_CAN_USE_APPEND &&
 	    btrfs_dev_is_sequential(dev, physical)) {
 		u64 zone_start = round_down(physical, dev->fs_info->zone_size);
 
@@ -781,10 +781,11 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 	if (bio_op(bio) == REQ_OP_WRITE && is_data_bbio(bbio))
 		bbio->orig_logical = logical;
 
-	bbio->can_use_append = btrfs_use_zone_append(bbio);
+	if (btrfs_use_zone_append(bbio))
+		bbio->flags |= BTRFS_BIO_CAN_USE_APPEND;
 
 	map_length = min(map_length, length);
-	if (bbio->can_use_append)
+	if (bbio->flags & BTRFS_BIO_CAN_USE_APPEND)
 		map_length = btrfs_append_map_length(bbio, map_length);
 
 	if (map_length < length) {
@@ -839,7 +840,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 			status = errno_to_blk_status(ret);
 			if (status)
 				goto fail;
-		} else if (bbio->can_use_append ||
+		} else if (bbio->flags & BTRFS_BIO_CAN_USE_APPEND ||
 			   (btrfs_is_zoned(fs_info) && inode &&
 			    inode->flags & BTRFS_INODE_NODATASUM)) {
 			ret = btrfs_alloc_dummy_sum(bbio);
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index d523929b4538..410a500144c1 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -29,6 +29,8 @@ typedef void (*btrfs_bio_end_io_t)(struct btrfs_bio *bbio);
 #define BTRFS_BIO_IS_SCRUB			(1 << 1)
 /* Whether the csum generation for data write is async. */
 #define BTRFS_BIO_ASYNC_CSUM			(1 << 2)
+/* Whether the bio is written using zone append. */
+#define BTRFS_BIO_CAN_USE_APPEND		(1 << 3)
 
 /*
  * Highlevel btrfs I/O structure.  It is allocated by btrfs_bio_alloc and
@@ -92,9 +94,6 @@ struct btrfs_bio {
 
 	unsigned int flags;
 
-	/* Whether the bio is written using zone append. */
-	bool can_use_append;
-
 	/*
 	 * This member must come last, bio_alloc_bioset will allocate enough
 	 * bytes for entire btrfs_bio but relies on bio being last.
-- 
2.52.0

Re: [PATCH v3 1/5] btrfs: zoned: don't zone append to conventional zone

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

On Thu, Dec 04, 2025 at 01:42:23PM +0100, Johannes Thumshirn wrote:
> In case of a zoned RAID, it can happen that a data write is targeting a
> sequential write required zone and a conventional zone. In this case the
> bio will be marked as REQ_OP_ZONE_APPEND but for the conventional zone,
> this needs to be REQ_OP_WRITE.
> 
> The setting of REQ_OP_ZONE_APPEND is deferred to the last possible time in
> btrfs_submit_dev_bio(), but the decision if we can use zone append is
> cached in btrfs_bio.
> 
> Cc: Naohiro Aota <naohiro.aota@wdc.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Fixes: e9b9b911e03c ("btrfs: add raid stripe tree to features enabled with debug config")
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  fs/btrfs/bio.c | 20 ++++++++++----------
>  fs/btrfs/bio.h |  3 +++
>  2 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index 4a7bef895b97..33149f07e62d 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -480,6 +480,8 @@ static void btrfs_clone_write_end_io(struct bio *bio)
>  
>  static void btrfs_submit_dev_bio(struct btrfs_device *dev, struct bio *bio)
>  {
> +	u64 physical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
> +
>  	if (!dev || !dev->bdev ||
>  	    test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state) ||
>  	    (btrfs_op(bio) == BTRFS_MAP_WRITE &&
> @@ -494,12 +496,14 @@ static void btrfs_submit_dev_bio(struct btrfs_device *dev, struct bio *bio)
>  	 * For zone append writing, bi_sector must point the beginning of the
>  	 * zone
>  	 */
> -	if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
> -		u64 physical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
> +	if (btrfs_bio(bio)->can_use_append &&
> +	    btrfs_dev_is_sequential(dev, physical)) {
>  		u64 zone_start = round_down(physical, dev->fs_info->zone_size);
>  
>  		ASSERT(btrfs_dev_is_sequential(dev, physical));
>  		bio->bi_iter.bi_sector = zone_start >> SECTOR_SHIFT;
> +		bio->bi_opf &= ~REQ_OP_WRITE;
> +		bio->bi_opf |= REQ_OP_ZONE_APPEND;
>  	}
>  	btrfs_debug(dev->fs_info,
>  	"%s: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u",
> @@ -747,7 +751,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>  	u64 logical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
>  	u64 length = bio->bi_iter.bi_size;
>  	u64 map_length = length;
> -	bool use_append = btrfs_use_zone_append(bbio);
>  	struct btrfs_io_context *bioc = NULL;
>  	struct btrfs_io_stripe smap;
>  	blk_status_t status;
> @@ -775,8 +778,10 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>  	if (bio_op(bio) == REQ_OP_WRITE && is_data_bbio(bbio))
>  		bbio->orig_logical = logical;
>  
> +	bbio->can_use_append = btrfs_use_zone_append(bbio);
> +
>  	map_length = min(map_length, length);
> -	if (use_append)
> +	if (bbio->can_use_append)
>  		map_length = btrfs_append_map_length(bbio, map_length);
>  
>  	if (map_length < length) {
> @@ -805,11 +810,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>  	}
>  
>  	if (btrfs_op(bio) == BTRFS_MAP_WRITE) {
> -		if (use_append) {
> -			bio->bi_opf &= ~REQ_OP_WRITE;
> -			bio->bi_opf |= REQ_OP_ZONE_APPEND;
> -		}
> -
>  		if (is_data_bbio(bbio) && bioc && bioc->use_rst) {
>  			/*
>  			 * No locking for the list update, as we only add to
> @@ -836,7 +836,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>  			status = errno_to_blk_status(ret);
>  			if (status)
>  				goto fail;
> -		} else if (use_append ||
> +		} else if (bbio->can_use_append ||
>  			   (btrfs_is_zoned(fs_info) && inode &&
>  			    inode->flags & BTRFS_INODE_NODATASUM)) {
>  			ret = btrfs_alloc_dummy_sum(bbio);
> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> index 56279b7f3b2a..d6da9ed08bfa 100644
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -92,6 +92,9 @@ struct btrfs_bio {
>  	/* Whether the csum generation for data write is async. */
>  	bool async_csum;
>  
> +	/* Whether the bio is written using zone append. */
> +	bool can_use_append;
> +
>  	/*
>  	 * This member must come last, bio_alloc_bioset will allocate enough
>  	 * bytes for entire btrfs_bio but relies on bio being last.
> -- 
> 2.52.0
---end quoted text---

Re: [PATCH v3 2/5] btrfs: move btrfs_bio::csum_search_commit_root into flags

[Filipe Manana]

On Thu, Dec 4, 2025 at 12:44 PM Johannes Thumshirn
<johannes.thumshirn@wdc.com> wrote:
>
> Remove struct btrfs_bio's csum_search_commit_root field and move it as a
> flag into the newly introduced flags member of struct btrfs_bio.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  fs/btrfs/bio.c         | 5 ++++-
>  fs/btrfs/bio.h         | 6 ++++--
>  fs/btrfs/compression.c | 4 +++-
>  fs/btrfs/extent_io.c   | 7 ++++---
>  fs/btrfs/file-item.c   | 4 ++--
>  5 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index 33149f07e62d..2a9ab1275b7d 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -97,7 +97,10 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
>                 bbio->orig_logical = orig_bbio->orig_logical;
>                 orig_bbio->orig_logical += map_length;
>         }
> -       bbio->csum_search_commit_root = orig_bbio->csum_search_commit_root;
> +
> +       if (orig_bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT)
> +               bbio->flags |= BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT;
> +
>         atomic_inc(&orig_bbio->pending_ios);
>         return bbio;
>  }
> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> index d6da9ed08bfa..18d7d441c1ec 100644
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -20,6 +20,9 @@ struct btrfs_inode;
>
>  typedef void (*btrfs_bio_end_io_t)(struct btrfs_bio *bbio);
>
> +/* Use the commit root to look up csums (data read bio only). */
> +#define BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT      (1 << 0)

Please use ENUM_BIT(), it's our preferred way for defining flags and
makes backports less likely to cause problems due to accidental flags
getting the same value (we've been hit by this a few times in the
past).
See examples such as in extent-io-tree.h.

Also, what's the motivation for this and the other similar patches?
Does it reduce the size of the structure? If so, please mention it in
the changelog and what are the old and new sizes.

Thanks.

> +
>  /*
>   * Highlevel btrfs I/O structure.  It is allocated by btrfs_bio_alloc and
>   * passed to btrfs_submit_bbio() for mapping to the physical devices.
> @@ -80,8 +83,7 @@ struct btrfs_bio {
>         /* Save the first error status of split bio. */
>         blk_status_t status;
>
> -       /* Use the commit root to look up csums (data read bio only). */
> -       bool csum_search_commit_root;
> +       unsigned int flags;
>
>         /*
>          * Since scrub will reuse btree inode, we need this flag to distinguish
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 913fddce356a..1823262fabbd 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -603,7 +603,9 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio)
>         cb->compressed_len = compressed_len;
>         cb->compress_type = btrfs_extent_map_compression(em);
>         cb->orig_bbio = bbio;
> -       cb->bbio.csum_search_commit_root = bbio->csum_search_commit_root;
> +
> +       if (bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT)
> +               cb->bbio.flags |= BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT;
>
>         btrfs_free_extent_map(em);
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 2d32dfc34ae3..d321f6897388 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -166,9 +166,10 @@ static void bio_set_csum_search_commit_root(struct btrfs_bio_ctrl *bio_ctrl)
>         if (!(btrfs_op(&bbio->bio) == BTRFS_MAP_READ && is_data_inode(bbio->inode)))
>                 return;
>
> -       bio_ctrl->bbio->csum_search_commit_root =
> -               (bio_ctrl->generation &&
> -                bio_ctrl->generation < btrfs_get_fs_generation(bbio->inode->root->fs_info));
> +
> +       if (bio_ctrl->generation &&
> +           bio_ctrl->generation < btrfs_get_fs_generation(bbio->inode->root->fs_info))
> +               bbio->flags |= BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT;
>  }
>
>  static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 14e5257f0f04..823c063bb4b7 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -422,7 +422,7 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
>          * while we are not holding the commit_root_sem, and we get csums
>          * from across transactions.
>          */
> -       if (bbio->csum_search_commit_root) {
> +       if (bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT) {
>                 path->search_commit_root = true;
>                 path->skip_locking = true;
>                 down_read(&fs_info->commit_root_sem);
> @@ -473,7 +473,7 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
>                 bio_offset += count * sectorsize;
>         }
>
> -       if (bbio->csum_search_commit_root)
> +       if (bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT)
>                 up_read(&fs_info->commit_root_sem);
>         return ret;
>  }
> --
> 2.52.0
>
>

Re: [PATCH v3 2/5] btrfs: move btrfs_bio::csum_search_commit_root into flags

[Johannes Thumshirn]

On 12/4/25 2:31 PM, Filipe Manana wrote:
> Also, what's the motivation for this and the other similar patches?
> Does it reduce the size of the structure? If so, please mention it in
> the changelog and what are the old and new sizes.

It doesn't so I didn't feel the need to document it. As for the 
motivation, both Qu and me had the idea to collapse the booleans into a 
flags field as a response to 
https://lore.kernel.org/linux-btrfs/20251204092649.GB19866@lst.de

That was the whole reason for this exercise. I only really care about 
1/5 of this series as it fixes an ASSERT() I had on one of my test 
machines. There's still room for 3 more booleans until the struct grows 
so the need for 2-5/5 isn't eminent yet.

Re: [PATCH v3 2/5] btrfs: move btrfs_bio::csum_search_commit_root into flags

[Qu Wenruo]

在 2025/12/4 23:12, Johannes Thumshirn 写道:
> Remove struct btrfs_bio's csum_search_commit_root field and move it as a
> flag into the newly introduced flags member of struct btrfs_bio.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
[...]
> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> index d6da9ed08bfa..18d7d441c1ec 100644
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -20,6 +20,9 @@ struct btrfs_inode;
>   
>   typedef void (*btrfs_bio_end_io_t)(struct btrfs_bio *bbio);
>   
> +/* Use the commit root to look up csums (data read bio only). */
> +#define BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT	(1 << 0)
> +
>   /*
>    * Highlevel btrfs I/O structure.  It is allocated by btrfs_bio_alloc and
>    * passed to btrfs_submit_bbio() for mapping to the physical devices.
> @@ -80,8 +83,7 @@ struct btrfs_bio {
>   	/* Save the first error status of split bio. */
>   	blk_status_t status;
>   
> -	/* Use the commit root to look up csums (data read bio only). */
> -	bool csum_search_commit_root;
> +	unsigned int flags;

Unsigned int is a little overkilled in this case.
We have at most 4 bits so far, but unsigned int is u32.

I think using bool bitfields is more space efficient, and the bitfields 
members are all set without race, it should be safe.

Furthermore I also tried to reduce the width of mirror_num, with all 
those work and properly re-order the members, I can reduce 8 bytes from 
btrfs_bio:

         } __attribute__((__aligned__(8)));               /*    16   120 */
         /* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
         btrfs_bio_end_io_t         end_io;               /*   136     8 */
         void *                     private;              /*   144     8 */
         atomic_t                   pending_ios;          /*   152     4 */
         u8                         mirror_num;           /*   156     1 */
         blk_status_t               status;               /*   157     1 */
         bool                       csum_search_commit_root:1; /*   158: 
0  1 */
         bool                       is_scrub:1;           /*   158: 1  1 */
         bool                       async_csum:1;         /*   158: 2  1 */

         /* XXX 5 bits hole, try to pack */
         /* XXX 1 byte hole, try to pack */

         struct work_struct         end_io_work;          /*   160    32 */
         /* --- cacheline 3 boundary (192 bytes) --- */
         struct bio                 bio __attribute__((__aligned__(8))); 
/*   192   112 */

         /* XXX last struct has 1 hole */

         /* size: 304, cachelines: 5, members: 13 */

The old size is 312, so a 8 bytes improvement on the size of btrfs_bio.

Thanks,
Qu

>   
>   	/*
>   	 * Since scrub will reuse btree inode, we need this flag to distinguish
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 913fddce356a..1823262fabbd 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -603,7 +603,9 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio)
>   	cb->compressed_len = compressed_len;
>   	cb->compress_type = btrfs_extent_map_compression(em);
>   	cb->orig_bbio = bbio;
> -	cb->bbio.csum_search_commit_root = bbio->csum_search_commit_root;
> +
> +	if (bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT)
> +		cb->bbio.flags |= BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT;
>   
>   	btrfs_free_extent_map(em);
>   
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 2d32dfc34ae3..d321f6897388 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -166,9 +166,10 @@ static void bio_set_csum_search_commit_root(struct btrfs_bio_ctrl *bio_ctrl)
>   	if (!(btrfs_op(&bbio->bio) == BTRFS_MAP_READ && is_data_inode(bbio->inode)))
>   		return;
>   
> -	bio_ctrl->bbio->csum_search_commit_root =
> -		(bio_ctrl->generation &&
> -		 bio_ctrl->generation < btrfs_get_fs_generation(bbio->inode->root->fs_info));
> +
> +	if (bio_ctrl->generation &&
> +	    bio_ctrl->generation < btrfs_get_fs_generation(bbio->inode->root->fs_info))
> +		bbio->flags |= BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT;
>   }
>   
>   static void submit_one_bio(struct btrfs_bio_ctrl *bio_ctrl)
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 14e5257f0f04..823c063bb4b7 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -422,7 +422,7 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
>   	 * while we are not holding the commit_root_sem, and we get csums
>   	 * from across transactions.
>   	 */
> -	if (bbio->csum_search_commit_root) {
> +	if (bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT) {
>   		path->search_commit_root = true;
>   		path->skip_locking = true;
>   		down_read(&fs_info->commit_root_sem);
> @@ -473,7 +473,7 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
>   		bio_offset += count * sectorsize;
>   	}
>   
> -	if (bbio->csum_search_commit_root)
> +	if (bbio->flags & BTRFS_BIO_CSUM_SEARCH_COMMIT_ROOT)
>   		up_read(&fs_info->commit_root_sem);
>   	return ret;
>   }

Re: [PATCH v3 2/5] btrfs: move btrfs_bio::csum_search_commit_root into flags

[Johannes Thumshirn]

On 12/4/25 11:12 PM, Qu Wenruo wrote:
> Unsigned int is a little overkilled in this case.
> We have at most 4 bits so far, but unsigned int is u32.
>
> I think using bool bitfields is more space efficient, and the bitfields
> members are all set without race, it should be safe.
>
> Furthermore I also tried to reduce the width of mirror_num, with all
> those work and properly re-order the members, I can reduce 8 bytes from
> btrfs_bio:
>
>           } __attribute__((__aligned__(8)));               /*    16   120 */
>           /* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
>           btrfs_bio_end_io_t         end_io;               /*   136     8 */
>           void *                     private;              /*   144     8 */
>           atomic_t                   pending_ios;          /*   152     4 */
>           u8                         mirror_num;           /*   156     1 */
>           blk_status_t               status;               /*   157     1 */
>           bool                       csum_search_commit_root:1; /*   158:
> 0  1 */
>           bool                       is_scrub:1;           /*   158: 1  1 */
>           bool                       async_csum:1;         /*   158: 2  1 */
>
>           /* XXX 5 bits hole, try to pack */
>           /* XXX 1 byte hole, try to pack */
>
>           struct work_struct         end_io_work;          /*   160    32 */
>           /* --- cacheline 3 boundary (192 bytes) --- */
>           struct bio                 bio __attribute__((__aligned__(8)));
> /*   192   112 */
>
>           /* XXX last struct has 1 hole */
>
>           /* size: 304, cachelines: 5, members: 13 */
>
> The old size is 312, so a 8 bytes improvement on the size of btrfs_bio.

That's definitely more the kind of improvement expected, can you send a 
patch for it? Meanwhile I've pushed 1/5 to for-next.

Re: [PATCH v3 2/5] btrfs: move btrfs_bio::csum_search_commit_root into flags

[Qu Wenruo]

在 2025/12/5 17:09, Johannes Thumshirn 写道:
> On 12/4/25 11:12 PM, Qu Wenruo wrote:
>> Unsigned int is a little overkilled in this case.
>> We have at most 4 bits so far, but unsigned int is u32.
>>
>> I think using bool bitfields is more space efficient, and the bitfields
>> members are all set without race, it should be safe.
>>
>> Furthermore I also tried to reduce the width of mirror_num, with all
>> those work and properly re-order the members, I can reduce 8 bytes from
>> btrfs_bio:
>>
>>            } __attribute__((__aligned__(8)));               /*    16   120 */
>>            /* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
>>            btrfs_bio_end_io_t         end_io;               /*   136     8 */
>>            void *                     private;              /*   144     8 */
>>            atomic_t                   pending_ios;          /*   152     4 */
>>            u8                         mirror_num;           /*   156     1 */
>>            blk_status_t               status;               /*   157     1 */
>>            bool                       csum_search_commit_root:1; /*   158:
>> 0  1 */
>>            bool                       is_scrub:1;           /*   158: 1  1 */
>>            bool                       async_csum:1;         /*   158: 2  1 */
>>
>>            /* XXX 5 bits hole, try to pack */
>>            /* XXX 1 byte hole, try to pack */
>>
>>            struct work_struct         end_io_work;          /*   160    32 */
>>            /* --- cacheline 3 boundary (192 bytes) --- */
>>            struct bio                 bio __attribute__((__aligned__(8)));
>> /*   192   112 */
>>
>>            /* XXX last struct has 1 hole */
>>
>>            /* size: 304, cachelines: 5, members: 13 */
>>
>> The old size is 312, so a 8 bytes improvement on the size of btrfs_bio.
> 
> That's definitely more the kind of improvement expected, can you send a
> patch for it? Meanwhile I've pushed 1/5 to for-next.
> 

Sure, after you.

Thanks,
Qu

Re: [PATCH v3 2/5] btrfs: move btrfs_bio::csum_search_commit_root into flags

[Johannes Thumshirn]

On 12/5/25 8:08 AM, Qu Wenruo wrote:
>
> 在 2025/12/5 17:09, Johannes Thumshirn 写道:
>> On 12/4/25 11:12 PM, Qu Wenruo wrote:
>>> Unsigned int is a little overkilled in this case.
>>> We have at most 4 bits so far, but unsigned int is u32.
>>>
>>> I think using bool bitfields is more space efficient, and the bitfields
>>> members are all set without race, it should be safe.
>>>
>>> Furthermore I also tried to reduce the width of mirror_num, with all
>>> those work and properly re-order the members, I can reduce 8 bytes from
>>> btrfs_bio:
>>>
>>>             } __attribute__((__aligned__(8)));               /*    16   120 */
>>>             /* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
>>>             btrfs_bio_end_io_t         end_io;               /*   136     8 */
>>>             void *                     private;              /*   144     8 */
>>>             atomic_t                   pending_ios;          /*   152     4 */
>>>             u8                         mirror_num;           /*   156     1 */
>>>             blk_status_t               status;               /*   157     1 */
>>>             bool                       csum_search_commit_root:1; /*   158:
>>> 0  1 */
>>>             bool                       is_scrub:1;           /*   158: 1  1 */
>>>             bool                       async_csum:1;         /*   158: 2  1 */
>>>
>>>             /* XXX 5 bits hole, try to pack */
>>>             /* XXX 1 byte hole, try to pack */
>>>
>>>             struct work_struct         end_io_work;          /*   160    32 */
>>>             /* --- cacheline 3 boundary (192 bytes) --- */
>>>             struct bio                 bio __attribute__((__aligned__(8)));
>>> /*   192   112 */
>>>
>>>             /* XXX last struct has 1 hole */
>>>
>>>             /* size: 304, cachelines: 5, members: 13 */
>>>
>>> The old size is 312, so a 8 bytes improvement on the size of btrfs_bio.
>> That's definitely more the kind of improvement expected, can you send a
>> patch for it? Meanwhile I've pushed 1/5 to for-next.
>>
> Sure, after you.
Already pushed.

Re: [PATCH v3 1/5] btrfs: zoned: don't zone append to conventional zone

[Naohiro Aota]

On Thu Dec 4, 2025 at 9:42 PM JST, Johannes Thumshirn wrote:
> In case of a zoned RAID, it can happen that a data write is targeting a
> sequential write required zone and a conventional zone. In this case the
> bio will be marked as REQ_OP_ZONE_APPEND but for the conventional zone,
> this needs to be REQ_OP_WRITE.
>
> The setting of REQ_OP_ZONE_APPEND is deferred to the last possible time in
> btrfs_submit_dev_bio(), but the decision if we can use zone append is
> cached in btrfs_bio.
>
> Cc: Naohiro Aota <naohiro.aota@wdc.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Fixes: e9b9b911e03c ("btrfs: add raid stripe tree to features enabled with debug config")
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Looks good.

Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>

Re: [PATCH v3 1/5] btrfs: zoned: don't zone append to conventional zone

[Chris Mason]

Johannes Thumshirn <johannes.thumshirn@wdc.com> wrote:
> In case of a zoned RAID, it can happen that a data write is targeting a
> sequential write required zone and a conventional zone. In this case the
> bio will be marked as REQ_OP_ZONE_APPEND but for the conventional zone,
> this needs to be REQ_OP_WRITE.
> 
> The setting of REQ_OP_ZONE_APPEND is deferred to the last possible time in
> btrfs_submit_dev_bio(), but the decision if we can use zone append is
> cached in btrfs_bio.

Hi everyone,

I ran linux-next through some AI patch review, and this one was flagged.

Apologies in advance if this is wonky, the text below is based on the
linux-next version of the commit and the mailing list has something slightly
different.  But I think the overall result is still valid, it doesn't look
like btrfs_split_bio() properly copies can_append?

It still seems valid after the enum bit transition later in this thread.

> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c

[ ... ]

> @@ -747,7 +750,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>  	u64 logical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
>  	u64 length = bio->bi_iter.bi_size;
>  	u64 map_length = length;
> -	bool use_append = btrfs_use_zone_append(bbio);
>  	struct btrfs_io_context *bioc = NULL;
>  	struct btrfs_io_stripe smap;
>  	blk_status_t status;
> @@ -775,8 +777,10 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>  	if (bio_op(bio) == REQ_OP_WRITE && is_data_bbio(bbio))
>  		bbio->orig_logical = logical;
>
> +	bbio->can_use_append = btrfs_use_zone_append(bbio);
> +
>  	map_length = min(map_length, length);
> -	if (use_append)
> +	if (bbio->can_use_append)
>  		map_length = btrfs_append_map_length(bbio, map_length);
>
>  	if (map_length < length) {
>  		struct btrfs_bio *split;
>
>  		split = btrfs_split_bio(fs_info, bbio, map_length);

When a bio is split here, btrfs_split_bio() creates the new split bio via
btrfs_bio_init() which zeroes the struct with memset. Looking at
btrfs_split_bio(), it copies csum_search_commit_root from the original but
does not copy can_use_append.

After the split, the code does:

    bbio = split;
    bio = &bbio->bio;

This means the split bio (with can_use_append = false) gets submitted, not
the original. In btrfs_submit_dev_bio(), the condition:

    if (btrfs_bio(bio)->can_use_append && btrfs_dev_is_sequential(...))

Will be false for the split bio even when writing to a sequential zone.
Does the split bio need to inherit can_use_append from the original? The
old code used a local variable use_append which persisted across the split.

> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -92,6 +92,9 @@ struct btrfs_bio {
>  	/* Whether the csum generation for data write is async. */
>  	bool async_csum;
>
> +	/* Whether the bio is written using zone append. */
> +	bool can_use_append;

Re: [PATCH v3 1/5] btrfs: zoned: don't zone append to conventional zone

[Johannes Thumshirn]

On 1/25/26 2:21 PM, Chris Mason wrote:
> Johannes Thumshirn <johannes.thumshirn@wdc.com> wrote:
>> In case of a zoned RAID, it can happen that a data write is targeting a
>> sequential write required zone and a conventional zone. In this case the
>> bio will be marked as REQ_OP_ZONE_APPEND but for the conventional zone,
>> this needs to be REQ_OP_WRITE.
>>
>> The setting of REQ_OP_ZONE_APPEND is deferred to the last possible time in
>> btrfs_submit_dev_bio(), but the decision if we can use zone append is
>> cached in btrfs_bio.
> Hi everyone,
>
> I ran linux-next through some AI patch review, and this one was flagged.
>
> Apologies in advance if this is wonky, the text below is based on the
> linux-next version of the commit and the mailing list has something slightly
> different.  But I think the overall result is still valid, it doesn't look
> like btrfs_split_bio() properly copies can_append?
>
> It still seems valid after the enum bit transition later in this thread.
>
>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>> --- a/fs/btrfs/bio.c
>> +++ b/fs/btrfs/bio.c
> [ ... ]
>
>> @@ -747,7 +750,6 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>>   	u64 logical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
>>   	u64 length = bio->bi_iter.bi_size;
>>   	u64 map_length = length;
>> -	bool use_append = btrfs_use_zone_append(bbio);
>>   	struct btrfs_io_context *bioc = NULL;
>>   	struct btrfs_io_stripe smap;
>>   	blk_status_t status;
>> @@ -775,8 +777,10 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>>   	if (bio_op(bio) == REQ_OP_WRITE && is_data_bbio(bbio))
>>   		bbio->orig_logical = logical;
>>
>> +	bbio->can_use_append = btrfs_use_zone_append(bbio);
>> +
>>   	map_length = min(map_length, length);
>> -	if (use_append)
>> +	if (bbio->can_use_append)
>>   		map_length = btrfs_append_map_length(bbio, map_length);
>>
>>   	if (map_length < length) {
>>   		struct btrfs_bio *split;
>>
>>   		split = btrfs_split_bio(fs_info, bbio, map_length);
> When a bio is split here, btrfs_split_bio() creates the new split bio via
> btrfs_bio_init() which zeroes the struct with memset. Looking at
> btrfs_split_bio(), it copies csum_search_commit_root from the original but
> does not copy can_use_append.
>
> After the split, the code does:
>
>      bbio = split;
>      bio = &bbio->bio;
>
> This means the split bio (with can_use_append = false) gets submitted, not
> the original. In btrfs_submit_dev_bio(), the condition:
>
>      if (btrfs_bio(bio)->can_use_append && btrfs_dev_is_sequential(...))
>
> Will be false for the split bio even when writing to a sequential zone.
> Does the split bio need to inherit can_use_append from the original? The
> old code used a local variable use_append which persisted across the split.


Hi Chris,

This indeed seems correct. Also for all the other "flags" (is_scrub, 
is_remap, async_csum) but csum_search_commit_root, isn't it?

So the correct fix would then be:

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index d3475d179362..0a69e09bfe28 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -97,7 +97,13 @@ static struct btrfs_bio *btrfs_split_bio(struct 
btrfs_fs_info *fs_info,
                 bbio->orig_logical = orig_bbio->orig_logical;
                 orig_bbio->orig_logical += map_length;
         }
+
         bbio->csum_search_commit_root = orig_bbio->csum_search_commit_root;
+       bbio->can_use_append = orig_bbio->can_use_append;
+       bbio->is_scrub = orig_bbio->is_scrub;
+       bbio->is_remap = orig_bbio->is_remap;
+       bbio->async_csum = orig_bbio->async_csum;
+
         atomic_inc(&orig_bbio->pending_ios);
         return bbio;
  }

Re: [PATCH v3 1/5] btrfs: zoned: don't zone append to conventional zone

[Chris Mason]

On 1/26/26 2:51 AM, Johannes Thumshirn wrote:
> On 1/25/26 2:21 PM, Chris Mason wrote:
>> Johannes Thumshirn <johannes.thumshirn@wdc.com> wrote:
>>> In case of a zoned RAID, it can happen that a data write is targeting a
>>> sequential write required zone and a conventional zone. In this case the
>>> bio will be marked as REQ_OP_ZONE_APPEND but for the conventional zone,
>>> this needs to be REQ_OP_WRITE.
>>>
>>> The setting of REQ_OP_ZONE_APPEND is deferred to the last possible time in
>>> btrfs_submit_dev_bio(), but the decision if we can use zone append is
>>> cached in btrfs_bio.
>> Hi everyone,
>>
>> I ran linux-next through some AI patch review, and this one was flagged.
>>
>> Apologies in advance if this is wonky, the text below is based on the
>> linux-next version of the commit and the mailing list has something slightly
>> different.  But I think the overall result is still valid, it doesn't look
>> like btrfs_split_bio() properly copies can_append?
>>
>> It still seems valid after the enum bit transition later in this thread.
>>

[ ... ]

> Hi Chris,
> 
> This indeed seems correct. Also for all the other "flags" (is_scrub, 
> is_remap, async_csum) but csum_search_commit_root, isn't it?
> 
> So the correct fix would then be:
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index d3475d179362..0a69e09bfe28 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -97,7 +97,13 @@ static struct btrfs_bio *btrfs_split_bio(struct 
> btrfs_fs_info *fs_info,
>                  bbio->orig_logical = orig_bbio->orig_logical;
>                  orig_bbio->orig_logical += map_length;
>          }
> +
>          bbio->csum_search_commit_root = orig_bbio->csum_search_commit_root;
> +       bbio->can_use_append = orig_bbio->can_use_append;
> +       bbio->is_scrub = orig_bbio->is_scrub;
> +       bbio->is_remap = orig_bbio->is_remap;
> +       bbio->async_csum = orig_bbio->async_csum;
> +
>          atomic_inc(&orig_bbio->pending_ios);
>          return bbio;
>   }
> 

Yeah, this seems right to me, thanks!

-chris