Re: [PATCH V5 7/7] blk-mq-sched: don't dequeue request until all in ->dispatch are flushed

From: Christoph Hellwig
Date: Tue Oct 03 2017 - 05:11:36 EST


This looks good in general:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>

Minor nitpicks below:

> const bool has_sched_dispatch = e && e->type->ops.mq.dispatch_request;

This is now only tested once, so you can remove the local variable
for it.

> + /*
> + * We may clear DISPATCH_BUSY just after it
> + * is set from another context, the only cost
> + * is that one request is dequeued a bit early,
> + * we can survive that. Given the window is
> + * small enough, no need to worry about performance
> + * effect.
> + */

Use your 80 line real estate for comments please.

> if (!has_sched_dispatch)
> + if (!q->queue_depth) {
> + blk_mq_flush_busy_ctxs(hctx, &rq_list);
> + blk_mq_dispatch_rq_list(q, &rq_list);
> + } else {
> + blk_mq_do_dispatch_ctx(q, hctx);
> + }
> + } else {
> blk_mq_do_dispatch_sched(q, e, hctx);
> + }

Maybe flatten this out to:

if (e && e->type->ops.mq.dispatch_request) {
blk_mq_do_dispatch_sched(q, e, hctx);
} else if (q->queue_depth) {
blk_mq_do_dispatch_ctx(q, hctx);
} else {
blk_mq_flush_busy_ctxs(hctx, &rq_list);
blk_mq_dispatch_rq_list(q, &rq_list);
}