On 10/20/20 2:23 AM, Xiaoguang Wang wrote:
> @@ -6638,24 +6578,52 @@ static int io_sq_thread(void *data)
> }
> io_sq_thread_associate_blkcg(ctx, &cur_css);
>
> - ret |= __io_sq_thread(ctx, start_jiffies, cap_entries);
> + ret = __io_sq_thread(ctx, cap_entries);
> + if (!sqt_spin && (ret > 0 || !list_empty(&ctx->iopoll_list)))
> + sqt_spin = true;
> + }
>
> - io_sq_thread_drop_mm();
> + if (sqt_spin) {
> + io_run_task_work();
> + cond_resched();
> + continue;
> }
>
> - if (ret & SQT_SPIN) {
> + if (!time_after(jiffies, timeout)) {
> io_run_task_work();
> cond_resched();
> - } else if (ret == SQT_IDLE) {
> - if (kthread_should_park())
> - continue;
> + io_sq_thread_drop_mm();
> + continue;
> + }
> +
> + needs_sched = true;
> + prepare_to_wait(&sqd->wait, &wait, TASK_INTERRUPTIBLE);
> + list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) {
> + if ((ctx->flags & IORING_SETUP_IOPOLL) &&
> + !list_empty_careful(&ctx->iopoll_list)) {
> + needs_sched = false;
> + break;
> + }
> + to_submit = io_sqring_entries(ctx);
> + if (to_submit) {
> + needs_sched = false;
> + break;
> + }
> + }
> +
> + if (needs_sched) {
> list_for_each_entry(ctx, &sqd->ctx_list, sqd_list)
> io_ring_set_wakeup_flag(ctx);
> + }
> +
> + if (needs_sched) {
> schedule();
> - start_jiffies = jiffies;
> list_for_each_entry(ctx, &sqd->ctx_list, sqd_list)
> io_ring_clear_wakeup_flag(ctx);
> - }
> + finish_wait(&sqd->wait, &wait);
> + } else
> + finish_wait(&sqd->wait, &wait);
> + timeout = jiffies + sqd->sq_thread_idle;
> }
Not sure why, but you have two
if (needs_sched) {
}
right after each other, that should be folded into one.
And I'd get rid of the to_submit, just do:
if (io_sqring_entries(ctx)) {
needs_sched = false;
break;
}
as we're not going to be using that value anyway.
As a minor style thing, always includes bracket on both cases if one has
it, don't do this:
if (foo) {
...
...
} else
bar();
--
Jens Axboe
