[PATCH 0/9] blk-mq: support batching dispatch from scheduler

[PATCH 0/9] blk-mq: support batching dispatch from scheduler

[Ming Lei]

Hi Guys,

More and more drivers want to get batching requests queued from
block layer, such as mmc[1], and tcp based storage drivers[2]. Also
current in-tree users have virtio-scsi, virtio-blk and nvme.

For none, we already support batching dispatch.

But for io scheduler, every time we just take one request from scheduler
and pass the single request to blk_mq_dispatch_rq_list(). This way makes
batching dispatch not possible when io scheduler is applied. One reason
is that we don't want to hurt sequential IO performance, becasue IO
merge chance is reduced if more requests are dequeued from scheduler
queue.

Tries to start the support by dequeuing more requests from scheduler
if budget is enough and device isn't busy.

Simple fio test over virtio-scsi shows IO can get improved by 5~10%.

Patches can be found from the following tree too:

	https://github.com/ming1/linux/commits/v5.7-rc-blk-mq-batching-submission

Patch 1 ~ 7 are improvement and cleanup, which can't applied without
supporting batching dispatch.

Patch 8 ~ 9 starts to support batching dispatch from scheduler.


Please review and comment!


[1] https://lore.kernel.org/linux-block/20200512075501.GF1531898@T590/#r
[2] https://lore.kernel.org/linux-block/fe6bd8b9-6ed9-b225-f80c-314746133722@grimberg.me/


Ming Lei (9):
  blk-mq: pass request queue into get/put budget callback
  blk-mq: pass hctx to blk_mq_dispatch_rq_list
  blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list
  blk-mq: move getting driver tag and bugget into one helper
  blk-mq: move .queue_rq code into one helper
  blk-mq: move code for handling partial dispatch into one helper
  blk-mq: remove dead check from blk_mq_dispatch_rq_list
  blk-mq: pass obtained budget count to blk_mq_dispatch_rq_list
  blk-mq: support batching dispatch in case of io scheduler

 block/blk-mq-sched.c    |  96 ++++++++++++++--
 block/blk-mq.c          | 248 +++++++++++++++++++++-------------------
 block/blk-mq.h          |  15 +--
 drivers/scsi/scsi_lib.c |   8 +-
 include/linux/blk-mq.h  |   4 +-
 5 files changed, 226 insertions(+), 145 deletions(-)

Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>

-- 
2.25.2

[PATCH 1/9] blk-mq: pass request queue into get/put budget callback

[Ming Lei]

blk-mq budget is abstract from scsi's device queue depth, and it is
always per-request-queue instead of hctx.

It can be quite absurd to get a budget from one hctx, then dequeue a
request from scheduler queue, and this request may not belong to this
hctx, at least for bfq and deadline.

So fix the mess and always pass request queue to get/put budget
callback.

Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq-sched.c    |  8 ++++----
 block/blk-mq.c          |  8 ++++----
 block/blk-mq.h          | 12 ++++--------
 drivers/scsi/scsi_lib.c |  8 +++-----
 include/linux/blk-mq.h  |  4 ++--
 5 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index fdcc2c1dd178..a31e281e9d31 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -108,12 +108,12 @@ static int blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx *hctx)
 			break;
 		}
 
