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;
