Messages in this thread | | | Subject | Re: [PATCH] block: Limit number of items taken from the I/O scheduler in one go | From | Bart Van Assche <> | Date | Fri, 7 Feb 2020 07:26:49 -0800 |
| |
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.
| |