From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUuVm-0003m8-AL for qemu-devel@nongnu.org; Fri, 19 Sep 2014 05:30:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XUuVc-0004FL-Og for qemu-devel@nongnu.org; Fri, 19 Sep 2014 05:29:54 -0400 Received: from mail-wi0-x22a.google.com ([2a00:1450:400c:c05::22a]:40805) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUuVc-0004Dd-5j for qemu-devel@nongnu.org; Fri, 19 Sep 2014 05:29:44 -0400 Received: by mail-wi0-f170.google.com with SMTP id em10so755503wid.3 for ; Fri, 19 Sep 2014 02:29:38 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <541BF77C.3090808@redhat.com> Date: Fri, 19 Sep 2014 11:29:32 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1407303308-4615-1-git-send-email-famz@redhat.com> <1407303308-4615-10-git-send-email-famz@redhat.com> In-Reply-To: <1407303308-4615-10-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: kwolf@redhat.com, stefanha@redhat.com Il 06/08/2014 07:35, Fam Zheng ha scritto: > This implements the core part of dataplane feature of virtio-scsi. > > A few fields are added in VirtIOSCSICommon to maintain the dataplane > status. These fields are managed by a new source file: > virtio-scsi-dataplane.c. > > Most code in this file will run on an iothread, unless otherwise > commented as in a global mutex context, such as those functions to > start, stop and setting the iothread property. > > Upon start, we set up guest/host event notifiers, in a same way as > virtio-blk does. The handlers then pop request from vring and call into > virtio-scsi.c functions to process it. So we need to make sure make all > those called functions work with iothread, too. > > Signed-off-by: Fam Zheng > --- > hw/scsi/Makefile.objs | 2 +- > hw/scsi/virtio-scsi-dataplane.c | 219 ++++++++++++++++++++++++++++++++++++++++ > include/hw/virtio/virtio-scsi.h | 19 ++++ > 3 files changed, 239 insertions(+), 1 deletion(-) > create mode 100644 hw/scsi/virtio-scsi-dataplane.c > > diff --git a/hw/scsi/Makefile.objs b/hw/scsi/Makefile.objs > index 121ddc5..40c79d3 100644 > --- a/hw/scsi/Makefile.objs > +++ b/hw/scsi/Makefile.objs > @@ -8,6 +8,6 @@ common-obj-$(CONFIG_ESP_PCI) += esp-pci.o > obj-$(CONFIG_PSERIES) += spapr_vscsi.o > > ifeq ($(CONFIG_VIRTIO),y) > -obj-y += virtio-scsi.o > +obj-y += virtio-scsi.o virtio-scsi-dataplane.o > obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o > endif > diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c > new file mode 100644 > index 0000000..d077b67 > --- /dev/null > +++ b/hw/scsi/virtio-scsi-dataplane.c > @@ -0,0 +1,219 @@ > +/* > + * Virtio SCSI dataplane > + * > + * Copyright Red Hat, Inc. 2014 > + * > + * Authors: > + * Fam Zheng > + * > + * 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/virtio/virtio-scsi.h" > +#include "qemu/error-report.h" > +#include > +#include > +#include > +#include "hw/virtio/virtio-access.h" > +#include "stdio.h" > + > +/* Context: QEMU global mutex held */ > +void virtio_scsi_set_iothread(VirtIOSCSICommon *s, IOThread *iothread) > +{ > + BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s))); > + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > + > + s->ctx = iothread_get_aio_context(s->conf.iothread); assert that it's NULL? > + > + /* Don't try if transport does not support notifiers. */ > + if (!k->set_guest_notifiers || !k->set_host_notifier) { > + fprintf(stderr, "virtio-scsi: Failed to set iothread " > + "(transport does not support notifiers)"); > + exit(1); > + } > +} > + > +static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSICommon *s, > + VirtQueue *vq, > + EventNotifierHandler *handler, > + int n) > +{ > + BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s))); > + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > + VirtIOSCSIVring *r = g_slice_new(VirtIOSCSIVring); > + > + /* Set up virtqueue notify */ > + if (k->set_host_notifier(qbus->parent, n, true) != 0) { > + fprintf(stderr, "virtio-scsi: Failed to set host notifier\n"); > + exit(1); > + } > + r->host_notifier = *virtio_queue_get_host_notifier(vq); > + r->guest_notifier = *virtio_queue_get_guest_notifier(vq); > + aio_set_event_notifier(s->ctx, &r->host_notifier, handler); > + > + r->parent = s; > + > + if (!vring_setup(&r->vring, VIRTIO_DEVICE(s), n)) { > + fprintf(stderr, "virtio-scsi: VRing setup failed\n"); > + exit(1); > + } > + return r; > +} > + > +VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s, > + VirtIOSCSIVring *vring) > +{ > + VirtIOSCSIReq *req = virtio_scsi_init_req(s, NULL); > + int r; > + > + req->vring = vring; > + r = vring_pop((VirtIODevice *)s, &vring->vring, &req->elem); > + if (r < 0) { > + virtio_scsi_free_req(req); > + req = NULL; > + } > + return req; > +} > + > +void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req) > +{ > + vring_push(&req->vring->vring, &req->elem, > + req->qsgl.size + req->resp_iov.size); > + event_notifier_set(&req->vring->guest_notifier); > +} > + > +static void virtio_scsi_iothread_handle_ctrl(EventNotifier *notifier) > +{ > + VirtIOSCSIVring *vring = container_of(notifier, > + VirtIOSCSIVring, host_notifier); > + VirtIOSCSI *s = VIRTIO_SCSI(vring->parent); > + VirtIOSCSIReq *req; > + > + event_notifier_test_and_clear(notifier); > + while ((req = virtio_scsi_pop_req_vring(s, vring))) { > + virtio_scsi_handle_ctrl_req(s, req); > + } > +} > + > +static void virtio_scsi_iothread_handle_event(EventNotifier *notifier) > +{ > + VirtIOSCSIVring *vring = container_of(notifier, > + VirtIOSCSIVring, host_notifier); > + VirtIOSCSICommon *vs = vring->parent; > + VirtIOSCSI *s = VIRTIO_SCSI(vs); > + VirtIODevice *vdev = VIRTIO_DEVICE(s); > + > + event_notifier_test_and_clear(notifier); > + > + if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) { > + return; > + } > + > + if (s->events_dropped) { > + virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0); > + } > +} > + > +static void virtio_scsi_iothread_handle_cmd(EventNotifier *notifier) > +{ > + VirtIOSCSIVring *vring = container_of(notifier, > + VirtIOSCSIVring, host_notifier); > + VirtIOSCSI *s = VIRTIO_SCSI(vring->parent); > + VirtIOSCSIReq *req; > + > + event_notifier_test_and_clear(notifier); > + while ((req = virtio_scsi_pop_req_vring(s, vring))) { > + virtio_scsi_handle_cmd_req(s, req); > + } Perhaps add bdrv_io_plug/bdrv_io_unplug? > +} > + > +/* Context: QEMU global mutex held */ > +void virtio_scsi_dataplane_start(VirtIOSCSICommon *s) > +{ > + int i; > + int rc; > + BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s))); > + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > + > + if (s->dataplane_started || > + s->dataplane_starting || > + s->ctx != iothread_get_aio_context(s->conf.iothread)) { > + return; > + } > + > + s->dataplane_starting = true; Please do a bdrv_drain_all() here. Then you can set the contexts for all children here too (for the hotplug case: in virtio_scsi_hotplug). Then you do not have to do it in virtio_scsi_do_tmf and virtio_scsi_handle_cmd_req. > + /* Set up guest notifier (irq) */ > + rc = k->set_guest_notifiers(qbus->parent, s->conf.num_queues + 2, true); > + if (rc != 0) { > + fprintf(stderr, "virtio-scsi: Failed to set guest notifiers, " > + "ensure -enable-kvm is set\n"); > + exit(1); > + } > + > + aio_context_acquire(s->ctx); > + s->ctrl_vring = virtio_scsi_vring_init(s, s->ctrl_vq, > + virtio_scsi_iothread_handle_ctrl, > + 0); > + s->event_vring = virtio_scsi_vring_init(s, s->event_vq, > + virtio_scsi_iothread_handle_event, > + 1); > + s->cmd_vrings = g_malloc0(sizeof(VirtIOSCSIVring) * s->conf.num_queues); > + for (i = 0; i < s->conf.num_queues; i++) { > + s->cmd_vrings[i] = > + virtio_scsi_vring_init(s, s->cmd_vqs[i], > + virtio_scsi_iothread_handle_cmd, > + i + 2); > + } > + > + aio_context_release(s->ctx); > + s->dataplane_starting = false; > + s->dataplane_started = true; > +} > + > +/* Context: QEMU global mutex held */ > +void virtio_scsi_dataplane_stop(VirtIOSCSICommon *s) > +{ > + BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s))); > + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > + VirtIODevice *vdev = VIRTIO_DEVICE(s); > + int i; > + > + if (!s->dataplane_started || s->dataplane_stopping) { > + return; > + } > + s->dataplane_stopping = true; > + assert(s->ctx == iothread_get_aio_context(s->conf.iothread)); > + > + aio_context_acquire(s->ctx); > + > + aio_set_event_notifier(s->ctx, &s->ctrl_vring->host_notifier, NULL); > + aio_set_event_notifier(s->ctx, &s->event_vring->host_notifier, NULL); > + for (i = 0; i < s->conf.num_queues; i++) { > + aio_set_event_notifier(s->ctx, &s->cmd_vrings[i]->host_notifier, NULL); > + } > + > + bdrv_drain_all(); /* ensure there are no in-flight requests */ > + > + aio_context_release(s->ctx); > + > + /* Sync vring state back to virtqueue so that non-dataplane request > + * processing can continue when we disable the host notifier below. > + */ > + vring_teardown(&s->ctrl_vring->vring, vdev, 0); > + vring_teardown(&s->event_vring->vring, vdev, 1); > + for (i = 0; i < s->conf.num_queues; i++) { > + vring_teardown(&s->cmd_vrings[i]->vring, vdev, 2 + i); > + } > + > + for (i = 0; i < s->conf.num_queues + 2; i++) { > + k->set_host_notifier(qbus->parent, i, false); > + } > + > + /* Clean up guest notifier (irq) */ > + k->set_guest_notifiers(qbus->parent, s->conf.num_queues + 2, false); > + s->dataplane_stopping = false; > + s->dataplane_started = false; > +} > diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h > index 6f92c29..b9f2197 100644 > --- a/include/hw/virtio/virtio-scsi.h > +++ b/include/hw/virtio/virtio-scsi.h > @@ -174,6 +174,18 @@ typedef struct VirtIOSCSICommon { > VirtQueue *ctrl_vq; > VirtQueue *event_vq; > VirtQueue **cmd_vqs; > + > + /* Fields for dataplane below */ > + AioContext *ctx; /* one iothread per virtio-scsi-pci for now */ > + > + /* Vring is used instead of vq in dataplane code, because of the underlying > + * memory layer thread safety */ > + VirtIOSCSIVring *ctrl_vring; > + VirtIOSCSIVring *event_vring; > + VirtIOSCSIVring **cmd_vrings; > + bool dataplane_started; > + bool dataplane_starting; > + bool dataplane_stopping; Please add these to VirtIOSCSI rather than VirtIOSCSICommon. Same for the new functions you declare below. Paolo > } VirtIOSCSICommon; > > typedef struct { > @@ -239,4 +251,11 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req); > void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, > uint32_t event, uint32_t reason); > > +void virtio_scsi_set_iothread(VirtIOSCSICommon *s, IOThread *iothread); > +void virtio_scsi_dataplane_start(VirtIOSCSICommon *s); > +void virtio_scsi_dataplane_stop(VirtIOSCSICommon *s); > +void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req); > +VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s, > + VirtIOSCSIVring *vring); > + > #endif /* _QEMU_VIRTIO_SCSI_H */ >