On 2020-02-06 13:12, Salman Qazi wrote:
> + *
> + * Returns true if hctx->dispatch was found non-empty and
> + * run_work has to be run again.
Please elaborate this comment and explain why this is necessary (to
avoid that flush processing is postponed forever).
> + * Returns true if hctx->dispatch was found non-empty and
> + * run_work has to be run again.
Same comment here.
> +again:
> + run_again = false;
> +
> /*
> * If we have previous entries on our dispatch list, grab them first for
> * more fair dispatch.
> @@ -208,19 +234,28 @@ void blk_mq_sched_dispatch_requests(struct blk_mq_hw_ctx *hctx)
> blk_mq_sched_mark_restart_hctx(hctx);
> if (blk_mq_dispatch_rq_list(q, &rq_list, false)) {
> if (has_sched_dispatch)
> - blk_mq_do_dispatch_sched(hctx);
> + run_again = blk_mq_do_dispatch_sched(hctx);
> else
> - blk_mq_do_dispatch_ctx(hctx);
> + run_again = blk_mq_do_dispatch_ctx(hctx);
> }
> } else if (has_sched_dispatch) {
> - blk_mq_do_dispatch_sched(hctx);
> + run_again = blk_mq_do_dispatch_sched(hctx);
> } else if (hctx->dispatch_busy) {
> /* dequeue request one by one from sw queue if queue is busy */
> - blk_mq_do_dispatch_ctx(hctx);
> + run_again = blk_mq_do_dispatch_ctx(hctx);
> } else {
> blk_mq_flush_busy_ctxs(hctx, &rq_list);
> blk_mq_dispatch_rq_list(q, &rq_list, false);
> }
> +
> + if (run_again) {
> + if (!restarted) {
> + restarted = true;
> + goto again;
> + }
> +
> + blk_mq_run_hw_queue(hctx, true);
> + }
So this patch changes blk_mq_sched_dispatch_requests() such that it
iterates at most two times? How about implementing that loop with an
explicit for-loop? I think that will make
blk_mq_sched_dispatch_requests() easier to read. As you may know forward
goto's are accepted in kernel code but backward goto's are frowned upon.
Thanks,
Bart.
