On Sat, 2017-08-05 at 14:56 +0800, Ming Lei wrote:
> easy to cause queue busy becasue of the small
^^^^^^^
because?
> -static void blk_mq_do_dispatch(struct request_queue *q,
> - struct elevator_queue *e,
> - struct blk_mq_hw_ctx *hctx)
> +static inline void blk_mq_do_dispatch_sched(struct request_queue *q,
> + struct elevator_queue *e,
> + struct blk_mq_hw_ctx *hctx)
> {
> LIST_HEAD(rq_list);
Why to declare this function "inline"? Are you sure that the compiler
is not smart enough to decide on its own whether or not to inline this
function?
> +static inline 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 inline void blk_mq_do_dispatch_ctx(struct request_queue *q,
> + struct blk_mq_hw_ctx *hctx)
> +{
> + LIST_HEAD(rq_list);
> + struct blk_mq_ctx *ctx = NULL;
> +
> + do {
> + struct request *rq;
> +
> + rq = blk_mq_dispatch_rq_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));
> +}
Please consider to move the blk_mq_next_ctx() functionality into
blk_mq_dispatch_rq_from_ctx() as requested in a comment on a previous patch.
> void blk_mq_sched_dispatch_requests(struct blk_mq_hw_ctx *hctx)
> {
> struct request_queue *q = hctx->queue;
> @@ -142,18 +172,31 @@ void blk_mq_sched_dispatch_requests(struct
> blk_mq_hw_ctx *hctx)
> if (!list_empty(&rq_list)) {
> blk_mq_sched_mark_restart_hctx(hctx);
> do_sched_dispatch = blk_mq_dispatch_rq_list(q, &rq_list);
> - } else if (!has_sched_dispatch) {
> + } else if (!has_sched_dispatch & !q->queue_depth) {
Please use "&&" instead of "&" to represent logical and.
Otherwise this patch looks fine to me.
Thanks,
Bart.
