From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39000) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XwXX7-0005mD-Ld for qemu-devel@nongnu.org; Thu, 04 Dec 2014 09:37:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XwXX1-0006pP-FN for qemu-devel@nongnu.org; Thu, 04 Dec 2014 09:37:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36742) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XwXX1-0006oy-68 for qemu-devel@nongnu.org; Thu, 04 Dec 2014 09:37:23 -0500 Date: Thu, 4 Dec 2014 15:37:10 +0100 From: Kevin Wolf Message-ID: <20141204143710.GE5129@noname.redhat.com> References: <1417013204-30676-1-git-send-email-kwolf@redhat.com> <1417013204-30676-4-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1417013204-30676-4-git-send-email-kwolf@redhat.com> Subject: Re: [Qemu-devel] [RFC PATCH 3/3] linux-aio: Don't reenter request coroutine recursively List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: pbonzini@redhat.com, ming.lei@canonical.com, pl@kamp.de, stefanha@redhat.com Am 26.11.2014 um 15:46 hat Kevin Wolf geschrieben: > When getting an error while submitting requests, we must be careful to > wake up only inactive coroutines. Therefore we must special-case the > currently active coroutine and communicate an error for that request > using the ordinary return value of ioq_submit(). > > Signed-off-by: Kevin Wolf > --- > block/linux-aio.c | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) Ming, did you have a look at this patch specifically? Does it fix the issue that you tried to address with a much more complex callback-based patch? I'd like to go forward with this as both Peter and I have measured considerable performance improvements with our optimisation proposals, and this series is an important part of it. Kevin > diff --git a/block/linux-aio.c b/block/linux-aio.c > index 99b259d..fd8f0e4 100644 > --- a/block/linux-aio.c > +++ b/block/linux-aio.c > @@ -177,12 +177,19 @@ static int ioq_submit(struct qemu_laio_state *s) > container_of(s->io_q.iocbs[i], struct qemu_laiocb, iocb); > > laiocb->ret = (ret < 0) ? ret : -EIO; > - qemu_laio_process_completion(s, laiocb); > + if (laiocb->co != qemu_coroutine_self()) { > + qemu_coroutine_enter(laiocb->co, NULL); > + } else { > + /* The return value is used for the currently active coroutine. > + * We're always in ioq_enqueue() here, ioq_submit() never runs from > + * a request's coroutine.*/ > + ret = laiocb->ret; > + } > } > return ret; > } > > -static void ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb) > +static int ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb) > { > unsigned int idx = s->io_q.idx; > > @@ -191,7 +198,9 @@ static void ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb) > > /* submit immediately if queue is full */ > if (idx == s->io_q.size) { > - ioq_submit(s); > + return ioq_submit(s); > + } else { > + return 0; > } > } > > @@ -253,11 +262,11 @@ int laio_submit_co(BlockDriverState *bs, void *aio_ctx, int fd, > > if (!s->io_q.plugged) { > ret = io_submit(s->ctx, 1, &iocbs); > - if (ret < 0) { > - return ret; > - } > } else { > - ioq_enqueue(s, iocbs); > + ret = ioq_enqueue(s, iocbs); > + } > + if (ret < 0) { > + return ret; > } > > qemu_coroutine_yield(); > -- > 1.8.3.1 >