From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35031) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tgynw-0003w8-NY for qemu-devel@nongnu.org; Fri, 07 Dec 2012 09:21:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tgynv-0003IR-DX for qemu-devel@nongnu.org; Fri, 07 Dec 2012 09:21:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:24121) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tgynv-0003IN-5p for qemu-devel@nongnu.org; Fri, 07 Dec 2012 09:21:27 -0500 Message-ID: <50C1FB62.3040401@redhat.com> Date: Fri, 07 Dec 2012 15:21:22 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1354740430-22452-1-git-send-email-stefanha@redhat.com> <1354740430-22452-7-git-send-email-stefanha@redhat.com> In-Reply-To: <1354740430-22452-7-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 06/11] dataplane: add Linux AIO request queue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Anthony Liguori , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Blue Swirl , khoa@us.ibm.com, Paolo Bonzini , asias@redhat.com Am 05.12.2012 21:47, schrieb Stefan Hajnoczi: > The IOQueue has a pool of iocb structs and a function to add new > read/write requests. Multiple requests can be added before calling the > submit function to actually tell the host kernel to begin I/O. This > allows callers to batch requests and submit them in one go. > > The actual I/O is performed using Linux AIO. > > Signed-off-by: Stefan Hajnoczi > --- > hw/dataplane/Makefile.objs | 2 +- > hw/dataplane/ioq.c | 118 +++++++++++++++++++++++++++++++++++++++++++++ > hw/dataplane/ioq.h | 57 ++++++++++++++++++++++ > 3 files changed, 176 insertions(+), 1 deletion(-) > create mode 100644 hw/dataplane/ioq.c > create mode 100644 hw/dataplane/ioq.h > > diff --git a/hw/dataplane/Makefile.objs b/hw/dataplane/Makefile.objs > index e26bd7d..abd408f 100644 > --- a/hw/dataplane/Makefile.objs > +++ b/hw/dataplane/Makefile.objs > @@ -1,3 +1,3 @@ > ifeq ($(CONFIG_VIRTIO), y) > -common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += hostmem.o vring.o event-poll.o > +common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += hostmem.o vring.o event-poll.o ioq.o > endif > diff --git a/hw/dataplane/ioq.c b/hw/dataplane/ioq.c > new file mode 100644 > index 0000000..7adeb5d > --- /dev/null > +++ b/hw/dataplane/ioq.c > @@ -0,0 +1,118 @@ > +/* > + * Linux AIO request queue > + * > + * Copyright 2012 IBM, Corp. > + * Copyright 2012 Red Hat, Inc. and/or its affiliates > + * > + * Authors: > + * Stefan Hajnoczi > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "hw/dataplane/ioq.h" > + > +void ioq_init(IOQueue *ioq, int fd, unsigned int max_reqs) > +{ > + int rc; > + > + ioq->fd = fd; > + ioq->max_reqs = max_reqs; > + > + memset(&ioq->io_ctx, 0, sizeof ioq->io_ctx); > + rc = io_setup(max_reqs, &ioq->io_ctx); > + if (rc != 0) { > + fprintf(stderr, "ioq io_setup failed %d\n", rc); > + exit(1); > + } > + > + rc = event_notifier_init(&ioq->io_notifier, 0); > + if (rc != 0) { > + fprintf(stderr, "ioq io event notifier creation failed %d\n", rc); > + exit(1); > + } > + > + ioq->freelist = g_malloc0(sizeof ioq->freelist[0] * max_reqs); > + ioq->freelist_idx = 0; > + > + ioq->queue = g_malloc0(sizeof ioq->queue[0] * max_reqs); > + ioq->queue_idx = 0; > +} > + > +void ioq_cleanup(IOQueue *ioq) > +{ > + g_free(ioq->freelist); > + g_free(ioq->queue); > + > + event_notifier_cleanup(&ioq->io_notifier); > + io_destroy(ioq->io_ctx); > +} > + > +EventNotifier *ioq_get_notifier(IOQueue *ioq) > +{ > + return &ioq->io_notifier; > +} > + > +struct iocb *ioq_get_iocb(IOQueue *ioq) > +{ > + if (unlikely(ioq->freelist_idx == 0)) { > + fprintf(stderr, "ioq underflow\n"); > + exit(1); > + } Can this happen? If no, it should be an assertion. If yes, the error handling code is wrong, we can't just exit qemu. It's already not nice to do it in setup functions, but during runtime I think it's not acceptable. > + struct iocb *iocb = ioq->freelist[--ioq->freelist_idx]; > + ioq->queue[ioq->queue_idx++] = iocb; > + return iocb; > +} > + > +void ioq_put_iocb(IOQueue *ioq, struct iocb *iocb) > +{ > + if (unlikely(ioq->freelist_idx == ioq->max_reqs)) { > + fprintf(stderr, "ioq overflow\n"); > + exit(1); > + } Same here. Kevin