-		if (!blk_mq_get_dispatch_budget(hctx))
+		if (!blk_mq_get_dispatch_budget(q))
 			break;
 
 		rq = e->type->ops.dispatch_request(hctx);
 		if (!rq) {
-			blk_mq_put_dispatch_budget(hctx);
+			blk_mq_put_dispatch_budget(q);
 			/*
 			 * We're releasing without dispatching. Holding the
 			 * budget could have blocked any "hctx"s with the
@@ -173,12 +173,12 @@ static int blk_mq_do_dispatch_ctx(struct blk_mq_hw_ctx *hctx)
 		if (!sbitmap_any_bit_set(&hctx->ctx_map))
 			break;
 
-		if (!blk_mq_get_dispatch_budget(hctx))
+		if (!blk_mq_get_dispatch_budget(q))
 			break;
 
 		rq = blk_mq_dequeue_from_ctx(hctx, ctx);
 		if (!rq) {
-			blk_mq_put_dispatch_budget(hctx);
+			blk_mq_put_dispatch_budget(q);
 			/*
 			 * We're releasing without dispatching. Holding the
 			 * budget could have blocked any "hctx"s with the
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9ee695bdf873..74ecaffd2d64 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1225,7 +1225,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;
-		if (!got_budget && !blk_mq_get_dispatch_budget(hctx)) {
+		if (!got_budget && !blk_mq_get_dispatch_budget(q)) {
 			blk_mq_put_driver_tag(rq);
 			no_budget_avail = true;
 			break;
@@ -1240,7 +1240,7 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q, struct list_head *list,
 			 * we'll re-run it below.
 			 */
 			if (!blk_mq_mark_tag_wait(hctx, rq)) {
-				blk_mq_put_dispatch_budget(hctx);
+				blk_mq_put_dispatch_budget(q);
 				/*
 				 * For non-shared tags, the RESTART check
 				 * will suffice.
@@ -1887,11 +1887,11 @@ static blk_status_t __blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
 	if (q->elevator && !bypass_insert)
 		goto insert;
 
-	if (!blk_mq_get_dispatch_budget(hctx))
+	if (!blk_mq_get_dispatch_budget(q))
 		goto insert;
 
 	if (!blk_mq_get_driver_tag(rq)) {
-		blk_mq_put_dispatch_budget(hctx);
+		blk_mq_put_dispatch_budget(q);
 		goto insert;
 	}
 
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 10bfdfb494fa..9540770de9dc 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -180,20 +180,16 @@ unsigned int blk_mq_in_flight(struct request_queue *q, struct hd_struct *part);
 void blk_mq_in_flight_rw(struct request_queue *q, struct hd_struct *part,
 			 unsigned int inflight[2]);
 
-static inline void blk_mq_put_dispatch_budget(struct blk_mq_hw_ctx *hctx)
+static inline void blk_mq_put_dispatch_budget(struct request_queue *q)
 {
-	struct request_queue *q = hctx->queue;
-
 	if (q->mq_ops->put_budget)
-		q->mq_ops->put_budget(hctx);
+		q->mq_ops->put_budget(q);
 }
 
-static inline bool blk_mq_get_dispatch_budget(struct blk_mq_hw_ctx *hctx)
+static inline bool blk_mq_get_dispatch_budget(struct request_queue *q)
 {
-	struct request_queue *q = hctx->queue;
-
 	if (q->mq_ops->get_budget)
-		return q->mq_ops->get_budget(hctx);
+		return q->mq_ops->get_budget(q);
 	return true;
 }
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 82ad0244b3d0..b9adee0a9266 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1624,17 +1624,15 @@ static void scsi_mq_done(struct scsi_cmnd *cmd)
 		clear_bit(SCMD_STATE_COMPLETE, &cmd->state);
 }
 
-static void scsi_mq_put_budget(struct blk_mq_hw_ctx *hctx)
+static void scsi_mq_put_budget(struct request_queue *q)
 {
-	struct request_queue *q = hctx->queue;
 	struct scsi_device *sdev = q->queuedata;
 
 	atomic_dec(&sdev->device_busy);
 }
 
-static bool scsi_mq_get_budget(struct blk_mq_hw_ctx *hctx)
+static bool scsi_mq_get_budget(struct request_queue *q)
 {
-	struct request_queue *q = hctx->queue;
 	struct scsi_device *sdev = q->queuedata;
 
 	return scsi_dev_queue_ready(q, sdev);
@@ -1701,7 +1699,7 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
 	if (scsi_target(sdev)->can_queue > 0)
 		atomic_dec(&scsi_target(sdev)->target_busy);
 out_put_budget:
-	scsi_mq_put_budget(hctx);
+	scsi_mq_put_budget(q);
 	switch (ret) {
 	case BLK_STS_OK:
 		break;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index d7307795439a..43e38d21ca4a 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -268,8 +268,8 @@ struct blk_mq_queue_data {
 typedef blk_status_t (queue_rq_fn)(struct blk_mq_hw_ctx *,
 		const struct blk_mq_queue_data *);
 typedef void (commit_rqs_fn)(struct blk_mq_hw_ctx *);
-typedef bool (get_budget_fn)(struct blk_mq_hw_ctx *);
-typedef void (put_budget_fn)(struct blk_mq_hw_ctx *);
+typedef bool (get_budget_fn)(struct request_queue *);
+typedef void (put_budget_fn)(struct request_queue *);
 typedef enum blk_eh_timer_return (timeout_fn)(struct request *, bool);
 typedef int (init_hctx_fn)(struct blk_mq_hw_ctx *, void *, unsigned int);
 typedef void (exit_hctx_fn)(struct blk_mq_hw_ctx *, unsigned int);
-- 
2.25.2

Re: [PATCH 1/9] blk-mq: pass request queue into get/put budget callback

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 1/9] blk-mq: pass request queue into get/put budget callback

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/9] blk-mq: pass request queue into get/put budget callback

[Doug Anderson]

Hi,

On Wed, May 13, 2020 at 2:55 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> blk-mq budget is abstract from scsi's device queue depth, and it is
> always per-request-queue instead of hctx.
>
> It can be quite absurd to get a budget from one hctx, then dequeue a
> request from scheduler queue, and this request may not belong to this
> hctx, at least for bfq and deadline.
>
> So fix the mess and always pass request queue to get/put budget
> callback.
>
> Cc: Sagi Grimberg <sagi@grimberg.me>
> Cc: Baolin Wang <baolin.wang7@gmail.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  block/blk-mq-sched.c    |  8 ++++----
>  block/blk-mq.c          |  8 ++++----
>  block/blk-mq.h          | 12 ++++--------
>  drivers/scsi/scsi_lib.c |  8 +++-----
>  include/linux/blk-mq.h  |  4 ++--
>  5 files changed, 17 insertions(+), 23 deletions(-)

Seems sane to me and probably would have helped me understand this
code a little faster in the past.  ;-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH 1/9] blk-mq: pass request queue into get/put budget callback

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH 2/9] blk-mq: pass hctx to blk_mq_dispatch_rq_list

[Ming Lei]

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@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 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));
 
 	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));
 
 	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 74ecaffd2d64..f064e7923ea5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1199,10 +1199,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;
@@ -1224,7 +1224,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 9540770de9dc..9c0e93d4fe38 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);
-- 
2.25.2

Re: [PATCH 2/9] blk-mq: pass hctx to blk_mq_dispatch_rq_list

[Christoph Hellwig]

On Wed, May 13, 2020 at 05:54:36PM +0800, 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.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/9] blk-mq: pass hctx to blk_mq_dispatch_rq_list

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list

[Ming Lei]

.commit_rqs() is supposed to handle partial dispatch when driver may not
see .last of flag passed to .queue_rq().

We have added .commit_rqs() in case of partial dispatch and all consumers
of bd->last have implemented .commit_rqs() callback, so it is perfect to
pass real .last flag of the request list to .queue_rq() instead of faking
it by trying to allocate driver tag for next request in the batching list.

Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index f064e7923ea5..f8c4b59022d7 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1169,16 +1169,6 @@ static void blk_mq_update_dispatch_busy(struct blk_mq_hw_ctx *hctx, bool busy)
 static void blk_mq_handle_dev_resource(struct request *rq,
 				       struct list_head *list)
 {
-	struct request *next =
-		list_first_entry_or_null(list, struct request, queuelist);
-
-	/*
-	 * If an I/O scheduler has been configured and we got a driver tag for
-	 * the next request already, free it.
-	 */
-	if (next)
-		blk_mq_put_driver_tag(next);
-
 	list_add(&rq->queuelist, list);
 	__blk_mq_requeue_request(rq);
 }
@@ -1203,7 +1193,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 			     bool got_budget)
 {
 	struct request_queue *q = hctx->queue;
-	struct request *rq, *nxt;
+	struct request *rq;
 	bool no_tag = false;
 	int errors, queued;
 	blk_status_t ret = BLK_STS_OK;
@@ -1254,17 +1244,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 		list_del_init(&rq->queuelist);
 
 		bd.rq = rq;
-
-		/*
-		 * Flag last if we have no more requests, or if we have more
-		 * but can't assign a driver tag to it.
-		 */
-		if (list_empty(list))
-			bd.last = true;
-		else {
-			nxt = list_first_entry(list, struct request, queuelist);
-			bd.last = !blk_mq_get_driver_tag(nxt);
-		}
+		bd.last = !!list_empty(list);
 
 		ret = q->mq_ops->queue_rq(hctx, &bd);
 		if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE) {
-- 
2.25.2

Re: [PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list

[Christoph Hellwig]

On Wed, May 13, 2020 at 05:54:37PM +0800, Ming Lei wrote:
> .commit_rqs() is supposed to handle partial dispatch when driver may not
> see .last of flag passed to .queue_rq().
> 
> We have added .commit_rqs() in case of partial dispatch and all consumers
> of bd->last have implemented .commit_rqs() callback, so it is perfect to
> pass real .last flag of the request list to .queue_rq() instead of faking
> it by trying to allocate driver tag for next request in the batching list.

The current case still seems like a nice optimization to avoid an extra
indirect function call.  So if you want to get rid of it I think it at
least needs a better rationale.

Re: [PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list

[Ming Lei]

On Wed, May 13, 2020 at 05:27:53AM -0700, Christoph Hellwig wrote:
> On Wed, May 13, 2020 at 05:54:37PM +0800, Ming Lei wrote:
> > .commit_rqs() is supposed to handle partial dispatch when driver may not
> > see .last of flag passed to .queue_rq().
> > 
> > We have added .commit_rqs() in case of partial dispatch and all consumers
> > of bd->last have implemented .commit_rqs() callback, so it is perfect to
> > pass real .last flag of the request list to .queue_rq() instead of faking
> > it by trying to allocate driver tag for next request in the batching list.
> 
> The current case still seems like a nice optimization to avoid an extra
> indirect function call.  So if you want to get rid of it I think it at
> least needs a better rationale.

You mean marking .last by trying to allocate for next request can
replace .commit_rqs()? No, it can't because .commit_rqs() can be
called no matter .last is set or not, both two are independent.

Removing it can avoid to pre-allocate one extra driver tag, and
improve driver tag's utilization.

Thanks,
Ming

Re: [PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list

[Christoph Hellwig]

On Thu, May 14, 2020 at 08:50:43AM +0800, Ming Lei wrote:
> On Wed, May 13, 2020 at 05:27:53AM -0700, Christoph Hellwig wrote:
> > On Wed, May 13, 2020 at 05:54:37PM +0800, Ming Lei wrote:
> > > .commit_rqs() is supposed to handle partial dispatch when driver may not
> > > see .last of flag passed to .queue_rq().
> > > 
> > > We have added .commit_rqs() in case of partial dispatch and all consumers
> > > of bd->last have implemented .commit_rqs() callback, so it is perfect to
> > > pass real .last flag of the request list to .queue_rq() instead of faking
> > > it by trying to allocate driver tag for next request in the batching list.
> > 
> > The current case still seems like a nice optimization to avoid an extra
> > indirect function call.  So if you want to get rid of it I think it at
> > least needs a better rationale.
> 
> You mean marking .last by trying to allocate for next request can
> replace .commit_rqs()? No, it can't because .commit_rqs() can be
> called no matter .last is set or not, both two are independent.
> 
> Removing it can avoid to pre-allocate one extra driver tag, and
> improve driver tag's utilization.

What I said is that the current scheme works, and the new one will
need an additional indirect function call in various scenarios.  The
commit log doesn't really "sell" that change very well.  Your new
explanation is much better, as would be saying it helps you with
the hanges in this series.

Re: [PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list

[Ming Lei]

On Wed, May 13, 2020 at 05:27:53AM -0700, Christoph Hellwig wrote:
> On Wed, May 13, 2020 at 05:54:37PM +0800, Ming Lei wrote:
> > .commit_rqs() is supposed to handle partial dispatch when driver may not
> > see .last of flag passed to .queue_rq().
> > 
> > We have added .commit_rqs() in case of partial dispatch and all consumers
> > of bd->last have implemented .commit_rqs() callback, so it is perfect to
> > pass real .last flag of the request list to .queue_rq() instead of faking
> > it by trying to allocate driver tag for next request in the batching list.
> 
> The current case still seems like a nice optimization to avoid an extra
> indirect function call.  So if you want to get rid of it I think it at
> least needs a better rationale.

Forget to mention, trying to predicate the last request via allocating
tag for next request can't avoid extra .commit_rqs() because this
indirect call is always called when the rq list isn't done.

Also no matter .last is set or not, every implementation of .commit_rqs
always grabs one lock, so looks this patch can get real win without any
performance loss.

On the other side, .commit_rqs() can be avoided iff the last queued(successful)
rq is marked as .last, and the cost is to keep current estimate on .last.
However, why is .commit_rqs() required? Why doesn't .queue_rq() handle the batching
submission before non-STS_OK is returned? And the inline handling can be quite
efficient because one more spin lock acquire can be avoided usually. Then
.commit_rqs() can be killed.


Thanks,
Ming

Re: [PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list

[Ming Lei]

On Thu, May 14, 2020 at 10:09:55AM +0800, Ming Lei wrote:
> On Wed, May 13, 2020 at 05:27:53AM -0700, Christoph Hellwig wrote:
> > On Wed, May 13, 2020 at 05:54:37PM +0800, Ming Lei wrote:
> > > .commit_rqs() is supposed to handle partial dispatch when driver may not
> > > see .last of flag passed to .queue_rq().
> > > 
> > > We have added .commit_rqs() in case of partial dispatch and all consumers
> > > of bd->last have implemented .commit_rqs() callback, so it is perfect to
> > > pass real .last flag of the request list to .queue_rq() instead of faking
> > > it by trying to allocate driver tag for next request in the batching list.
> > 
> > The current case still seems like a nice optimization to avoid an extra
> > indirect function call.  So if you want to get rid of it I think it at
> > least needs a better rationale.
> 
> Forget to mention, trying to predicate the last request via allocating
> tag for next request can't avoid extra .commit_rqs() because this
> indirect call is always called when the rq list isn't done.
> 
> Also no matter .last is set or not, every implementation of .commit_rqs
> always grabs one lock, so looks this patch can get real win without any
> performance loss.
> 
> On the other side, .commit_rqs() can be avoided iff the last queued(successful)
> rq is marked as .last, and the cost is to keep current estimate on .last.
> However, why is .commit_rqs() required? Why doesn't .queue_rq() handle the batching
> submission before non-STS_OK is returned? And the inline handling can be quite
> efficient because one more spin lock acquire can be avoided usually. Then
> .commit_rqs() can be killed.

The only chance we need .commit_rqs() should be:

- requests are queued successfully, and the last queued rq isn't marked
  as last
- running out of budget or driver tag before queueing one new request

I think we need to document the interfaces(.commit_rqs & .queue_rq) clearly.

thanks,
Ming

Re: [PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list

[Keith Busch]

On Thu, May 14, 2020 at 10:09:55AM +0800, Ming Lei wrote:
> However, why is .commit_rqs() required? Why doesn't .queue_rq() handle the batching
> submission before non-STS_OK is returned?

Wouldn't the driver need to know that the request is !first in that case
so that it doesn't commit nothing if the first request fails?

Re: [PATCH 3/9] blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list

[Ming Lei]

On Thu, May 14, 2020 at 12:21:43PM +0900, Keith Busch wrote:
> On Thu, May 14, 2020 at 10:09:55AM +0800, Ming Lei wrote:
> > However, why is .commit_rqs() required? Why doesn't .queue_rq() handle the batching
> > submission before non-STS_OK is returned?
> 
> Wouldn't the driver need to know that the request is !first in that case
> so that it doesn't commit nothing if the first request fails?

Yeah, I have replied on this question, :-)

In short, I plan to make the interface explicit and more efficient:

1) .commit_rqs() is used for notifying driver that one batching requests
are done, which is usually caused by lacking of resource for queuing more
requests to driver

2) one request with .last flag marks end of one batching submission, so
.commit_rq() will not be called if no new request with !.last is queued

3) it is driver's responsibility for handling batching submission if non
STS_OK is returned from .queue_rq() to block layer.

Then we can minimize .commit_rqs() uses.

Thanks,
Ming

[PATCH 4/9] blk-mq: move getting driver tag and bugget into one helper

[Ming Lei]

Move code for getting driver tag and bugget into one helper, so
blk_mq_dispatch_rq_list gets a bit simpified, and easier to read.

Meantime move updating of 'no_tag' and 'no_budget_avaiable' into
the branch for handling partial dispatch because that is exactly
consumer of the two local variables.

Also rename the parameter of 'got_budget' as 'ask_budget'.

No functional change.

Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 75 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 49 insertions(+), 26 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index f8c4b59022d7..c06421faa555 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1186,18 +1186,51 @@ static void blk_mq_handle_zone_resource(struct request *rq,
 	__blk_mq_requeue_request(rq);
 }
 
+enum prep_dispatch {
+	PREP_DISPATCH_OK,
+	PREP_DISPATCH_NO_TAG,
+	PREP_DISPATCH_NO_BUDGET,
+};
+
+static enum prep_dispatch blk_mq_prep_dispatch_rq(struct request *rq,
+						  bool ask_budget)
+{
+	struct blk_mq_hw_ctx *hctx = rq->mq_hctx;
+
+	if (ask_budget && !blk_mq_get_dispatch_budget(rq->q)) {
+		blk_mq_put_driver_tag(rq);
+		return PREP_DISPATCH_NO_BUDGET;
+	}
+
+	if (!blk_mq_get_driver_tag(rq)) {
+		/*
+		 * The initial allocation attempt failed, so we need to
+		 * rerun the hardware queue when a tag is freed. The
+		 * waitqueue takes care of that. If the queue is run
+		 * before we add this entry back on the dispatch list,
+		 * we'll re-run it below.
+		 */
+		if (!blk_mq_mark_tag_wait(hctx, rq)) {
+			/* budget is always obtained before getting tag */
+			blk_mq_put_dispatch_budget(rq->q);
+			return PREP_DISPATCH_NO_TAG;
+		}
+	}
+
+	return PREP_DISPATCH_OK;
+}
+
 /*
  * Returns true if we did some work AND can potentially do more.
  */
 bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 			     bool got_budget)
 {
+	enum prep_dispatch prep;
 	struct request_queue *q = hctx->queue;
 	struct request *rq;
-	bool no_tag = false;
 	int errors, queued;
 	blk_status_t ret = BLK_STS_OK;
-	bool no_budget_avail = false;
 	LIST_HEAD(zone_list);
 
 	if (list_empty(list))
@@ -1215,31 +1248,9 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 		rq = list_first_entry(list, struct request, queuelist);
 
 		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;
+		prep = blk_mq_prep_dispatch_rq(rq, !got_budget);
+		if (prep != PREP_DISPATCH_OK)
 			break;
-		}
-
-		if (!blk_mq_get_driver_tag(rq)) {
-			/*
-			 * The initial allocation attempt failed, so we need to
-			 * rerun the hardware queue when a tag is freed. The
-			 * waitqueue takes care of that. If the queue is run
-			 * before we add this entry back on the dispatch list,
-			 * we'll re-run it below.
-			 */
-			if (!blk_mq_mark_tag_wait(hctx, rq)) {
-				blk_mq_put_dispatch_budget(q);
-				/*
-				 * For non-shared tags, the RESTART check
-				 * will suffice.
-				 */
-				if (hctx->flags & BLK_MQ_F_TAG_SHARED)
-					no_tag = true;
-				break;
-			}
-		}
 
 		list_del_init(&rq->queuelist);
 
@@ -1282,6 +1293,18 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 	 */
 	if (!list_empty(list)) {
 		bool needs_restart;
+		bool no_tag = false;
+		bool no_budget_avail = false;
+
+		/*
+		 * For non-shared tags, the RESTART check
+		 * will suffice.
+		 */
+		if (prep == PREP_DISPATCH_NO_TAG &&
+				(hctx->flags & BLK_MQ_F_TAG_SHARED))
+			no_tag = true;
+		if (prep == PREP_DISPATCH_NO_BUDGET)
+			no_budget_avail = true;
 
 		/*
 		 * If we didn't flush the entire list, we could have told
-- 
2.25.2

Re: [PATCH 4/9] blk-mq: move getting driver tag and bugget into one helper

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/9] blk-mq: move getting driver tag and bugget into one helper

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH 5/9] blk-mq: move .queue_rq code into one helper

[Ming Lei]

Move code for queueing rq into one helper, so that blk_mq_dispatch_rq_list
gets a bit simpified, and easier to read.

No functional change.

Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index c06421faa555..34fd09adb7fc 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1220,6 +1220,17 @@ static enum prep_dispatch blk_mq_prep_dispatch_rq(struct request *rq,
 	return PREP_DISPATCH_OK;
 }
 
+static blk_status_t blk_mq_dispatch_rq(struct request *rq, bool is_last)
+{
+	struct blk_mq_queue_data bd;
+
+	list_del_init(&rq->queuelist);
+	bd.rq = rq;
+	bd.last = is_last;
+
+	return rq->q->mq_ops->queue_rq(rq->mq_hctx, &bd);
+}
+
 /*
  * Returns true if we did some work AND can potentially do more.
  */
@@ -1243,8 +1254,6 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 	 */
 	errors = queued = 0;
 	do {
-		struct blk_mq_queue_data bd;
-
 		rq = list_first_entry(list, struct request, queuelist);
 
 		WARN_ON_ONCE(hctx != rq->mq_hctx);
@@ -1252,12 +1261,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 		if (prep != PREP_DISPATCH_OK)
 			break;
 
-		list_del_init(&rq->queuelist);
-
-		bd.rq = rq;
-		bd.last = !!list_empty(list);
-
-		ret = q->mq_ops->queue_rq(hctx, &bd);
+		ret = blk_mq_dispatch_rq(rq, list_is_singular(list));
 		if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE) {
 			blk_mq_handle_dev_resource(rq, list);
 			break;
-- 
2.25.2

Re: [PATCH 5/9] blk-mq: move .queue_rq code into one helper

[Christoph Hellwig]

On Wed, May 13, 2020 at 05:54:39PM +0800, Ming Lei wrote:
> Move code for queueing rq into one helper, so that blk_mq_dispatch_rq_list
> gets a bit simpified, and easier to read.
> 
> No functional change.

I don't see much benefit.  Especially moving the list_del out of the
main function doesn't really help readability.

[PATCH 6/9] blk-mq: move code for handling partial dispatch into one helper

[Ming Lei]

Move code for handling partial dispatch into one helper, so that
blk_mq_dispatch_rq_list gets a bit simpified, and easier to read.

No functional change.

Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 126 ++++++++++++++++++++++++++-----------------------
 1 file changed, 66 insertions(+), 60 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 34fd09adb7fc..86beb8c66868 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1231,6 +1231,71 @@ static blk_status_t blk_mq_dispatch_rq(struct request *rq, bool is_last)
 	return rq->q->mq_ops->queue_rq(rq->mq_hctx, &bd);
 }
 
+static void blk_mq_handle_partial_dispatch(struct blk_mq_hw_ctx *hctx,
+		struct list_head *list, enum prep_dispatch prep,
+		blk_status_t ret, bool queued)
+{
+	struct request_queue *q = hctx->queue;
+	bool needs_restart;
+	bool no_tag = false;
+	bool no_budget_avail = false;
+
+	/*
+	 * For non-shared tags, the RESTART check
+	 * will suffice.
+	 */
+	if (prep == PREP_DISPATCH_NO_TAG &&
+			(hctx->flags & BLK_MQ_F_TAG_SHARED))
+		no_tag = true;
+	if (prep == PREP_DISPATCH_NO_BUDGET)
+		no_budget_avail = true;
+
+	/*
+	 * If we didn't flush the entire list, we could have told
+	 * the driver there was more coming, but that turned out to
+	 * be a lie.
+	 */
+	if (q->mq_ops->commit_rqs && queued)
+		q->mq_ops->commit_rqs(hctx);
+
+	spin_lock(&hctx->lock);
+	list_splice_tail_init(list, &hctx->dispatch);
+	spin_unlock(&hctx->lock);
+
+	/*
+	 * If SCHED_RESTART was set by the caller of this function and
+	 * it is no longer set that means that it was cleared by another
+	 * thread and hence that a queue rerun is needed.
+	 *
+	 * If 'no_tag' is set, that means that we failed getting
+	 * a driver tag with an I/O scheduler attached. If our dispatch
+	 * waitqueue is no longer active, ensure that we run the queue
+	 * AFTER adding our entries back to the list.
+	 *
+	 * If no I/O scheduler has been configured it is possible that
+	 * the hardware queue got stopped and restarted before requests
+	 * were pushed back onto the dispatch list. Rerun the queue to
+	 * avoid starvation. Notes:
+	 * - blk_mq_run_hw_queue() checks whether or not a queue has
+	 *   been stopped before rerunning a queue.
+	 * - Some but not all block drivers stop a queue before
+	 *   returning BLK_STS_RESOURCE. Two exceptions are scsi-mq
+	 *   and dm-rq.
+	 *
+	 * If driver returns BLK_STS_RESOURCE and SCHED_RESTART
+	 * bit is set, run queue after a delay to avoid IO stalls
+	 * that could otherwise occur if the queue is idle.  We'll do
+	 * similar if we couldn't get budget and SCHED_RESTART is set.
+	 */
+	needs_restart = blk_mq_sched_needs_restart(hctx);
+	if (!needs_restart ||
+	    (no_tag && list_empty_careful(&hctx->dispatch_wait.entry)))
+		blk_mq_run_hw_queue(hctx, true);
+	else if (needs_restart && (ret == BLK_STS_RESOURCE ||
+				   no_budget_avail))
+		blk_mq_delay_run_hw_queue(hctx, BLK_MQ_RESOURCE_DELAY);
+}
+
 /*
  * Returns true if we did some work AND can potentially do more.
  */
@@ -1238,7 +1303,6 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 			     bool got_budget)
 {
 	enum prep_dispatch prep;
-	struct request_queue *q = hctx->queue;
 	struct request *rq;
 	int errors, queued;
 	blk_status_t ret = BLK_STS_OK;
@@ -1296,65 +1360,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 	 * that is where we will continue on next queue run.
 	 */
 	if (!list_empty(list)) {
-		bool needs_restart;
-		bool no_tag = false;
-		bool no_budget_avail = false;
-
-		/*
-		 * For non-shared tags, the RESTART check
-		 * will suffice.
-		 */
-		if (prep == PREP_DISPATCH_NO_TAG &&
-				(hctx->flags & BLK_MQ_F_TAG_SHARED))
-			no_tag = true;
-		if (prep == PREP_DISPATCH_NO_BUDGET)
-			no_budget_avail = true;
-
-		/*
-		 * If we didn't flush the entire list, we could have told
-		 * the driver there was more coming, but that turned out to
-		 * be a lie.
-		 */
-		if (q->mq_ops->commit_rqs && queued)
-			q->mq_ops->commit_rqs(hctx);
-
-		spin_lock(&hctx->lock);
-		list_splice_tail_init(list, &hctx->dispatch);
-		spin_unlock(&hctx->lock);
-
-		/*
-		 * If SCHED_RESTART was set by the caller of this function and
-		 * it is no longer set that means that it was cleared by another
-		 * thread and hence that a queue rerun is needed.
-		 *
-		 * If 'no_tag' is set, that means that we failed getting
-		 * a driver tag with an I/O scheduler attached. If our dispatch
-		 * waitqueue is no longer active, ensure that we run the queue
-		 * AFTER adding our entries back to the list.
-		 *
-		 * If no I/O scheduler has been configured it is possible that
-		 * the hardware queue got stopped and restarted before requests
-		 * were pushed back onto the dispatch list. Rerun the queue to
-		 * avoid starvation. Notes:
-		 * - blk_mq_run_hw_queue() checks whether or not a queue has
-		 *   been stopped before rerunning a queue.
-		 * - Some but not all block drivers stop a queue before
-		 *   returning BLK_STS_RESOURCE. Two exceptions are scsi-mq
-		 *   and dm-rq.
-		 *
-		 * If driver returns BLK_STS_RESOURCE and SCHED_RESTART
-		 * bit is set, run queue after a delay to avoid IO stalls
-		 * that could otherwise occur if the queue is idle.  We'll do
-		 * similar if we couldn't get budget and SCHED_RESTART is set.
-		 */
-		needs_restart = blk_mq_sched_needs_restart(hctx);
-		if (!needs_restart ||
-		    (no_tag && list_empty_careful(&hctx->dispatch_wait.entry)))
-			blk_mq_run_hw_queue(hctx, true);
-		else if (needs_restart && (ret == BLK_STS_RESOURCE ||
-					   no_budget_avail))
-			blk_mq_delay_run_hw_queue(hctx, BLK_MQ_RESOURCE_DELAY);
-
+		blk_mq_handle_partial_dispatch(hctx, list, prep, ret, !!queued);
 		blk_mq_update_dispatch_busy(hctx, true);
 		return false;
 	} else
-- 
2.25.2

Re: [PATCH 6/9] blk-mq: move code for handling partial dispatch into one helper

[Christoph Hellwig]

On Wed, May 13, 2020 at 05:54:40PM +0800, Ming Lei wrote:
> Move code for handling partial dispatch into one helper, so that
> blk_mq_dispatch_rq_list gets a bit simpified, and easier to read.
> 
> No functional change.

The concept looks good, but some of the logic is very convoluted.
What do you think of something like this on top:


diff --git a/block/blk-mq.c b/block/blk-mq.c
index 86beb8c668689..8c9a6a886919c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1236,24 +1236,11 @@ static void blk_mq_handle_partial_dispatch(struct blk_mq_hw_ctx *hctx,
 		blk_status_t ret, bool queued)
 {
 	struct request_queue *q = hctx->queue;
-	bool needs_restart;
-	bool no_tag = false;
 	bool no_budget_avail = false;
 
 	/*
-	 * For non-shared tags, the RESTART check
-	 * will suffice.
-	 */
-	if (prep == PREP_DISPATCH_NO_TAG &&
-			(hctx->flags & BLK_MQ_F_TAG_SHARED))
-		no_tag = true;
-	if (prep == PREP_DISPATCH_NO_BUDGET)
-		no_budget_avail = true;
-
-	/*
-	 * If we didn't flush the entire list, we could have told
-	 * the driver there was more coming, but that turned out to
-	 * be a lie.
+	 * Commit the current batch.  There are more waiting requests, but we
+	 * can't guarantee that we'll handle them ASAP.
 	 */
 	if (q->mq_ops->commit_rqs && queued)
 		q->mq_ops->commit_rqs(hctx);
@@ -1263,36 +1250,52 @@ static void blk_mq_handle_partial_dispatch(struct blk_mq_hw_ctx *hctx,
 	spin_unlock(&hctx->lock);
 
 	/*
-	 * If SCHED_RESTART was set by the caller of this function and
-	 * it is no longer set that means that it was cleared by another
-	 * thread and hence that a queue rerun is needed.
+	 * If SCHED_RESTART was set by the caller and it is no longer set, it
+	 * must have been cleared by another thread and hence a queue rerun is
+	 * needed.
 	 *
-	 * If 'no_tag' is set, that means that we failed getting
-	 * a driver tag with an I/O scheduler attached. If our dispatch
+	 * If blk_mq_prep_dispatch_rq returned PREP_DISPATCH_NO_TAG, we failed
+	 * to get a driver tag with an I/O scheduler attached. If our dispatch
 	 * waitqueue is no longer active, ensure that we run the queue
 	 * AFTER adding our entries back to the list.
+	 * If no I/O scheduler has been configured it is possible that the
+	 * hardware queue got stopped and restarted before requests were pushed
+	 * back onto the dispatch list.  Rerun the queue to avoid starvation.
 	 *
-	 * If no I/O scheduler has been configured it is possible that
-	 * the hardware queue got stopped and restarted before requests
-	 * were pushed back onto the dispatch list. Rerun the queue to
-	 * avoid starvation. Notes:
-	 * - blk_mq_run_hw_queue() checks whether or not a queue has
-	 *   been stopped before rerunning a queue.
-	 * - Some but not all block drivers stop a queue before
-	 *   returning BLK_STS_RESOURCE. Two exceptions are scsi-mq
-	 *   and dm-rq.
+	 * Notes:
+	 *   - blk_mq_run_hw_queue() checks whether or not a queue has been
+	 *     stopped before rerunning a queue.
+	 *   - Some but not all block drivers stop a queue before returning
+	 *     BLK_STS_RESOURCE. Two exceptions are scsi-mq and dm-rq.
 	 *
-	 * If driver returns BLK_STS_RESOURCE and SCHED_RESTART
-	 * bit is set, run queue after a delay to avoid IO stalls
-	 * that could otherwise occur if the queue is idle.  We'll do
-	 * similar if we couldn't get budget and SCHED_RESTART is set.
+	 * If driver returns BLK_STS_RESOURCE and the SCHED_RESTART bit is set,
+	 * run queue after a delay to avoid IO stalls that could otherwise occur
+	 * if the queue is idle.  We'll do similar if we couldn't get budget and
+	 * SCHED_RESTART is set.
 	 */
-	needs_restart = blk_mq_sched_needs_restart(hctx);
-	if (!needs_restart ||
-	    (no_tag && list_empty_careful(&hctx->dispatch_wait.entry)))
+	switch (prep) {
+	case PREP_DISPATCH_NO_TAG:
+		if ((hctx->flags & BLK_MQ_F_TAG_SHARED) &&
+		    list_empty_careful(&hctx->dispatch_wait.entry)) {
+		    	blk_mq_run_hw_queue(hctx, true);
+			return;
+		}
+		/*
+		 * For non-shared tags, the RESTART check will suffice.
+		 */
+		break;
+	case PREP_DISPATCH_OK:
+		if (ret == BLK_STS_RESOURCE)
+			no_budget_avail = true;
+		break;
+	case PREP_DISPATCH_NO_BUDGET:
+		no_budget_avail = true;
+		break;
+	}
+
+	if (!blk_mq_sched_needs_restart(hctx))
 		blk_mq_run_hw_queue(hctx, true);
-	else if (needs_restart && (ret == BLK_STS_RESOURCE ||
-				   no_budget_avail))
+	else if (no_budget_avail)
 		blk_mq_delay_run_hw_queue(hctx, BLK_MQ_RESOURCE_DELAY);
 }
 
@@ -1336,8 +1339,6 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 			 * accept.
 			 */
 			blk_mq_handle_zone_resource(rq, &zone_list);
-			if (list_empty(list))
-				break;
 			continue;
 		}
 
@@ -1350,9 +1351,6 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 		queued++;
 	} while (!list_empty(list));
 
-	if (!list_empty(&zone_list))
-		list_splice_tail_init(&zone_list, list);
-
 	hctx->dispatched[queued_to_index(queued)]++;
 
 	/*
@@ -1360,11 +1358,13 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 	 * that is where we will continue on next queue run.
 	 */
 	if (!list_empty(list)) {
-		blk_mq_handle_partial_dispatch(hctx, list, prep, ret, !!queued);
+		list_splice_tail_init(&zone_list, list);
+		blk_mq_handle_partial_dispatch(hctx, list, prep, ret, queued);
 		blk_mq_update_dispatch_busy(hctx, true);
 		return false;
-	} else
-		blk_mq_update_dispatch_busy(hctx, false);
+	}
+
+	blk_mq_update_dispatch_busy(hctx, false);
 
 	/*
 	 * If the host/device is unable to accept more work, inform the

Re: [PATCH 6/9] blk-mq: move code for handling partial dispatch into one helper

[Christoph Hellwig]

Btw, with the many arguments an more beeing added later I'm not sure
anymore if this is really worth a separate function or just if goto
label at the end of blk_mq_dispatch_rq_list that can be jumped to,
and which has the same amount of indentation is the better idea.

Re: [PATCH 6/9] blk-mq: move code for handling partial dispatch into one helper

[Ming Lei]

On Wed, May 13, 2020 at 06:01:00AM -0700, Christoph Hellwig wrote:
> Btw, with the many arguments an more beeing added later I'm not sure

blk_mq_handle_partial_dispatch() is always inlined, and each parameter
has precise meaning, so IMO more parameters shouldn't be an issue.

> anymore if this is really worth a separate function or just if goto
> label at the end of blk_mq_dispatch_rq_list that can be jumped to,
> and which has the same amount of indentation is the better idea.
> 

There are more benefits in this way:

1) blk_mq_dispatch_rq_list() becomes more readable

2) name of blk_mq_handle_partial_dispatch() has document benefit.

3) it is easier to add new code into both blk_mq_dispatch_rq_list()
and blk_mq_handle_partial_dispatch(), same with changes on both two
functions.

4) easier to verify/review two small function

So IMO it is worth a separate function, especially blk_mq_dispatch_rq_list()
is becoming a big monster function.


Thanks,
Ming

[PATCH 7/9] blk-mq: remove dead check from blk_mq_dispatch_rq_list

[Ming Lei]

When BLK_STS_RESOURCE or BLK_STS_DEV_RESOURCE is returned from
.queue_rq, the 'list' variable always holds this rq which isn't
queued to LLD successfully.

So blk_mq_dispatch_rq_list() always returns false from the branch
of '!list_empty(list)'.

No functional change.

Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 86beb8c66868..3e3fbb13f3b9 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1366,13 +1366,6 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 	} else
 		blk_mq_update_dispatch_busy(hctx, false);
 
-	/*
-	 * If the host/device is unable to accept more work, inform the
-	 * caller of that.
-	 */
-	if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE)
-		return false;
-
 	return (queued + errors) != 0;
 }
 
