Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread

[Fam Zheng]

On Fri, 09/19 11:29, Paolo Bonzini wrote:
> Il 06/08/2014 07:35, Fam Zheng ha scritto:
> > +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.

Do we want multiple iothreads in the future? If yes, the aio context will need
to be checked/set for each command, that's why I put it in virtio_scsi_do_tmf
and virtio_scsi_handle_cmd_req.

Fam

> 
> > +    /* 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 */
> > 
>