On Mon, Oct 09, 2017 at 07:24:23PM +0800, Ming Lei wrote:
> SCSI devices use host-wide tagset, and the shared driver tag space is
> often quite big. Meantime there is also queue depth for each lun(
> .cmd_per_lun), which is often small, for example, on both lpfc and
> qla2xxx, .cmd_per_lun is just 3.
>
> So lots of requests may stay in sw queue, and we always flush all
> belonging to same hw queue and dispatch them all to driver, unfortunately
> it is easy to cause queue busy because of the small .cmd_per_lun.
> Once these requests are flushed out, they have to stay in hctx->dispatch,
> and no bio merge can participate into these requests, and sequential IO
> performance is hurt a lot.
>
> This patch introduces blk_mq_dequeue_from_ctx for dequeuing request from
> sw queue so that we can dispatch them in scheduler's way, then we can
> avoid to dequeue too many requests from sw queue when ->dispatch isn't
> flushed completely.
>
> This patch improves dispatching from sw queue when there is per-request-queue
> queue depth by taking request one by one from sw queue, just like the way
> of IO scheduler.
This still didn't address Jens' concern about using q->queue_depth as
the heuristic for whether to do the full sw queue flush or one-by-one
dispatch. The EWMA approach is a bit too complex for now, can you please
try the heuristic of whether the driver ever returned BLK_STS_RESOURCE?
> Reviewed-by: Omar Sandoval <osan...@fb.com>
> Reviewed-by: Bart Van Assche <bart.vanass...@wdc.com>
> Signed-off-by: Ming Lei <ming....@redhat.com>
> ---
> block/blk-mq-sched.c | 50
> ++++++++++++++++++++++++++++++++++++++++++++++++--
> block/blk-mq.c | 39 +++++++++++++++++++++++++++++++++++++++
> block/blk-mq.h | 2 ++
> include/linux/blk-mq.h | 2 ++
> 4 files changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index be29ba849408..14b354f617e5 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -104,6 +104,39 @@ static void blk_mq_do_dispatch_sched(struct
> blk_mq_hw_ctx *hctx)
> } while (blk_mq_dispatch_rq_list(q, &rq_list));
> }
>
> +static struct blk_mq_ctx *blk_mq_next_ctx(struct blk_mq_hw_ctx *hctx,
> + struct blk_mq_ctx *ctx)
> +{
> + unsigned idx = ctx->index_hw;
> +
> + if (++idx == hctx->nr_ctx)
> + idx = 0;
> +
> + return hctx->ctxs[idx];
> +}
> +
> +static void blk_mq_do_dispatch_ctx(struct blk_mq_hw_ctx *hctx)
> +{
> + struct request_queue *q = hctx->queue;
> + LIST_HEAD(rq_list);
> + struct blk_mq_ctx *ctx = READ_ONCE(hctx->dispatch_from);
> +
> + do {
> + struct request *rq;
> +
> + rq = blk_mq_dequeue_from_ctx(hctx, ctx);
> + if (!rq)
> + break;
> + list_add(&rq->queuelist, &rq_list);
> +
> + /* round robin for fair dispatch */
> + ctx = blk_mq_next_ctx(hctx, rq->mq_ctx);
> +
> + } while (blk_mq_dispatch_rq_list(q, &rq_list));
> +
> + WRITE_ONCE(hctx->dispatch_from, ctx);
> +}
> +
> void blk_mq_sched_dispatch_requests(struct blk_mq_hw_ctx *hctx)
> {
> struct request_queue *q = hctx->queue;
> @@ -143,10 +176,23 @@ void 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) && has_sched_dispatch)
> - blk_mq_do_dispatch_sched(hctx);
> + if (blk_mq_dispatch_rq_list(q, &rq_list)) {
> + if (has_sched_dispatch)
> + blk_mq_do_dispatch_sched(hctx);
> + else
> + blk_mq_do_dispatch_ctx(hctx);
> + }
> } else if (has_sched_dispatch) {
> blk_mq_do_dispatch_sched(hctx);
> + } else if (q->queue_depth) {
> + /*
> + * If there is per-request_queue depth, we dequeue
> + * request one by one from sw queue for avoiding to mess
> + * up I/O merge when dispatch runs out of resource, which
> + * can be triggered easily when there is per-request_queue
> + * queue depth or .cmd_per_lun, such as SCSI device.
> + */
> + blk_mq_do_dispatch_ctx(hctx);
> } else {
> blk_mq_flush_busy_ctxs(hctx, &rq_list);
> blk_mq_dispatch_rq_list(q, &rq_list);
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 076cbab9c3e0..394cb75d66fa 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -911,6 +911,45 @@ void blk_mq_flush_busy_ctxs(struct blk_mq_hw_ctx *hctx,
> struct list_head *list)
> }
> EXPORT_SYMBOL_GPL(blk_mq_flush_busy_ctxs);
>
> +struct dispatch_rq_data {
> + struct blk_mq_hw_ctx *hctx;
> + struct request *rq;
> +};
> +
> +static bool dispatch_rq_from_ctx(struct sbitmap *sb, unsigned int bitnr,
> + void *data)
> +{
> + struct dispatch_rq_data *dispatch_data = data;
> + struct blk_mq_hw_ctx *hctx = dispatch_data->hctx;
> + struct blk_mq_ctx *ctx = hctx->ctxs[bitnr];
> +
> + spin_lock(&ctx->lock);
> + if (unlikely(!list_empty(&ctx->rq_list))) {
> + dispatch_data->rq = list_entry_rq(ctx->rq_list.next);
> + list_del_init(&dispatch_data->rq->queuelist);
> + if (list_empty(&ctx->rq_list))
> + sbitmap_clear_bit(sb, bitnr);
> + }
> + spin_unlock(&ctx->lock);
> +
> + return !dispatch_data->rq;
> +}
> +
> +struct request *blk_mq_dequeue_from_ctx(struct blk_mq_hw_ctx *hctx,
> + struct blk_mq_ctx *start)
> +{
> + unsigned off = start ? start->index_hw : 0;
> + struct dispatch_rq_data data = {
> + .hctx = hctx,
> + .rq = NULL,
> + };
> +
> + __sbitmap_for_each_set(&hctx->ctx_map, off,
> + dispatch_rq_from_ctx, &data);
> +
> + return data.rq;
> +}
> +
> static inline unsigned int queued_to_index(unsigned int queued)
> {
> if (!queued)
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index ef15b3414da5..231cfb0d973b 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -35,6 +35,8 @@ void blk_mq_flush_busy_ctxs(struct blk_mq_hw_ctx *hctx,
> struct list_head *list);
> bool blk_mq_hctx_has_pending(struct blk_mq_hw_ctx *hctx);
> bool blk_mq_get_driver_tag(struct request *rq, struct blk_mq_hw_ctx **hctx,
> bool wait);
> +struct request *blk_mq_dequeue_from_ctx(struct blk_mq_hw_ctx *hctx,
> + struct blk_mq_ctx *start);
>
> /*
> * Internal helpers for allocating/freeing the request map
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 50c6485cb04f..7b7a366a97f3 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -30,6 +30,8 @@ struct blk_mq_hw_ctx {
>
> struct sbitmap ctx_map;
>
> + struct blk_mq_ctx *dispatch_from;
> +
> struct blk_mq_ctx **ctxs;
> unsigned int nr_ctx;
>
> --
> 2.9.5
>