-- 
2.25.2

Re: [PATCH 7/9] blk-mq: remove dead check from blk_mq_dispatch_rq_list

[Christoph Hellwig]

On Wed, May 13, 2020 at 05:54:41PM +0800, Ming Lei wrote:
> When BLK_STS_RESOURCE or BLK_STS_DEV_RESOURCE is returned from
> .queue_rq, the 'list' variable always holds this rq which isn't
> queued to LLD successfully.
> 
> So blk_mq_dispatch_rq_list() always returns false from the branch
> of '!list_empty(list)'.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 7/9] blk-mq: remove dead check from blk_mq_dispatch_rq_list

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH 8/9] blk-mq: pass obtained budget count to blk_mq_dispatch_rq_list

[Ming Lei]

Pass obtained budget count to blk_mq_dispatch_rq_list(), and prepare
for supporting fully batching submission.

With the obtained budget count, it is easier to put extra budgets
in case of .queue_rq failure.

Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq-sched.c |  8 ++++----
 block/blk-mq.c       | 18 ++++++++++++++----
 block/blk-mq.h       |  3 ++-
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 632c6f8b63f7..78fc8d80caaf 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -130,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(rq->mq_hctx, &rq_list, true));
+	} while (blk_mq_dispatch_rq_list(rq->mq_hctx, &rq_list, true, 1));
 
 	return ret;
 }
