On 2020/06/02 18:15, Ming Lei wrote:
> All requests in the 'list' of blk_mq_dispatch_rq_list belong to same
> hctx, so it is better to pass hctx instead of request queue, because
> blk-mq's dispatch target is hctx instead of request queue.
>
> Cc: Sagi Grimberg <sagi@xxxxxxxxxxx>
> Cc: Baolin Wang <baolin.wang7@xxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Sagi Grimberg <sagi@xxxxxxxxxxx>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> Tested-by: Baolin Wang <baolin.wang7@xxxxxxxxx>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> block/blk-mq-sched.c | 14 ++++++--------
> block/blk-mq.c | 6 +++---
> block/blk-mq.h | 2 +-
> 3 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index a31e281e9d31..632c6f8b63f7 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -96,10 +96,9 @@ static int blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx *hctx)
> struct elevator_queue *e = q->elevator;
> LIST_HEAD(rq_list);
> int ret = 0;
> + struct request *rq;
>
> do {
> - struct request *rq;
> -
> if (e->type->ops.has_work && !e->type->ops.has_work(hctx))
> break;
>
> @@ -131,7 +130,7 @@ static int blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx *hctx)
> * in blk_mq_dispatch_rq_list().
> */
> list_add(&rq->queuelist, &rq_list);
> - } while (blk_mq_dispatch_rq_list(q, &rq_list, true));
> + } while (blk_mq_dispatch_rq_list(rq->mq_hctx, &rq_list, true));
Why not use the hctx argument passed to the function instead of rq->mq_hctx ?
>
> return ret;
> }
> @@ -161,10 +160,9 @@ static int blk_mq_do_dispatch_ctx(struct blk_mq_hw_ctx *hctx)
> LIST_HEAD(rq_list);
> struct blk_mq_ctx *ctx = READ_ONCE(hctx->dispatch_from);
> int ret = 0;
> + struct request *rq;
>
> do {
> - struct request *rq;
> -
> if (!list_empty_careful(&hctx->dispatch)) {
> ret = -EAGAIN;
> break;
> @@ -200,7 +198,7 @@ static int blk_mq_do_dispatch_ctx(struct blk_mq_hw_ctx *hctx)
> /* round robin for fair dispatch */
> ctx = blk_mq_next_ctx(hctx, rq->mq_ctx);
>
> - } while (blk_mq_dispatch_rq_list(q, &rq_list, true));
> + } while (blk_mq_dispatch_rq_list(rq->mq_hctx, &rq_list, true));
Same here.
>
> WRITE_ONCE(hctx->dispatch_from, ctx);
> return ret;
> @@ -240,7 +238,7 @@ static int __blk_mq_sched_dispatch_requests(struct blk_mq_hw_ctx *hctx)
> */
> if (!list_empty(&rq_list)) {
> blk_mq_sched_mark_restart_hctx(hctx);
> - if (blk_mq_dispatch_rq_list(q, &rq_list, false)) {
> + if (blk_mq_dispatch_rq_list(hctx, &rq_list, false)) {
> if (has_sched_dispatch)
> ret = blk_mq_do_dispatch_sched(hctx);
> else
> @@ -253,7 +251,7 @@ static int __blk_mq_sched_dispatch_requests(struct blk_mq_hw_ctx *hctx)
> ret = blk_mq_do_dispatch_ctx(hctx);
> } else {
> blk_mq_flush_busy_ctxs(hctx, &rq_list);
> - blk_mq_dispatch_rq_list(q, &rq_list, false);
> + blk_mq_dispatch_rq_list(hctx, &rq_list, false);
> }
>
> return ret;
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index bcbf49bd7ebe..723bc39507fe 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1236,10 +1236,10 @@ static void blk_mq_handle_zone_resource(struct request *rq,
> /*
> * Returns true if we did some work AND can potentially do more.
> */
> -bool blk_mq_dispatch_rq_list(struct request_queue *q, struct list_head *list,
> +bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
> bool got_budget)
> {
> - struct blk_mq_hw_ctx *hctx;
> + struct request_queue *q = hctx->queue;
> struct request *rq, *nxt;
> bool no_tag = false;
> int errors, queued;
> @@ -1261,7 +1261,7 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q, struct list_head *list,
>
> rq = list_first_entry(list, struct request, queuelist);
>
> - hctx = rq->mq_hctx;
> + WARN_ON_ONCE(hctx != rq->mq_hctx);
> if (!got_budget && !blk_mq_get_dispatch_budget(q)) {
> blk_mq_put_driver_tag(rq);
> no_budget_avail = true;
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index 21d877105224..d2d737b16e0e 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -40,7 +40,7 @@ struct blk_mq_ctx {
> void blk_mq_exit_queue(struct request_queue *q);
> int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr);
> void blk_mq_wake_waiters(struct request_queue *q);
> -bool blk_mq_dispatch_rq_list(struct request_queue *, struct list_head *, bool);
> +bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *, bool);
> void blk_mq_add_to_requeue_list(struct request *rq, bool at_head,
> bool kick_requeue_list);
> void blk_mq_flush_busy_ctxs(struct blk_mq_hw_ctx *hctx, struct list_head *list);
>
--
Damien Le Moal
Western Digital Research
