From 9ce44e2ce267caf5559904a201aa1986b0a8326b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 5 Oct 2020 17:58:50 +0200 Subject: qmp: Move dispatcher to a coroutine This moves the QMP dispatcher to a coroutine and runs all QMP command handlers that declare 'coroutine': true in coroutine context so they can avoid blocking the main loop while doing I/O or waiting for other events. For commands that are not declared safe to run in a coroutine, the dispatcher drops out of coroutine context by calling the QMP command handler from a bottom half. Signed-off-by: Kevin Wolf Reviewed-by: Markus Armbruster Message-Id: <20201005155855.256490-10-kwolf@redhat.com> Reviewed-by: Markus Armbruster Reviewed-by: Stefan Hajnoczi Signed-off-by: Markus Armbruster --- monitor/monitor.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 8 deletions(-) (limited to 'monitor/monitor.c') diff --git a/monitor/monitor.c b/monitor/monitor.c index ef68ca9d21..ceffe1a83b 100644 --- a/monitor/monitor.c +++ b/monitor/monitor.c @@ -55,8 +55,32 @@ typedef struct { /* Shared monitor I/O thread */ IOThread *mon_iothread; -/* Bottom half to dispatch the requests received from I/O thread */ -QEMUBH *qmp_dispatcher_bh; +/* Coroutine to dispatch the requests received from I/O thread */ +Coroutine *qmp_dispatcher_co; + +/* Set to true when the dispatcher coroutine should terminate */ +bool qmp_dispatcher_co_shutdown; + +/* + * qmp_dispatcher_co_busy is used for synchronisation between the + * monitor thread and the main thread to ensure that the dispatcher + * coroutine never gets scheduled a second time when it's already + * scheduled (scheduling the same coroutine twice is forbidden). + * + * It is true if the coroutine is active and processing requests. + * Additional requests may then be pushed onto mon->qmp_requests, + * and @qmp_dispatcher_co_shutdown may be set without further ado. + * @qmp_dispatcher_co_busy must not be woken up in this case. + * + * If false, you also have to set @qmp_dispatcher_co_busy to true and + * wake up @qmp_dispatcher_co after pushing the new requests. + * + * The coroutine will automatically change this variable back to false + * before it yields. Nobody else may set the variable to false. + * + * Access must be atomic for thread safety. + */ +bool qmp_dispatcher_co_busy; /* * Protects mon_list, monitor_qapi_event_state, coroutine_mon, @@ -623,9 +647,24 @@ void monitor_cleanup(void) } qemu_mutex_unlock(&monitor_lock); - /* QEMUBHs needs to be deleted before destroying the I/O thread */ - qemu_bh_delete(qmp_dispatcher_bh); - qmp_dispatcher_bh = NULL; + /* + * The dispatcher needs to stop before destroying the I/O thread. + * + * We need to poll both qemu_aio_context and iohandler_ctx to make + * sure that the dispatcher coroutine keeps making progress and + * eventually terminates. qemu_aio_context is automatically + * polled by calling AIO_WAIT_WHILE on it, but we must poll + * iohandler_ctx manually. + */ + qmp_dispatcher_co_shutdown = true; + if (!qatomic_xchg(&qmp_dispatcher_co_busy, true)) { + aio_co_wake(qmp_dispatcher_co); + } + + AIO_WAIT_WHILE(qemu_get_aio_context(), + (aio_poll(iohandler_get_aio_context(), false), + qatomic_mb_read(&qmp_dispatcher_co_busy))); + if (mon_iothread) { iothread_destroy(mon_iothread); mon_iothread = NULL; @@ -649,9 +688,9 @@ void monitor_init_globals_core(void) * have commands assuming that context. It would be nice to get * rid of those assumptions. */ - qmp_dispatcher_bh = aio_bh_new(iohandler_get_aio_context(), - monitor_qmp_bh_dispatcher, - NULL); + qmp_dispatcher_co = qemu_coroutine_create(monitor_qmp_dispatcher_co, NULL); + qatomic_mb_set(&qmp_dispatcher_co_busy, true); + aio_co_schedule(iohandler_get_aio_context(), qmp_dispatcher_co); } int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp) -- cgit v1.2.3