@@ -198,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(rq->mq_hctx, &rq_list, true));
+	} while (blk_mq_dispatch_rq_list(rq->mq_hctx, &rq_list, true, 1));
 
 	WRITE_ONCE(hctx->dispatch_from, ctx);
 	return ret;
@@ -238,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(hctx, &rq_list, false)) {
+		if (blk_mq_dispatch_rq_list(hctx, &rq_list, false, 0)) {
 			if (has_sched_dispatch)
 				ret = blk_mq_do_dispatch_sched(hctx);
 			else
@@ -251,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(hctx, &rq_list, false);
+		blk_mq_dispatch_rq_list(hctx, &rq_list, false, 0);
 	}
 
 	return ret;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 3e3fbb13f3b9..bfdfdd61e663 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1212,7 +1212,8 @@ static enum prep_dispatch blk_mq_prep_dispatch_rq(struct request *rq,
 		 */
 		if (!blk_mq_mark_tag_wait(hctx, rq)) {
 			/* budget is always obtained before getting tag */
-			blk_mq_put_dispatch_budget(rq->q);
+			if (ask_budget)
+				blk_mq_put_dispatch_budget(rq->q);
 			return PREP_DISPATCH_NO_TAG;
 		}
 	}
@@ -1233,12 +1234,17 @@ static blk_status_t blk_mq_dispatch_rq(struct request *rq, bool is_last)
 
 static void blk_mq_handle_partial_dispatch(struct blk_mq_hw_ctx *hctx,
 		struct list_head *list, enum prep_dispatch prep,
-		blk_status_t ret, bool queued)
+		blk_status_t ret, bool queued, unsigned budgets)
 {
 	struct request_queue *q = hctx->queue;
 	bool needs_restart;
 	bool no_tag = false;
 	bool no_budget_avail = false;
+	unsigned i = 0;
+
+	/* release got budgets */
+	while (i++ < budgets)
+		blk_mq_put_dispatch_budget(hctx->queue);
 
 	/*
 	 * For non-shared tags, the RESTART check
@@ -1298,9 +1304,11 @@ static void blk_mq_handle_partial_dispatch(struct blk_mq_hw_ctx *hctx,
 
 /*
  * Returns true if we did some work AND can potentially do more.
+ *
+ * @budgets is only valid iff @got_budget is true.
  */
 bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
-			     bool got_budget)
+			     bool got_budget, unsigned int budgets)
 {
 	enum prep_dispatch prep;
 	struct request *rq;
@@ -1360,7 +1368,9 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 	 * that is where we will continue on next queue run.
 	 */
 	if (!list_empty(list)) {
-		blk_mq_handle_partial_dispatch(hctx, list, prep, ret, !!queued);
+		blk_mq_handle_partial_dispatch(hctx, list, prep, ret,
+				!!queued,
+				got_budget ? budgets - queued : 0);
 		blk_mq_update_dispatch_busy(hctx, true);
 		return false;
 	} else
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 9c0e93d4fe38..a37f8f673a19 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -40,7 +40,8 @@ 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 blk_mq_hw_ctx *hctx, struct list_head *, bool);
+bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *,
+		bool, unsigned int);
 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);
