From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47713) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XnoWt-0002A5-MK for qemu-devel@nongnu.org; Mon, 10 Nov 2014 07:57:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XnoWo-0007RR-Eq for qemu-devel@nongnu.org; Mon, 10 Nov 2014 07:57:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55352) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XnoWo-0007RE-7M for qemu-devel@nongnu.org; Mon, 10 Nov 2014 07:57:06 -0500 Message-ID: <5460B617.7020107@redhat.com> Date: Mon, 10 Nov 2014 13:56:55 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1415548218-25193-1-git-send-email-ming.lei@canonical.com> <1415548218-25193-3-git-send-email-ming.lei@canonical.com> In-Reply-To: <1415548218-25193-3-git-send-email-ming.lei@canonical.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] virtio-scsi-dataplane: notify guest as batch List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei , qemu-devel@nongnu.org, Stefan Hajnoczi , Kevin Wolf Cc: Fam Zheng , Anthony Liguori , "Michael S. Tsirkin" On 09/11/2014 16:50, Ming Lei wrote: > It isn't necessery to notify guest each time when one request > is completed, and it should be enough to just notify one time > for each running of virtio_scsi_iothread_handle_cmd(). > > This patch supresses about 30K/sec write on eventfd. > > Signed-off-by: Ming Lei > --- > hw/scsi/virtio-scsi-dataplane.c | 4 +++- > hw/scsi/virtio-scsi.c | 22 ++++++++++++++++++++++ > include/hw/virtio/virtio-scsi.h | 4 ++++ > 3 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c > index 8a7cd9f..1fb83de 100644 > --- a/hw/scsi/virtio-scsi-dataplane.c > +++ b/hw/scsi/virtio-scsi-dataplane.c > @@ -62,6 +62,7 @@ static VirtIOSCSIVring *virtio_scsi_vring_init(VirtIOSCSI *s, > aio_set_event_notifier(s->ctx, &r->host_notifier, handler); > > r->parent = s; > + r->qid = n; > > if (!vring_setup(&r->vring, VIRTIO_DEVICE(s), n)) { > fprintf(stderr, "virtio-scsi: VRing setup failed\n"); > @@ -95,7 +96,8 @@ 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); > + req->dev->pending_guest_notify |= 1 << (req->vring->qid - 2); > + qemu_bh_schedule(req->dev->guest_notify_bh); > } > > static void virtio_scsi_iothread_handle_ctrl(EventNotifier *notifier) > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index 6e34a2c..1b9c35c 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -86,6 +86,21 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req) > virtio_scsi_free_req(req); > } > > +static void notify_guest_bh(void *opaque) > +{ > + VirtIOSCSI *s = opaque; > + unsigned int qid; > + uint64_t pending = s->pending_guest_notify; > + > + s->pending_guest_notify = 0; > + > + while ((qid = ffsl(pending))) { > + qid--; > + event_notifier_set(&s->cmd_vrings[qid]->guest_notifier); > + pending &= ~(1 << qid); > + } > +} > + > static void virtio_scsi_bad_req(void) > { > error_report("wrong size for virtio-scsi headers"); > @@ -825,6 +840,10 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp, > > if (s->conf.iothread) { > virtio_scsi_set_iothread(VIRTIO_SCSI(s), s->conf.iothread); > + VIRTIO_SCSI(s)->pending_guest_notify = 0; > + VIRTIO_SCSI(s)->guest_notify_bh = aio_bh_new(VIRTIO_SCSI(s)->ctx, > + notify_guest_bh, > + VIRTIO_SCSI(s)); Please use a separete variable to host VIRTIO_SCSI(s). This is usually the standard if the result of the cast is dereferenced. > } > } > > @@ -902,6 +921,9 @@ void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp) > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev); > > + if (vs->conf.iothread) { > + qemu_bh_delete(VIRTIO_SCSI(vs)->guest_notify_bh); Here too. > + } > g_free(vs->cmd_vqs); > virtio_cleanup(vdev); > } > diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h > index 9e1a49c..5e6c57e 100644 > --- a/include/hw/virtio/virtio-scsi.h > +++ b/include/hw/virtio/virtio-scsi.h > @@ -163,6 +163,7 @@ typedef struct { > Vring vring; > EventNotifier host_notifier; > EventNotifier guest_notifier; > + uint32_t qid; > } VirtIOSCSIVring; > > typedef struct VirtIOSCSICommon { > @@ -198,6 +199,9 @@ typedef struct VirtIOSCSI { > bool dataplane_fenced; > Error *blocker; > Notifier migration_state_notifier; > + > + QEMUBH *guest_notify_bh; > + uint64_t pending_guest_notify; Please add a BUILD_BUG_ON(VIRTIO_PCI_QUEUE_MAX <= 64); too. > } VirtIOSCSI; > > typedef struct VirtIOSCSIReq { > Thanks, Paolo