From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56196) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2IMb-0007JU-PW for qemu-devel@nongnu.org; Fri, 27 Nov 2015 07:42:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a2IMY-00078w-HC for qemu-devel@nongnu.org; Fri, 27 Nov 2015 07:42:57 -0500 Received: from e06smtp11.uk.ibm.com ([195.75.94.107]:57569) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2IMY-00078n-7b for qemu-devel@nongnu.org; Fri, 27 Nov 2015 07:42:54 -0500 Received: from localhost by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 27 Nov 2015 12:42:52 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id CC6471B0804B for ; Fri, 27 Nov 2015 12:43:14 +0000 (GMT) Received: from d06av08.portsmouth.uk.ibm.com (d06av08.portsmouth.uk.ibm.com [9.149.37.249]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id tARCgoMA65077346 for ; Fri, 27 Nov 2015 12:42:50 GMT Received: from d06av08.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av08.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id tARCgo3h031681 for ; Fri, 27 Nov 2015 05:42:50 -0700 Date: Fri, 27 Nov 2015 13:42:46 +0100 From: Greg Kurz Message-ID: <20151127134246.3a33702d@bahia.local> In-Reply-To: <1448624586-28299-2-git-send-email-pbonzini@redhat.com> References: <1448624586-28299-2-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.5] virtio-9p: use QEMU thread pool List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, aneesh.kumar@linux.vnet.ibm.com On Fri, 27 Nov 2015 12:43:06 +0100 Paolo Bonzini 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 > --- I have just one minor remark in hw/9pfs/virtio-9p-coth.h but anyway: Reviewed-by: Greg Kurz Tested-by: Greg Kurz > 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 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