On Mon, Apr 24, 2023 at 01:33:28PM -0700, Bart Van Assche wrote:
> Start dispatching from the start of a zone instead of from the starting
> position of the most recently dispatched request.
>
> If a zoned write is requeued with an LBA that is lower than already
> inserted zoned writes, make sure that it is submitted first.
>
> Cc: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Ming Lei <ming.lei@xxxxxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> block/mq-deadline.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/block/mq-deadline.c b/block/mq-deadline.c
> index bf1dfe9fe9c9..2de6c63190d8 100644
> --- a/block/mq-deadline.c
> +++ b/block/mq-deadline.c
> @@ -156,13 +156,23 @@ deadline_latter_request(struct request *rq)
> return NULL;
> }
>
> -/* Return the first request for which blk_rq_pos() >= pos. */
> +/*
> + * Return the first request for which blk_rq_pos() >= @pos. For zoned devices,
> + * return the first request after the highest zone start <= @pos.
> + */
> static inline struct request *deadline_from_pos(struct dd_per_prio *per_prio,
> enum dd_data_dir data_dir, sector_t pos)
> {
> struct rb_node *node = per_prio->sort_list[data_dir].rb_node;
> struct request *rq, *res = NULL;
>
> + if (!node)
> + return NULL;
> +
> + rq = rb_entry_rq(node);
> + if (blk_rq_is_seq_zoned_write(rq))
> + pos -= bdev_offset_from_zone_start(rq->q->disk->part0, pos);
This looks a bit odd. I'd write this as:
if (blk_rq_is_seq_zoned_write(rq))
pos = round_down(pos, rq->q->limits.chunk_sectors);
> @@ -821,7 +833,16 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
> * set expire time and add to fifo list
> */
> rq->fifo_time = jiffies + dd->fifo_expire[data_dir];
> - list_add_tail(&rq->queuelist, &per_prio->fifo_list[data_dir]);
> + insert_before = &per_prio->fifo_list[data_dir];
> +#ifdef CONFIG_BLK_DEV_ZONED
> + if (blk_rq_is_seq_zoned_write(rq)) {
> + struct request *rq2 = deadline_latter_request(rq);
> +
> + if (rq2 && blk_rq_zone_no(rq2) == blk_rq_zone_no(rq))
> + insert_before = &rq2->queuelist;
> + }
> +#endif
Why does this need an ifdef?
Also can you please always add comments for these special cases?
Same for the one above.
> + list_add_tail(&rq->queuelist, insert_before);
> }
> }
>
---end quoted text---
