Skip to content

Commit

Permalink
AioContext: force event loop iteration using BH
Browse files Browse the repository at this point in the history
The notify_me optimization introduced in commit eabc977
("AioContext: fix broken ctx->dispatching optimization") skips
event_notifier_set() calls when the event loop thread is not blocked in
ppoll(2).

This optimization causes a deadlock if two aio_context_acquire() calls
race.  notify_me = 0 during the race so the winning thread can enter
ppoll(2) unaware that the other thread is waiting its turn to acquire
the AioContext.

This patch forces ppoll(2) to return by scheduling a BH instead of
calling aio_notify().

The following deadlock with virtio-blk dataplane is fixed:

  qemu ... -object iothread,id=iothread0 \
           -drive if=none,id=drive0,file=test.img,... \
           -device virtio-blk-pci,iothread=iothread0,drive=drive0

This command-line results in a hang early on without this patch.

Thanks to Paolo Bonzini <pbonzini@redhat.com> for investigating this bug
with me.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 1438101249-25166-4-git-send-email-pbonzini@redhat.com
Message-Id: <1438014819-18125-3-git-send-email-stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
  • Loading branch information
stefanhaRH committed Jul 29, 2015
1 parent a076972 commit ca96ac4
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
16 changes: 14 additions & 2 deletions async.c
Expand Up @@ -79,8 +79,10 @@ int aio_bh_poll(AioContext *ctx)
* aio_notify again if necessary.
*/
if (!bh->deleted && atomic_xchg(&bh->scheduled, 0)) {
if (!bh->idle)
/* Idle BHs and the notify BH don't count as progress */
if (!bh->idle && bh != ctx->notify_dummy_bh) {
ret = 1;
}
bh->idle = 0;
bh->cb(bh->opaque);
}
Expand Down Expand Up @@ -230,6 +232,7 @@ aio_ctx_finalize(GSource *source)
{
AioContext *ctx = (AioContext *) source;

qemu_bh_delete(ctx->notify_dummy_bh);
thread_pool_free(ctx->thread_pool);

qemu_mutex_lock(&ctx->bh_lock);
Expand Down Expand Up @@ -298,8 +301,15 @@ static void aio_timerlist_notify(void *opaque)

static void aio_rfifolock_cb(void *opaque)
{
AioContext *ctx = opaque;

/* Kick owner thread in case they are blocked in aio_poll() */
aio_notify(opaque);
qemu_bh_schedule(ctx->notify_dummy_bh);
}

static void notify_dummy_bh(void *opaque)
{
/* Do nothing, we were invoked just to force the event loop to iterate */
}

static void event_notifier_dummy_cb(EventNotifier *e)
Expand All @@ -326,6 +336,8 @@ AioContext *aio_context_new(Error **errp)
rfifolock_init(&ctx->lock, aio_rfifolock_cb, ctx);
timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);

ctx->notify_dummy_bh = aio_bh_new(ctx, notify_dummy_bh, NULL);

return ctx;
}

Expand Down
3 changes: 3 additions & 0 deletions include/block/aio.h
Expand Up @@ -114,6 +114,9 @@ struct AioContext {
bool notified;
EventNotifier notifier;

/* Scheduling this BH forces the event loop it iterate */
QEMUBH *notify_dummy_bh;

/* Thread pool for performing work and receiving completion callbacks */
struct ThreadPool *thread_pool;

Expand Down

0 comments on commit ca96ac4

Please sign in to comment.