-- 
2.25.2

Re: [PATCH 8/9] blk-mq: pass obtained budget count to blk_mq_dispatch_rq_list

[Christoph Hellwig]

On Wed, May 13, 2020 at 05:54:42PM +0800, Ming Lei wrote:
> Pass obtained budget count to blk_mq_dispatch_rq_list(), and prepare
> for supporting fully batching submission.
> 
> With the obtained budget count, it is easier to put extra budgets
> in case of .queue_rq failure.

I think you can remove the got_budget parameter, as it will be false
exactly when 0 is passed in the new arguent.  Also I think nr_budgets
might be a better name.

[PATCH 9/9] blk-mq: support batching dispatch in case of io scheduler

[Ming Lei]

More and more drivers want to get batching requests queued from
block layer, such as mmc, and tcp based storage drivers. Also
current in-tree users have virtio-scsi, virtio-blk and nvme.

For none, we already support batching dispatch.

But for io scheduler, every time we just take one request from scheduler
and pass the single request to blk_mq_dispatch_rq_list(). This way makes
batching dispatch not possible when io scheduler is applied. One reason
is that we don't want to hurt sequential IO performance, becasue IO
merge chance is reduced if more requests are dequeued from scheduler
queue.

Try to support batching dispatch for io scheduler by starting with the
following simple approach:

