On Fri 17-03-23 12:59:38, Bart Van Assche wrote:
> Instead of submitting the bio fragment with the highest LBA first,
> submit the bio fragment with the lowest LBA first. If plugging is
> active, append requests at the end of the list with plugged requests
> instead of at the start. This approach prevents write errors when
> submitting large bios to host-managed zoned block devices.
>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Ming Lei <ming.lei@xxxxxxxxxx>
> Cc: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> Cc: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
...
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index cc32ad0cd548..9b0f9f3fdba0 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1300,7 +1300,7 @@ static inline unsigned short blk_plug_max_rq_count(struct blk_plug *plug)
>
> static void blk_add_rq_to_plug(struct blk_plug *plug, struct request *rq)
> {
> - struct request *last = rq_list_peek(&plug->mq_list);
> + struct request *last = rq_list_peek(&plug->mq_list), **last_p;
>
> if (!plug->rq_count) {
> trace_block_plug(rq->q);
> @@ -1317,7 +1317,10 @@ static void blk_add_rq_to_plug(struct blk_plug *plug, struct request *rq)
> if (!plug->has_elevator && (rq->rq_flags & RQF_ELV))
> plug->has_elevator = true;
> rq->rq_next = NULL;
> - rq_list_add(&plug->mq_list, rq);
> + last_p = &plug->mq_list;
> + while (*last_p)
> + last_p = &(*last_p)->rq_next;
> + rq_list_add_tail(&last_p, rq);
> plug->rq_count++;
> }
Uh, I don't think we want to traverse the whole plug list each time we are
adding a request to it. We have just recently managed to avoid that at
least for single-device cases and apparently it was a win for fast devices
(see commit d38a9c04c0d5 ("block: only check previous entry for plug merge
attempt")).
If anything, I'd rather modify blk_mq_dispatch_plug_list() to add requests
to the queuelist in reverse order in this case, like I did it in
34e0a279a993 ("block: do not reverse request order when flushing plug
list") which is now in Jens' tree.
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index dd5ce1137f04..5e01791967c0 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -228,6 +228,12 @@ static inline unsigned short req_get_ioprio(struct request *req)
> *(listptr) = rq; \
> } while (0)
>
> +#define rq_list_add_tail(lastpptr, rq) do { \
> + (rq)->rq_next = NULL; \
> + **(lastpptr) = rq; \
> + *(lastpptr) = &rq->rq_next; \
> +} while (0)
> +
And this function should be already in Jens's tree as part of commit
34e0a279a993 ("block: do not reverse request order when flushing plug
list").
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
