Re: [PATCH v2 4/4] block/mq-deadline: Prevent zoned write reordering due to I/O prioritization

Bart Van Assche
2023-12-19T17:42:38Z

On 12/19/23 04:10, Christoph Hellwig wrote:
> On Mon, Dec 18, 2023 at 01:13:42PM -0800, Bart Van Assche wrote:
>> Assigning I/O priorities with the ioprio cgroup policy may cause
>> different I/O priorities to be assigned to write requests for the same
>> zone. Prevent that this causes unaligned write errors by adding zoned
>> writes for the same zone in the same priority queue as prior zoned
>> writes.
>
> I still think this is fundamentally the wrong thing to do. If you set
> different priorities, you want I/O to be reordered, so ignoring that
> is a bad thing.

Hi Christoph,

How about not setting the I/O priority of sequential zoned writes as in
the (untested) patch below?

Thanks,

Bart.

[PATCH] block: Do not set the I/O priority for sequential zoned writes

---
block/blk-mq.c | 7 +++++++
include/linux/blk-mq.h | 17 +++++++++++++++++
2 files changed, 24 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index c11c97afa0bc..668888103a47 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2922,6 +2922,13 @@ static bool blk_mq_can_use_cached_rq(struct request *rq, struct blk_plug *plug,

static void bio_set_ioprio(struct bio *bio)
{
+ /*
+ * Do not set the I/O priority of sequential zoned write bios because
+ * this could lead to reordering and hence to unaligned write errors.
+ */
+ if (blk_bio_is_seq_zoned_write(bio))
+ return;
+
/* Nobody set ioprio so far? Initialize it based on task's nice value */
if (IOPRIO_PRIO_CLASS(bio->bi_ioprio) == IOPRIO_CLASS_NONE)
bio->bi_ioprio = get_current_ioprio();
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 1ab3081c82ed..e7fa81170b7c 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -1149,6 +1149,18 @@ static inline unsigned int blk_rq_zone_no(struct request *rq)
return disk_zone_no(rq->q->disk, blk_rq_pos(rq));
}

+/**
+ * blk_bio_is_seq_zoned_write() - Check if @bio requires write serialization.
+ * @bio: Bio to examine.
+ *
+ * Note: REQ_OP_ZONE_APPEND bios do not require serialization.
+ */
+static inline bool blk_bio_is_seq_zoned_write(struct bio *bio)
+{
+ return disk_zone_is_seq(bio->bi_bdev->bd_disk, bio->bi_iter.bi_sector) &&
+ op_needs_zoned_write_locking(bio_op(bio));
+}
+
static inline unsigned int blk_rq_zone_is_seq(struct request *rq)
{
return disk_zone_is_seq(rq->q->disk, blk_rq_pos(rq));
@@ -1196,6 +1208,11 @@ static inline bool blk_req_can_dispatch_to_zone(struct request *rq)
return !blk_req_zone_is_write_locked(rq);
}
#else /* CONFIG_BLK_DEV_ZONED */
+static inline bool blk_bio_is_seq_zoned_write(struct bio *bio)
+{
+ return false;
+}
+
static inline bool blk_rq_is_seq_zoned_write(struct request *rq)
{
return false;