1) still make sure we can get budget before dequeueing request

2) use hctx->dispatch_busy to evaluate if queue is busy, if it is busy
we fackback to non-batching dispatch, otherwise dequeue as many as
possible requests from scheduler, and pass them to blk_mq_dispatch_rq_list().

Wrt. 2), we use similar policy for none, and turns out that SCSI SSD
performance got improved much.

In future, maybe we can develop more intelligent algorithem for batching
dispatch.

[1] https://lore.kernel.org/linux-block/20200512075501.GF1531898@T590/#r
[2] https://lore.kernel.org/linux-block/fe6bd8b9-6ed9-b225-f80c-314746133722@grimberg.me/

Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq-sched.c | 76 +++++++++++++++++++++++++++++++++++++++++++-
 block/blk-mq.c       |  2 --
 2 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 78fc8d80caaf..77d5093916b7 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -7,6 +7,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/blk-mq.h>
+#include <linux/list_sort.h>
 
 #include <trace/events/block.h>
 
@@ -80,6 +81,69 @@ void blk_mq_sched_restart(struct blk_mq_hw_ctx *hctx)
 	blk_mq_run_hw_queue(hctx, true);
 }
 
+/*
+ * We know bfq and deadline apply single scheduler queue instead of multi
+ * queue. However, the two are often used on single queue devices, also
+ * the current @hctx should affect the real device status most of times
+ * because of locality principle.
+ *
+ * So use current hctx->dispatch_busy directly for figuring out batching
+ * dispatch count.
+ */
+static unsigned int blk_mq_sched_get_batching_nr(struct blk_mq_hw_ctx *hctx)
+{
+	if (hctx->dispatch_busy)
+		return 1;
+	return hctx->queue->nr_requests;
+}
+
+static int sched_rq_cmp(void *priv, struct list_head *a, struct list_head *b)
+{
+	struct request *rqa = container_of(a, struct request, queuelist);
+	struct request *rqb = container_of(b, struct request, queuelist);
+
+	return rqa->mq_hctx > rqb->mq_hctx;
+}
+
+static inline void blk_mq_do_dispatch_rq_lists(struct blk_mq_hw_ctx *hctx,
+		struct list_head *lists, bool multi_hctxs, unsigned count)
+{
+
+	if (likely(!multi_hctxs)) {
+		blk_mq_dispatch_rq_list(hctx, lists, true, count);
+		return;
+	}
+
+	/*
+	 * Requests from different hctx may be dequeued from some scheduler,
+	 * such as bfq and deadline.
+	 *
+	 * Sort the requests in the list according to their hctx, dispatch
+	 * batching requests from same hctx
+	 */
+	list_sort(NULL, lists, sched_rq_cmp);
+
+	while (!list_empty(lists)) {
+		LIST_HEAD(list);
+		struct request *new, *rq = list_first_entry(lists,
+				struct request, queuelist);
+		unsigned cnt = 0;
+
+		list_for_each_entry(new, lists, queuelist) {
+			if (new->mq_hctx != rq->mq_hctx)
+				break;
+			cnt++;
+		}
+
+		if (new->mq_hctx == rq->mq_hctx)
+			list_splice_tail_init(lists, &list);
+		else
+			list_cut_before(&list, lists, &new->queuelist);
+
+		blk_mq_dispatch_rq_list(rq->mq_hctx, &list, true, cnt);
+	}
+}
+
 #define BLK_MQ_BUDGET_DELAY	3		/* ms units */
 
 /*
@@ -97,6 +161,9 @@ static int blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx *hctx)
 	LIST_HEAD(rq_list);
 	int ret = 0;
 	struct request *rq;
+	int cnt = 0;
+	unsigned int max_dispatch = blk_mq_sched_get_batching_nr(hctx);
+	bool multi_hctxs = false;
 
 	do {
 		if (e->type->ops.has_work && !e->type->ops.has_work(hctx))
@@ -130,7 +197,14 @@ 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(rq->mq_hctx, &rq_list, true, 1));
+		cnt++;
+
+		if (rq->mq_hctx != hctx && !multi_hctxs)
+			multi_hctxs = true;
+	} while (cnt < max_dispatch);
+
+	if (cnt)
+		blk_mq_do_dispatch_rq_lists(hctx, &rq_list, multi_hctxs, cnt);
 
 	return ret;
 }
diff --git a/block/blk-mq.c b/block/blk-mq.c
index bfdfdd61e663..b83c59d640b4 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1319,8 +1319,6 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list,
 	if (list_empty(list))
 		return false;
 
-	WARN_ON(!list_is_singular(list) && got_budget);
-
 	/*
 	 * Now process all the entries, sending them to the driver.
 	 */
