[V7,5/6] blk-mq-sched: improve dispatching from sw queue

Commit Message

Ming Lei Oct. 12, 2017, 6:37 p.m. UTC

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 by using the callback of
.get_budget and .put_budget

Reviewed-by: Omar Sandoval <[e-mail]>
Reviewed-by: Bart Van Assche <[e-mail]>
Signed-off-by: Ming Lei <[e-mail]>
---

Jens/Omar, once .get_budget is introduced, it can be unusual to
return busy from .queue_rq, so we can't use the approach discussed
to decide to take one by one or flush all. Then this patch
simply checks if .get_budget is implemented, looks it is reasonable,
and in future it is even possible to take more requests by getting
enough budget first.


 block/blk-mq-sched.c   | 63 +++++++++++++++++++++++++++++++++++++++++++++++---
 block/blk-mq.c         | 39 +++++++++++++++++++++++++++++++
 block/blk-mq.h         |  2 ++
 include/linux/blk-mq.h |  2 ++
 4 files changed, 103 insertions(+), 3 deletions(-)

Comments

Bart Van Assche Oct. 12, 2017, 6:48 p.m. UTC | #1

On 10/12/17 11:37, Ming Lei wrote:
> [ ... ]
> 
> This patch improves dispatching from sw queue by using the callback of
> .get_budget and .put_budget
> 
> Reviewed-by: Omar Sandoval <[e-mail]>
> Reviewed-by: Bart Van Assche <[e-mail]>
> Signed-off-by: Ming Lei <[e-mail]>
> ---
> 
> Jens/Omar, once .get_budget is introduced, it can be unusual to
> return busy from .queue_rq, so we can't use the approach discussed
> to decide to take one by one or flush all. Then this patch
> simply checks if .get_budget is implemented, looks it is reasonable,
> and in future it is even possible to take more requests by getting
> enough budget first.
> [ ... ]

Version 7 of this patch differs significantly from the version I 
reviewed so I think you should remove my Reviewed-by tag.

Bart.

Ming Lei Oct. 13, 2017, 12:20 a.m. UTC | #2

On Thu, Oct 12, 2017 at 11:48:22AM -0700, Bart Van Assche wrote:
> On 10/12/17 11:37, Ming Lei wrote:
> > [ ... ]
> > 
> > This patch improves dispatching from sw queue by using the callback of
> > .get_budget and .put_budget
> > 
> > Reviewed-by: Omar Sandoval <[e-mail]>
> > Reviewed-by: Bart Van Assche <[e-mail]>
> > Signed-off-by: Ming Lei <[e-mail]>
> > ---
> > 
> > Jens/Omar, once .get_budget is introduced, it can be unusual to
> > return busy from .queue_rq, so we can't use the approach discussed
> > to decide to take one by one or flush all. Then this patch
> > simply checks if .get_budget is implemented, looks it is reasonable,
> > and in future it is even possible to take more requests by getting
> > enough budget first.
> > [ ... ]
> 
> Version 7 of this patch differs significantly from the version I reviewed so
> I think you should remove my Reviewed-by tag.

OK, no problem, the only difference is on the introduced .get_budget().

Patch

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index bcf4773ee1ca..ccbbc7e108ea 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -117,6 +117,50 @@  static bool blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx *hctx)
 	return false;
 }
 
+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 bool 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;
+
+		if (!sbitmap_any_bit_set(&hctx->ctx_map))
+			break;
+
+		if (q->mq_ops->get_budget && !q->mq_ops->get_budget(hctx))
+			return true;
+
+		rq = blk_mq_dequeue_from_ctx(hctx, ctx);
+		if (!rq) {
+			if (q->mq_ops->put_budget)
+				q->mq_ops->put_budget(hctx);
+			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, true));
+
+	WRITE_ONCE(hctx->dispatch_from, ctx);
+
+	return false;
+}
+
 void blk_mq_sched_dispatch_requests(struct blk_mq_hw_ctx *hctx)
 {
 	struct request_queue *q = hctx->queue;
@@ -157,11 +201,24 @@  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, false) &&
-				has_sched_dispatch)
-			run_queue = blk_mq_do_dispatch_sched(hctx);
+		if (blk_mq_dispatch_rq_list(q, &rq_list, false)) {
+			if (has_sched_dispatch)
+				run_queue = blk_mq_do_dispatch_sched(hctx);
+			else
+				run_queue = blk_mq_do_dispatch_ctx(hctx);
+		}
 	} else if (has_sched_dispatch) {
 		run_queue = blk_mq_do_dispatch_sched(hctx);
+	} else if (q->mq_ops->get_budget) {
+		/*
+		 * If we need to get budget before queuing request, we
+		 * dequeue request one by one from sw queue for avoiding
+		 * to mess up I/O merge when dispatch runs out of resource.
+		 *
+		 * TODO: get more budgets, and dequeue more requests in
+		 * one time.
+		 */
+		run_queue = blk_mq_do_dispatch_ctx(hctx);
 	} else {
 		blk_mq_flush_busy_ctxs(hctx, &rq_list);
 		blk_mq_dispatch_rq_list(q, &rq_list, false);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index e6e50a91f170..9e2fc26a9018 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 d1bfce3e69ad..4d12ef08b0a9 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 f8c4ba2682ec..cc757dcc49e1 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;