On Fri, 27 Nov 2015 12:43:06 +0100 Paolo Bonzini <pbonz...@redhat.com> wrote:
> The QEMU thread pool already has a mechanism to invoke callbacks in the main > thread. It does not need an EventNotifier and it is more efficient too. > Use it instead of GAsyncQueue + GThreadPool + glue. > > As a side effect, it silences Coverity's complaint about an unchecked > return value for event_notifier_init. > And it makes the code a lot nicer too ! > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- I have just one minor remark in hw/9pfs/virtio-9p-coth.h but anyway: Reviewed-by: Greg Kurz <gk...@linux.vnet.ibm.com> Tested-by: Greg Kurz <gk...@linux.vnet.ibm.com> > hw/9pfs/virtio-9p-coth.c | 79 > +++++++++++----------------------------------- > hw/9pfs/virtio-9p-coth.h | 9 +----- > hw/9pfs/virtio-9p-device.c | 4 --- > 3 files changed, 20 insertions(+), 72 deletions(-) > > diff --git a/hw/9pfs/virtio-9p-coth.c b/hw/9pfs/virtio-9p-coth.c > index 5057f8d..fb6e8f8 100644 > --- a/hw/9pfs/virtio-9p-coth.c > +++ b/hw/9pfs/virtio-9p-coth.c > @@ -12,71 +12,30 @@ > * > */ > > -#include "fsdev/qemu-fsdev.h" > -#include "qemu/thread.h" > -#include "qemu/event_notifier.h" > +#include "qemu-common.h" > +#include "block/thread-pool.h" > #include "qemu/coroutine.h" > +#include "qemu/main-loop.h" > #include "virtio-9p-coth.h" > > -/* v9fs glib thread pool */ > -static V9fsThPool v9fs_pool; > +/* Called from QEMU I/O thread. */ > +static void coroutine_enter_cb(void *opaque, int ret) > +{ > + Coroutine *co = opaque; > + qemu_coroutine_enter(co, NULL); > +} > + > +/* Called from worker thread. */ > +static int coroutine_enter_func(void *arg) > +{ > + Coroutine *co = arg; > + qemu_coroutine_enter(co, NULL); > + return 0; > +} > > void co_run_in_worker_bh(void *opaque) > { > Coroutine *co = opaque; > - g_thread_pool_push(v9fs_pool.pool, co, NULL); > -} > - > -static void v9fs_qemu_process_req_done(EventNotifier *e) > -{ > - Coroutine *co; > - > - event_notifier_test_and_clear(e); > - > - while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) { > - qemu_coroutine_enter(co, NULL); > - } > -} > - > -static void v9fs_thread_routine(gpointer data, gpointer user_data) > -{ > - Coroutine *co = data; > - > - qemu_coroutine_enter(co, NULL); > - > - g_async_queue_push(v9fs_pool.completed, co); > - > - event_notifier_set(&v9fs_pool.e); > -} > - > -int v9fs_init_worker_threads(void) > -{ > - int ret = 0; > - V9fsThPool *p = &v9fs_pool; > - sigset_t set, oldset; > - > - sigfillset(&set); > - /* Leave signal handling to the iothread. */ > - pthread_sigmask(SIG_SETMASK, &set, &oldset); > - > - p->pool = g_thread_pool_new(v9fs_thread_routine, p, -1, FALSE, NULL); > - if (!p->pool) { > - ret = -1; > - goto err_out; > - } > - p->completed = g_async_queue_new(); > - if (!p->completed) { > - /* > - * We are going to terminate. > - * So don't worry about cleanup > - */ > - ret = -1; > - goto err_out; > - } > - event_notifier_init(&p->e, 0); > - > - event_notifier_set_handler(&p->e, v9fs_qemu_process_req_done); > -err_out: > - pthread_sigmask(SIG_SETMASK, &oldset, NULL); > - return ret; > + thread_pool_submit_aio(qemu_get_aio_context()->thread_pool, > + coroutine_enter_func, co, coroutine_enter_cb, co); > } > diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h > index 0fbe49a..535743f 100644 > --- a/hw/9pfs/virtio-9p-coth.h > +++ b/hw/9pfs/virtio-9p-coth.h > @@ -20,13 +20,6 @@ > #include "virtio-9p.h" > #include <glib.h> > glib.h is no more needed. > -typedef struct V9fsThPool { > - EventNotifier e; > - > - GThreadPool *pool; > - GAsyncQueue *completed; > -} V9fsThPool; > - > /* > * we want to use bottom half because we want to make sure the below > * sequence of events. > @@ -45,7 +38,7 @@ typedef struct V9fsThPool { > qemu_bh_schedule(co_bh); \ > /* \ > * yield in qemu thread and re-enter back \ > - * in glib worker thread \ > + * in worker thread \ > */ \ > qemu_coroutine_yield(); \ > qemu_bh_delete(co_bh); \ > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c > index e3abcfa..944b5f5 100644 > --- a/hw/9pfs/virtio-9p-device.c > +++ b/hw/9pfs/virtio-9p-device.c > @@ -116,10 +116,6 @@ static void virtio_9p_device_realize(DeviceState *dev, > Error **errp) > " and export path:%s", s->fsconf.fsdev_id, > s->ctx.fs_root); > goto out; > } > - if (v9fs_init_worker_threads() < 0) { > - error_setg(errp, "worker thread initialization failed"); > - goto out; > - } > > /* > * Check details of export path, We need to use fs driver