From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42964) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XCUCE-0006tz-KK for qemu-devel@nongnu.org; Wed, 30 Jul 2014 09:45:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XCUC9-0000ao-Bw for qemu-devel@nongnu.org; Wed, 30 Jul 2014 09:45:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42444) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XCUC9-0000Za-4P for qemu-devel@nongnu.org; Wed, 30 Jul 2014 09:45:29 -0400 Message-ID: <53D8F6F0.7040106@redhat.com> Date: Wed, 30 Jul 2014 15:45:20 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1406720388-18671-1-git-send-email-ming.lei@canonical.com> <1406720388-18671-2-git-send-email-ming.lei@canonical.com> In-Reply-To: <1406720388-18671-2-git-send-email-ming.lei@canonical.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 01/15] qemu coroutine: support bypass mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei , qemu-devel@nongnu.org, Peter Maydell , Stefan Hajnoczi Cc: Kevin Wolf , Fam Zheng , "Michael S. Tsirkin" Il 30/07/2014 13:39, Ming Lei ha scritto: > This patch introduces several APIs for supporting bypass qemu coroutine > in case of being not necessary and for performance's sake. No, this is wrong. Dataplane *must* use the same code as non-dataplane, anything else is a step backwards. If you want to bypass coroutines, bdrv_aio_readv/writev must detect the conditions that allow doing that and call the bdrv_aio_readv/writev directly. To begin with, have you benchmarked QEMU and can you provide a trace of *where* the coroutine overhead lies? Paolo > Signed-off-by: Ming Lei > --- > include/block/coroutine.h | 8 ++++++++ > include/block/coroutine_int.h | 5 +++++ > qemu-coroutine-lock.c | 4 ++-- > qemu-coroutine.c | 33 +++++++++++++++++++++++++++++++++ > 4 files changed, 48 insertions(+), 2 deletions(-) > > diff --git a/include/block/coroutine.h b/include/block/coroutine.h > index b408f96..46d2642 100644 > --- a/include/block/coroutine.h > +++ b/include/block/coroutine.h > @@ -223,4 +223,12 @@ void coroutine_fn co_aio_sleep_ns(AioContext *ctx, QEMUClockType type, > * Note that this function clobbers the handlers for the file descriptor. > */ > void coroutine_fn yield_until_fd_readable(int fd); > + > +/* qemu coroutine bypass APIs */ > +void qemu_coroutine_set_bypass(bool bypass); > +bool qemu_coroutine_bypassed(Coroutine *self); > +bool qemu_coroutine_self_bypassed(void); > +void qemu_coroutine_set_var(void *var); > +void *qemu_coroutine_get_var(void); > + > #endif /* QEMU_COROUTINE_H */ > diff --git a/include/block/coroutine_int.h b/include/block/coroutine_int.h > index f133d65..106d0b2 100644 > --- a/include/block/coroutine_int.h > +++ b/include/block/coroutine_int.h > @@ -39,6 +39,11 @@ struct Coroutine { > Coroutine *caller; > QSLIST_ENTRY(Coroutine) pool_next; > > + bool bypass; > + > + /* only used in bypass mode */ > + void *opaque; > + > /* Coroutines that should be woken up when we yield or terminate */ > QTAILQ_HEAD(, Coroutine) co_queue_wakeup; > QTAILQ_ENTRY(Coroutine) co_queue_next; > diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c > index e4860ae..7c69ff6 100644 > --- a/qemu-coroutine-lock.c > +++ b/qemu-coroutine-lock.c > @@ -82,13 +82,13 @@ static bool qemu_co_queue_do_restart(CoQueue *queue, bool single) > > bool coroutine_fn qemu_co_queue_next(CoQueue *queue) > { > - assert(qemu_in_coroutine()); > + assert(qemu_in_coroutine() || qemu_coroutine_self_bypassed()); > return qemu_co_queue_do_restart(queue, true); > } > > void coroutine_fn qemu_co_queue_restart_all(CoQueue *queue) > { > - assert(qemu_in_coroutine()); > + assert(qemu_in_coroutine() || qemu_coroutine_self_bypassed()); > qemu_co_queue_do_restart(queue, false); > } > > diff --git a/qemu-coroutine.c b/qemu-coroutine.c > index 4708521..0597ed9 100644 > --- a/qemu-coroutine.c > +++ b/qemu-coroutine.c > @@ -137,3 +137,36 @@ void coroutine_fn qemu_coroutine_yield(void) > self->caller = NULL; > coroutine_swap(self, to); > } > + > +void qemu_coroutine_set_bypass(bool bypass) > +{ > + Coroutine *self = qemu_coroutine_self(); > + > + self->bypass = bypass; > +} > + > +bool qemu_coroutine_bypassed(Coroutine *self) > +{ > + return self->bypass; > +} > + > +bool qemu_coroutine_self_bypassed(void) > +{ > + Coroutine *self = qemu_coroutine_self(); > + > + return qemu_coroutine_bypassed(self); > +} > + > +void qemu_coroutine_set_var(void *var) > +{ > + Coroutine *self = qemu_coroutine_self(); > + > + self->opaque = var; > +} > + > +void *qemu_coroutine_get_var(void) > +{ > + Coroutine *self = qemu_coroutine_self(); > + > + return self->opaque; > +} >