From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39763) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2dVQ-0005jH-Dw for qemu-devel@nongnu.org; Thu, 03 Jul 2014 05:40:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X2dVK-0000pD-Jz for qemu-devel@nongnu.org; Thu, 03 Jul 2014 05:40:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56420) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2dVK-0000ot-Bt for qemu-devel@nongnu.org; Thu, 03 Jul 2014 05:40:34 -0400 Date: Thu, 3 Jul 2014 11:40:09 +0200 From: Kevin Wolf Message-ID: <20140703094009.GC4322@noname.redhat.com> References: <1404303528-7115-1-git-send-email-ming.lei@canonical.com> <1404303528-7115-3-git-send-email-ming.lei@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1404303528-7115-3-git-send-email-ming.lei@canonical.com> Subject: Re: [Qemu-devel] [PATCH v4 2/3] linux-aio: implement io plug, unplug and flush io queue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei Cc: Peter Maydell , Fam Zheng , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini Am 02.07.2014 um 14:18 hat Ming Lei geschrieben: > This patch implements .bdrv_io_plug, .bdrv_io_unplug and > .bdrv_flush_io_queue callbacks for linux-aio Block Drivers, > so that submitting I/O as a batch can be supported on linux-aio. > > Signed-off-by: Ming Lei Just a couple of minor comments, see inline. > block/linux-aio.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++-- > block/raw-aio.h | 2 ++ > block/raw-posix.c | 45 +++++++++++++++++++++++++++ > 3 files changed, 133 insertions(+), 2 deletions(-) > > diff --git a/block/linux-aio.c b/block/linux-aio.c > index f0a2c08..1cb4845 100644 > --- a/block/linux-aio.c > +++ b/block/linux-aio.c > @@ -25,6 +25,8 @@ > */ > #define MAX_EVENTS 128 > > +#define MAX_QUEUED_IO 128 > + > struct qemu_laiocb { > BlockDriverAIOCB common; > struct qemu_laio_state *ctx; > @@ -36,9 +38,19 @@ struct qemu_laiocb { > QLIST_ENTRY(qemu_laiocb) node; > }; > > +struct laio_queue { > + struct iocb *iocbs[MAX_QUEUED_IO]; > + int plugged; Why signed? > + unsigned int size; > + unsigned int idx; > +}; > + > struct qemu_laio_state { > io_context_t ctx; > EventNotifier e; > + > + /* io queue for submit at batch */ > + struct laio_queue io_q; > }; > > static inline ssize_t io_event_ret(struct io_event *ev) > @@ -135,6 +147,72 @@ static const AIOCBInfo laio_aiocb_info = { > .cancel = laio_cancel, > }; > > +static void ioq_init(struct laio_queue *io_q) > +{ > + io_q->size = MAX_QUEUED_IO; > + io_q->idx = 0; > + io_q->plugged = 0; > +} > + > +static int ioq_submit(struct qemu_laio_state *s) > +{ > + int ret, i = 0; > + int len = s->io_q.idx; > + > + do { > + ret = io_submit(s->ctx, len, s->io_q.iocbs); > + } while (i++ < 3 && ret == -EAGAIN); > + > + /* empty io queue */ > + s->io_q.idx = 0; > + > + if (ret >= 0) > + return 0; Indentation is off and braces are missing. > + > + for (i = 0; i < len; i++) { > + struct qemu_laiocb *laiocb = > + container_of(s->io_q.iocbs[i], struct qemu_laiocb, iocb); > + > + laiocb->ret = ret; > + qemu_laio_process_completion(s, laiocb); > + } > + return ret; > +} > + > +static void ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb) > +{ > + unsigned int idx = s->io_q.idx; > + > + s->io_q.iocbs[idx++] = iocb; > + s->io_q.idx = idx; > + > + /* submit immediately if queue is full */ > + if (idx == s->io_q.size) > + ioq_submit(s); Missing braces. > +} > + > +void laio_io_plug(BlockDriverState *bs, void *aio_ctx) > +{ > + struct qemu_laio_state *s = aio_ctx; > + > + s->io_q.plugged++; > +} > + > +int laio_io_unplug(BlockDriverState *bs, void *aio_ctx, bool unplug) > +{ > + struct qemu_laio_state *s = aio_ctx; > + int ret = 0; > + How about an assert(s->io_q.plugged > 0); here? > + if (unplug && --s->io_q.plugged > 0) > + return 0; Missing braces. > + > + if (s->io_q.idx > 0) { > + ret = ioq_submit(s); > + } > + > + return ret; > +} > + > BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd, > int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, > BlockDriverCompletionFunc *cb, void *opaque, int type)