-- 
2.25.2

Re: [PATCH 0/9] blk-mq: support batching dispatch from scheduler

[Baolin Wang]

Hi Ming,

On Wed, May 13, 2020 at 5:55 PM Ming Lei <ming.lei@redhat.com> wrote:
>
> Hi Guys,
>
> More and more drivers want to get batching requests queued from
> block layer, such as mmc[1], and tcp based storage drivers[2]. Also
> current in-tree users have virtio-scsi, virtio-blk and nvme.
>
> For none, we already support batching dispatch.
>
> But for io scheduler, every time we just take one request from scheduler
> and pass the single request to blk_mq_dispatch_rq_list(). This way makes
> batching dispatch not possible when io scheduler is applied. One reason
> is that we don't want to hurt sequential IO performance, becasue IO
> merge chance is reduced if more requests are dequeued from scheduler
> queue.
>
> Tries to start the support by dequeuing more requests from scheduler
> if budget is enough and device isn't busy.
>
> Simple fio test over virtio-scsi shows IO can get improved by 5~10%.
>
> Patches can be found from the following tree too:
>
>         https://github.com/ming1/linux/commits/v5.7-rc-blk-mq-batching-submission
>
> Patch 1 ~ 7 are improvement and cleanup, which can't applied without
> supporting batching dispatch.
>
> Patch 8 ~ 9 starts to support batching dispatch from scheduler.

