On Tue, Mar 26, 2019 at 12:44:11PM +0100, Sergio Lopez wrote: > > Stefan Hajnoczi writes: > > > On Mon, Mar 25, 2019 at 02:34:36PM +0100, Sergio Lopez wrote: > >> Our current ThreadPool implementation lacks support for AioContext's > >> event notifications. This not only means that it can't take advantage > >> from the IOThread's adaptive polling, but also that the latter works > >> against it, as it delays the execution of the bottom-halves > >> completions. > > > > util/async.c:event_notifier_poll() participates in polling and should > > detect that a bottom half was scheduled by a worker thread. > > > > Can you investigate, because I'm not sure this commit description has > > identified the root cause of the performance improvement? > > The problem with event_notifier_poll() is that ctx->notifier is > explicitly ignored in run_poll_handlers_once(), so the BH notification > doesn't count as making progress and run_poll_handlers() keeps running > the polling loop: > > 513 static bool run_poll_handlers_once(AioContext *ctx, int64_t *timeout) > 514 { > 515 bool progress = false; > 516 AioHandler *node; > 517 > 518 QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) { > 519 if (!node->deleted && node->io_poll && > 520 aio_node_check(ctx, node->is_external) && > 521 node->io_poll(node->opaque)) { > 522 *timeout = 0; > 523 if (node->opaque != &ctx->notifier) { > 524 progress = true; > 525 } > 526 } > 527 > 528 /* Caller handles freeing deleted nodes. Don't do it here. */ > 529 } > 530 > 531 return progress; > 532 } > > 547 static bool run_poll_handlers(AioContext *ctx, int64_t max_ns, int64_t > *timeout) > 548 { > 549 bool progress; > 550 int64_t start_time, elapsed_time; > 551 > 552 assert(ctx->notify_me); > 553 assert(qemu_lockcnt_count(&ctx->list_lock) > 0); > 554 > 555 trace_run_poll_handlers_begin(ctx, max_ns, *timeout); > 556 > 557 start_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); > 558 do { > 559 progress = run_poll_handlers_once(ctx, timeout); > 560 elapsed_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - > start_time; > 561 } while (!progress && elapsed_time < max_ns > 562 && !atomic_read(&ctx->poll_disable_cnt)); > (...)
For the record, Sergio, Paolo, and I discussed this more on IRC. Sergio is investigating the options here. Stefan
signature.asc
Description: PGP signature