On 7/18/25 12:38 AM, Damien Le Moal wrote:
On 7/18/25 05:58, Bart Van Assche wrote:
@@ -768,14 +771,19 @@ static bool blk_zone_wplug_handle_reset_all(struct bio *bio)
static void disk_zone_wplug_schedule_bio_work(struct gendisk *disk,
struct blk_zone_wplug *zwplug)
{
+ lockdep_assert_held(&zwplug->lock);
Unrelated change. Please move this to a prep patch.
I will drop this change since I don't really need this change.
+
/*
* Take a reference on the zone write plug and schedule the submission
* of the next plugged BIO. blk_zone_wplug_bio_work() will release the
* reference we take here.
*/
- WARN_ON_ONCE(!(zwplug->flags & BLK_ZONE_WPLUG_PLUGGED));
Why do you remove this warning ?
This warning probably can be retained. I will look into restoring it.
@@ -972,9 +980,12 @@ static bool blk_zone_wplug_prepare_bio(struct blk_zone_wplug *zwplug,
return true;
}
-static bool blk_zone_wplug_handle_write(struct bio *bio, unsigned int nr_segs)
+static bool blk_zone_wplug_handle_write(struct bio *bio, unsigned int nr_segs,
+ int from_cpu)
{
struct gendisk *disk = bio->bi_bdev->bd_disk;
+ const bool ordered_hwq = bio_op(bio) != REQ_OP_ZONE_APPEND &&
+ disk->queue->limits.features & BLK_FEAT_ORDERED_HWQ;
This is not correct. If the BIO is a zone append and
blk_zone_wplug_handle_write() is called, it means that we need to handle the BIO
using zone append emulation, that is, the BIO will be a regular write. So you
must treat it as if it originally was a regular write.
Hmm ... my understanding is that zone append emulation and also the
conversion of REQ_OP_ZONE_APPEND into REQ_OP_WRITE happens after the
above code has been executed, namely by blk_zone_wplug_prepare_bio().
From that function:
[ ... ]
if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
bio->bi_opf &= ~REQ_OP_MASK;
bio->bi_opf |= REQ_OP_WRITE | REQ_NOMERGE;
[ ... ]
Did I perhaps misunderstand your comment?
+ if (refcount_read(&zwplug->ref) == 2)
+ zwplug->from_cpu = -1;
This needs a comment explaining why you use the plug ref count instead of
unconditionally clearing from_cpu.
I'm considering to add the following comment:
/*
* zwplug->from_cpu must not change while one or more writes are pending
* for the zone associated with zwplug. zwplug->ref is 2 when the plug
* is unused (one reference taken when the plug was allocated and
* another reference taken by the caller context). Reset
* zwplug->from_cpu if no more writes are pending.
*/
Thanks,
Bart.