Sorry for late reply. I've tested your patch set and got some better
performance. Thanks.
Tested-by: Baolin Wang <baolin.wang7@gmail.com>

> Please review and comment!
>
>
> [1] https://lore.kernel.org/linux-block/20200512075501.GF1531898@T590/#r
> [2] https://lore.kernel.org/linux-block/fe6bd8b9-6ed9-b225-f80c-314746133722@grimberg.me/
>
>
> Ming Lei (9):
>   blk-mq: pass request queue into get/put budget callback
>   blk-mq: pass hctx to blk_mq_dispatch_rq_list
>   blk-mq: don't predicate last flag in blk_mq_dispatch_rq_list
>   blk-mq: move getting driver tag and bugget into one helper
>   blk-mq: move .queue_rq code into one helper
>   blk-mq: move code for handling partial dispatch into one helper
>   blk-mq: remove dead check from blk_mq_dispatch_rq_list
>   blk-mq: pass obtained budget count to blk_mq_dispatch_rq_list
>   blk-mq: support batching dispatch in case of io scheduler
>
>  block/blk-mq-sched.c    |  96 ++++++++++++++--
>  block/blk-mq.c          | 248 +++++++++++++++++++++-------------------
>  block/blk-mq.h          |  15 +--
>  drivers/scsi/scsi_lib.c |   8 +-
>  include/linux/blk-mq.h  |   4 +-
>  5 files changed, 226 insertions(+), 145 deletions(-)
>
> Cc: Sagi Grimberg <sagi@grimberg.me>
> Cc: Baolin Wang <baolin.wang7@gmail.com>
> Cc: Christoph Hellwig <hch@infradead.org>
>
> --
> 2.25.2
>


-- 
Baolin Wang

Re: [PATCH 0/9] blk-mq: support batching dispatch from scheduler

[Ming Lei]

On Sat, May 23, 2020 at 03:45:55PM +0800, Baolin Wang wrote:
> Hi Ming,
> 
> On Wed, May 13, 2020 at 5:55 PM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > Hi Guys,
> >
> > More and more drivers want to get batching requests queued from
> > block layer, such as mmc[1], and tcp based storage drivers[2]. Also
> > current in-tree users have virtio-scsi, virtio-blk and nvme.
> >
> > For none, we already support batching dispatch.
> >
> > But for io scheduler, every time we just take one request from scheduler
> > and pass the single request to blk_mq_dispatch_rq_list(). This way makes
> > batching dispatch not possible when io scheduler is applied. One reason
> > is that we don't want to hurt sequential IO performance, becasue IO
> > merge chance is reduced if more requests are dequeued from scheduler
> > queue.
> >
> > Tries to start the support by dequeuing more requests from scheduler
> > if budget is enough and device isn't busy.
> >
> > Simple fio test over virtio-scsi shows IO can get improved by 5~10%.
> >
> > Patches can be found from the following tree too:
> >
> >         https://github.com/ming1/linux/commits/v5.7-rc-blk-mq-batching-submission
> >
> > Patch 1 ~ 7 are improvement and cleanup, which can't applied without
> > supporting batching dispatch.
> >
> > Patch 8 ~ 9 starts to support batching dispatch from scheduler.
> 
> Sorry for late reply. I've tested your patch set and got some better
> performance. Thanks.
> Tested-by: Baolin Wang <baolin.wang7@gmail.com>

Hi Baolin,

Thanks for your test & feedback, then looks this approach is good.

I will address comments on v1 and post v2 soon.


Thanks,
Ming