Skip to content

Commit

Permalink
async: the main AioContext is only "current" if under the BQL
Browse files Browse the repository at this point in the history
If we want to wake up a coroutine from a worker thread, aio_co_wake()
currently does not work.  In that scenario, aio_co_wake() calls
aio_co_enter(), but there is no current AioContext and therefore
qemu_get_current_aio_context() returns the main thread.  aio_co_wake()
then attempts to call aio_context_acquire() instead of going through
aio_co_schedule().

The default case of qemu_get_current_aio_context() was added to cover
synchronous I/O started from the vCPU thread, but the main and vCPU
threads are quite different.  The main thread is an I/O thread itself,
only running a more complicated event loop; the vCPU thread instead
is essentially a worker thread that occasionally calls
qemu_mutex_lock_iothread().  It is only in those critical sections
that it acts as if it were the home thread of the main AioContext.

Therefore, this patch detaches qemu_get_current_aio_context() from
iothreads, which is a useless complication.  The AioContext pointer
is stored directly in the thread-local variable, including for the
main loop.  Worker threads (including vCPU threads) optionally behave
as temporary home threads if they have taken the big QEMU lock,
but if that is not the case they will always schedule coroutines
on remote threads via aio_co_schedule().

With this change, the stub qemu_mutex_iothread_locked() must be changed
from true to false.  The previous value of true was needed because the
main thread did not have an AioContext in the thread-local variable,
but now it does have one.

Reported-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Tested-by: Vladimir Sementsov-Ogievskiy <[email protected]>
[eblake: tweak commit message per Vladimir's review]
Signed-off-by: Eric Blake <[email protected]>
  • Loading branch information
bonzini authored and ebblake committed Jun 18, 2021
1 parent 3ccf6cd commit 5f50be9
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 27 deletions.
5 changes: 4 additions & 1 deletion include/block/aio.h
Original file line number Diff line number Diff line change
Expand Up @@ -691,10 +691,13 @@ void aio_co_enter(AioContext *ctx, struct Coroutine *co);
* Return the AioContext whose event loop runs in the current thread.
*
* If called from an IOThread this will be the IOThread's AioContext. If
* called from another thread it will be the main loop AioContext.
* called from the main thread or with the "big QEMU lock" taken it
* will be the main loop AioContext.
*/
AioContext *qemu_get_current_aio_context(void);

void qemu_set_current_aio_context(AioContext *ctx);

/**
* aio_context_setup:
* @ctx: the aio context
Expand Down
9 changes: 1 addition & 8 deletions iothread.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,6 @@ DECLARE_CLASS_CHECKERS(IOThreadClass, IOTHREAD,
#define IOTHREAD_POLL_MAX_NS_DEFAULT 0ULL
#endif

static __thread IOThread *my_iothread;

AioContext *qemu_get_current_aio_context(void)
{
return my_iothread ? my_iothread->ctx : qemu_get_aio_context();
}

static void *iothread_run(void *opaque)
{
IOThread *iothread = opaque;
Expand All @@ -56,7 +49,7 @@ static void *iothread_run(void *opaque)
* in this new thread uses glib.
*/
g_main_context_push_thread_default(iothread->worker_context);
my_iothread = iothread;
qemu_set_current_aio_context(iothread->ctx);
iothread->thread_id = qemu_get_thread_id();
qemu_sem_post(&iothread->init_done_sem);

Expand Down
2 changes: 1 addition & 1 deletion stubs/iothread-lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

bool qemu_mutex_iothread_locked(void)
{
return true;
return false;
}

void qemu_mutex_lock_iothread_impl(const char *file, int line)
Expand Down
8 changes: 0 additions & 8 deletions stubs/iothread.c

This file was deleted.

1 change: 0 additions & 1 deletion stubs/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ stub_ss.add(files('fw_cfg.c'))
stub_ss.add(files('gdbstub.c'))
stub_ss.add(files('get-vm-name.c'))
stub_ss.add(when: 'CONFIG_LINUX_IO_URING', if_true: files('io_uring.c'))
stub_ss.add(files('iothread.c'))
stub_ss.add(files('iothread-lock.c'))
stub_ss.add(files('isa-bus.c'))
stub_ss.add(files('is-daemonized.c'))
Expand Down
9 changes: 1 addition & 8 deletions tests/unit/iothread.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ struct IOThread {
bool stopping;
};

static __thread IOThread *my_iothread;

AioContext *qemu_get_current_aio_context(void)
{
return my_iothread ? my_iothread->ctx : qemu_get_aio_context();
}

static void iothread_init_gcontext(IOThread *iothread)
{
GSource *source;
Expand All @@ -54,9 +47,9 @@ static void *iothread_run(void *opaque)

rcu_register_thread();

my_iothread = iothread;
qemu_mutex_lock(&iothread->init_done_lock);
iothread->ctx = aio_context_new(&error_abort);
qemu_set_current_aio_context(iothread->ctx);

/*
* We must connect the ctx to a GMainContext, because in older versions
Expand Down
20 changes: 20 additions & 0 deletions util/async.c
Original file line number Diff line number Diff line change
Expand Up @@ -649,3 +649,23 @@ void aio_context_release(AioContext *ctx)
{
qemu_rec_mutex_unlock(&ctx->lock);
}

static __thread AioContext *my_aiocontext;

AioContext *qemu_get_current_aio_context(void)
{
if (my_aiocontext) {
return my_aiocontext;
}
if (qemu_mutex_iothread_locked()) {
/* Possibly in a vCPU thread. */
return qemu_get_aio_context();
}
return NULL;
}

void qemu_set_current_aio_context(AioContext *ctx)
{
assert(!my_aiocontext);
my_aiocontext = ctx;
}
1 change: 1 addition & 0 deletions util/main-loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ int qemu_init_main_loop(Error **errp)
if (!qemu_aio_context) {
return -EMFILE;
}
qemu_set_current_aio_context(qemu_aio_context);
qemu_notify_bh = qemu_bh_new(notify_event_cb, NULL);
gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
src = aio_get_g_source(qemu_aio_context);
Expand Down

0 comments on commit 5f50be9

Please sign in to comment.