Re: [Qemu-block] [PATCH] thread-pool: Use an EventNotifier to coordinate with AioContext

[Sergio Lopez]

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));
(...)

Thanks,
Sergio.