All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs: zoned: don't zone append to conventional zone
@ 2025-12-02 10:16 Johannes Thumshirn
  2025-12-02 13:29 ` Christoph Hellwig
  0 siblings, 1 reply; 5+ messages in thread
From: Johannes Thumshirn @ 2025-12-02 10:16 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Johannes Thumshirn, Naohiro Aota, Christoph Hellwig

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.

This is a partial revert of commit d5e4377d5051 ("btrfs: split zone append
bios in btrfs_submit_bio") which was introduced before zoned RAID.

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 | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 4a7bef895b97..bc64faf361c9 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -492,14 +492,25 @@ 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
+	 * zone.
+	 *
+	 * 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.
 	 */
 	if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
 		u64 physical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
-		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;
+		if (btrfs_dev_is_sequential(dev, physical)) {
+			u64 zone_start =
+				round_down(physical, dev->fs_info->zone_size);
+
+			bio->bi_iter.bi_sector = zone_start >> SECTOR_SHIFT;
+		} else {
+			bio->bi_opf &= ~REQ_OP_ZONE_APPEND;
+			bio->bi_opf |= REQ_OP_WRITE;
+		}
 	}
 	btrfs_debug(dev->fs_info,
 	"%s: rw %d 0x%x, sector=%llu, dev=%lu (%s id %llu), size=%u",
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs: zoned: don't zone append to conventional zone
  2025-12-02 10:16 [PATCH] btrfs: zoned: don't zone append to conventional zone Johannes Thumshirn
@ 2025-12-02 13:29 ` Christoph Hellwig
  2025-12-02 13:43   ` Johannes Thumshirn
  0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2025-12-02 13:29 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: linux-btrfs, Naohiro Aota, Christoph Hellwig

On Tue, Dec 02, 2025 at 11:16:31AM +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.
> 
> This is a partial revert of commit d5e4377d5051 ("btrfs: split zone append
> bios in btrfs_submit_bio") which was introduced before zoned RAID.

Hmm, how does the BLOCK_GROUP_FLAG_SEQUENTIAL_ZONE flag used by
btrfs_use_zone_append actually work for the raid code?

Either way, this is a bit ugly as we now special case zone append in
multiple places.  Can we just pass the use_append flag down to
btrfs_submit_dev_bio and only set REQ_OP_ZONE_APPEND there to keep it
all tidy?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs: zoned: don't zone append to conventional zone
  2025-12-02 13:29 ` Christoph Hellwig
@ 2025-12-02 13:43   ` Johannes Thumshirn
  2025-12-02 13:44     ` hch
  0 siblings, 1 reply; 5+ messages in thread
From: Johannes Thumshirn @ 2025-12-02 13:43 UTC (permalink / raw)
  To: hch; +Cc: linux-btrfs@vger.kernel.org, Naohiro Aota

On 12/2/25 2:29 PM, Christoph Hellwig wrote:
> On Tue, Dec 02, 2025 at 11:16:31AM +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.
>>
>> This is a partial revert of commit d5e4377d5051 ("btrfs: split zone append
>> bios in btrfs_submit_bio") which was introduced before zoned RAID.
> Hmm, how does the BLOCK_GROUP_FLAG_SEQUENTIAL_ZONE flag used by
> btrfs_use_zone_append actually work for the raid code?


If one of the zones backing the block-group is sequential the flag is 
set, see btrfs_load_block_group_zone_info().

> Either way, this is a bit ugly as we now special case zone append in
> multiple places.  Can we just pass the use_append flag down to
> btrfs_submit_dev_bio and only set REQ_OP_ZONE_APPEND there to keep it
> all tidy?
Let me have a look how we can make that non-ugly. Or just use 
btrfs_dev_is_sequential() in btrfs_submit_dev_bio(), which is probably 
nicer as it doesn't need a rbtree lookup for the block-group.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs: zoned: don't zone append to conventional zone
  2025-12-02 13:43   ` Johannes Thumshirn
@ 2025-12-02 13:44     ` hch
  2025-12-02 13:50       ` Johannes Thumshirn
  0 siblings, 1 reply; 5+ messages in thread
From: hch @ 2025-12-02 13:44 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: hch, linux-btrfs@vger.kernel.org, Naohiro Aota

On Tue, Dec 02, 2025 at 01:43:07PM +0000, Johannes Thumshirn wrote:
> On 12/2/25 2:29 PM, Christoph Hellwig wrote:
> > On Tue, Dec 02, 2025 at 11:16:31AM +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.
> >>
> >> This is a partial revert of commit d5e4377d5051 ("btrfs: split zone append
> >> bios in btrfs_submit_bio") which was introduced before zoned RAID.
> > Hmm, how does the BLOCK_GROUP_FLAG_SEQUENTIAL_ZONE flag used by
> > btrfs_use_zone_append actually work for the raid code?
> 
> 
> If one of the zones backing the block-group is sequential the flag is 
> set, see btrfs_load_block_group_zone_info().
> 
> > Either way, this is a bit ugly as we now special case zone append in
> > multiple places.  Can we just pass the use_append flag down to
> > btrfs_submit_dev_bio and only set REQ_OP_ZONE_APPEND there to keep it
> > all tidy?
> Let me have a look how we can make that non-ugly. Or just use 
> btrfs_dev_is_sequential() in btrfs_submit_dev_bio(), which is probably 
> nicer as it doesn't need a rbtree lookup for the block-group.

Well, it still needs to check all the other conditions that prohibit
using zone append (metadata, reloc inode, ...)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs: zoned: don't zone append to conventional zone
  2025-12-02 13:44     ` hch
@ 2025-12-02 13:50       ` Johannes Thumshirn
  0 siblings, 0 replies; 5+ messages in thread
From: Johannes Thumshirn @ 2025-12-02 13:50 UTC (permalink / raw)
  To: hch; +Cc: linux-btrfs@vger.kernel.org, Naohiro Aota

On 12/2/25 2:44 PM, hch wrote:
> On Tue, Dec 02, 2025 at 01:43:07PM +0000, Johannes Thumshirn wrote:
>> On 12/2/25 2:29 PM, Christoph Hellwig wrote:
>>> On Tue, Dec 02, 2025 at 11:16:31AM +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.
>>>>
>>>> This is a partial revert of commit d5e4377d5051 ("btrfs: split zone append
>>>> bios in btrfs_submit_bio") which was introduced before zoned RAID.
>>> Hmm, how does the BLOCK_GROUP_FLAG_SEQUENTIAL_ZONE flag used by
>>> btrfs_use_zone_append actually work for the raid code?
>>
>> If one of the zones backing the block-group is sequential the flag is
>> set, see btrfs_load_block_group_zone_info().
>>
>>> Either way, this is a bit ugly as we now special case zone append in
>>> multiple places.  Can we just pass the use_append flag down to
>>> btrfs_submit_dev_bio and only set REQ_OP_ZONE_APPEND there to keep it
>>> all tidy?
>> Let me have a look how we can make that non-ugly. Or just use
>> btrfs_dev_is_sequential() in btrfs_submit_dev_bio(), which is probably
>> nicer as it doesn't need a rbtree lookup for the block-group.
> Well, it still needs to check all the other conditions that prohibit
> using zone append (metadata, reloc inode, ...)
>
Yes I just realized that...


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-12-02 13:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02 10:16 [PATCH] btrfs: zoned: don't zone append to conventional zone Johannes Thumshirn
2025-12-02 13:29 ` Christoph Hellwig
2025-12-02 13:43   ` Johannes Thumshirn
2025-12-02 13:44     ` hch
2025-12-02 13:50       ` Johannes Thumshirn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.