From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Hajnoczi Subject: [RFC v9 19/27] virtio-blk: Disable guest->host notifies while processing vring Date: Wed, 18 Jul 2012 16:07:46 +0100 Message-ID: <1342624074-24650-20-git-send-email-stefanha@linux.vnet.ibm.com> References: <1342624074-24650-1-git-send-email-stefanha@linux.vnet.ibm.com> Cc: , Anthony Liguori , Kevin Wolf , Paolo Bonzini , "Michael S. Tsirkin" , Asias He , Khoa Huynh , Stefan Hajnoczi To: Return-path: Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:38329 "EHLO e06smtp17.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753967Ab2GRPIp (ORCPT ); Wed, 18 Jul 2012 11:08:45 -0400 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 18 Jul 2012 16:08:43 +0100 Received: from d06av09.portsmouth.uk.ibm.com (d06av09.portsmouth.uk.ibm.com [9.149.37.250]) by d06nrmr1806.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q6IF8gEk2904104 for ; Wed, 18 Jul 2012 16:08:42 +0100 Received: from d06av09.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av09.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q6IF8eLS013285 for ; Wed, 18 Jul 2012 09:08:41 -0600 In-Reply-To: <1342624074-24650-1-git-send-email-stefanha@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: --- hw/dataplane/vring.h | 28 +++++++++++++++++++++++----- hw/virtio-blk.c | 47 +++++++++++++++++++++++++++++++++++------------ 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/hw/dataplane/vring.h b/hw/dataplane/vring.h index 44ef4a9..cdd4d4a 100644 --- a/hw/dataplane/vring.h +++ b/hw/dataplane/vring.h @@ -69,11 +69,29 @@ static void vring_setup(Vring *vring, VirtIODevice *vdev, int n) vring->vr.desc, vring->vr.avail, vring->vr.used); } +/* Are there more descriptors available? */ static bool vring_more_avail(Vring *vring) { return vring->vr.avail->idx != vring->last_avail_idx; } +/* Hint to disable guest->host notifies */ +static void vring_disable_cb(Vring *vring) +{ + vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY; +} + +/* Re-enable guest->host notifies + * + * Returns false if there are more descriptors in the ring. + */ +static bool vring_enable_cb(Vring *vring) +{ + vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY; + __sync_synchronize(); /* mb() */ + return !vring_more_avail(vring); +} + /* This is stolen from linux-2.6/drivers/vhost/vhost.c. */ static bool get_indirect(Vring *vring, struct iovec iov[], struct iovec *iov_end, @@ -160,7 +178,7 @@ static bool get_indirect(Vring *vring, * * Stolen from linux-2.6/drivers/vhost/vhost.c. */ -static unsigned int vring_pop(Vring *vring, +static int vring_pop(Vring *vring, struct iovec iov[], struct iovec *iov_end, unsigned int *out_num, unsigned int *in_num) { @@ -178,9 +196,9 @@ static unsigned int vring_pop(Vring *vring, exit(1); } - /* If there's nothing new since last we looked, return invalid. */ + /* If there's nothing new since last we looked. */ if (avail_idx == last_avail_idx) - return num; + return -EAGAIN; /* Only get avail ring entries after they have been exposed by guest. */ __sync_synchronize(); /* smp_rmb() */ @@ -215,7 +233,7 @@ static unsigned int vring_pop(Vring *vring, desc = vring->vr.desc[i]; if (desc.flags & VRING_DESC_F_INDIRECT) { if (!get_indirect(vring, iov, iov_end, out_num, in_num, &desc)) { - return num; /* not enough iovecs, stop for now */ + return -ENOBUFS; /* not enough iovecs, stop for now */ } continue; } @@ -225,7 +243,7 @@ static unsigned int vring_pop(Vring *vring, * with the current set. */ if (iov >= iov_end) { - return num; + return -ENOBUFS; } iov->iov_base = phys_to_host(vring, desc.addr); diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index efeffa0..f67fdb7 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -202,7 +202,8 @@ static bool handle_notify(EventHandler *handler) * accept more I/O. This is not implemented yet. */ struct iovec iovec[VRING_MAX]; - struct iovec *iov, *end = &iovec[VRING_MAX]; + struct iovec *end = &iovec[VRING_MAX]; + struct iovec *iov = iovec; /* When a request is read from the vring, the index of the first descriptor * (aka head) is returned so that the completed request can be pushed onto @@ -211,19 +212,41 @@ static bool handle_notify(EventHandler *handler) * The number of hypervisor read-only iovecs is out_num. The number of * hypervisor write-only iovecs is in_num. */ - unsigned int head, out_num = 0, in_num = 0; + int head; + unsigned int out_num = 0, in_num = 0; - for (iov = iovec; ; iov += out_num + in_num) { - head = vring_pop(&s->vring, iov, end, &out_num, &in_num); - if (head >= vring_get_num(&s->vring)) { - break; /* no more requests */ - } + for (;;) { + /* Disable guest->host notifies to avoid unnecessary vmexits */ + vring_disable_cb(&s->vring); + + for (;;) { + head = vring_pop(&s->vring, iov, end, &out_num, &in_num); + if (head < 0) { + break; /* no more requests */ + } - /* - fprintf(stderr, "out_num=%u in_num=%u head=%u\n", out_num, in_num, head); - */ + /* + fprintf(stderr, "out_num=%u in_num=%u head=%d\n", out_num, in_num, head); + */ - process_request(&s->ioqueue, iov, out_num, in_num, head); + process_request(&s->ioqueue, iov, out_num, in_num, head); + iov += out_num + in_num; + } + + if (likely(head == -EAGAIN)) { /* vring emptied */ + /* Re-enable guest->host notifies and stop processing the vring. + * But if the guest has snuck in more descriptors, keep processing. + */ + if (likely(vring_enable_cb(&s->vring))) { + break; + } + } else { /* head == -ENOBUFS, cannot continue since iovecs[] is depleted */ + /* Since there are no iovecs[] left, stop processing for now. Do + * not re-enable guest->host notifies since the I/O completion + * handler knows to check for more vring descriptors anyway. + */ + break; + } } /* Submit requests, if any */ @@ -247,7 +270,7 @@ static bool handle_io(EventHandler *handler) * so check again. There should now be enough resources to process more * requests. */ - if (vring_more_avail(&s->vring)) { + if (unlikely(vring_more_avail(&s->vring))) { return handle_notify(&s->notify_handler); } -- 1.7.10.4 From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53112) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SrVry-00069p-Dn for qemu-devel@nongnu.org; Wed, 18 Jul 2012 11:08:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SrVro-0000mN-Cx for qemu-devel@nongnu.org; Wed, 18 Jul 2012 11:08:54 -0400 Received: from e06smtp11.uk.ibm.com ([195.75.94.107]:34176) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SrVro-0000lq-1o for qemu-devel@nongnu.org; Wed, 18 Jul 2012 11:08:44 -0400 Received: from /spool/local by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 18 Jul 2012 16:08:43 +0100 Received: from d06av09.portsmouth.uk.ibm.com (d06av09.portsmouth.uk.ibm.com [9.149.37.250]) by d06nrmr1407.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q6IF8ek52461798 for ; Wed, 18 Jul 2012 16:08:40 +0100 Received: from d06av09.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av09.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q6IF8eLM013285 for ; Wed, 18 Jul 2012 09:08:40 -0600 From: Stefan Hajnoczi Date: Wed, 18 Jul 2012 16:07:46 +0100 Message-Id: <1342624074-24650-20-git-send-email-stefanha@linux.vnet.ibm.com> In-Reply-To: <1342624074-24650-1-git-send-email-stefanha@linux.vnet.ibm.com> References: <1342624074-24650-1-git-send-email-stefanha@linux.vnet.ibm.com> Subject: [Qemu-devel] [RFC v9 19/27] virtio-blk: Disable guest->host notifies while processing vring List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Kevin Wolf , Anthony Liguori , Stefan Hajnoczi , kvm@vger.kernel.org, "Michael S. Tsirkin" , Khoa Huynh , Paolo Bonzini , Asias He --- hw/dataplane/vring.h | 28 +++++++++++++++++++++++----- hw/virtio-blk.c | 47 +++++++++++++++++++++++++++++++++++------------ 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/hw/dataplane/vring.h b/hw/dataplane/vring.h index 44ef4a9..cdd4d4a 100644 --- a/hw/dataplane/vring.h +++ b/hw/dataplane/vring.h @@ -69,11 +69,29 @@ static void vring_setup(Vring *vring, VirtIODevice *vdev, int n) vring->vr.desc, vring->vr.avail, vring->vr.used); } +/* Are there more descriptors available? */ static bool vring_more_avail(Vring *vring) { return vring->vr.avail->idx != vring->last_avail_idx; } +/* Hint to disable guest->host notifies */ +static void vring_disable_cb(Vring *vring) +{ + vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY; +} + +/* Re-enable guest->host notifies + * + * Returns false if there are more descriptors in the ring. + */ +static bool vring_enable_cb(Vring *vring) +{ + vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY; + __sync_synchronize(); /* mb() */ + return !vring_more_avail(vring); +} + /* This is stolen from linux-2.6/drivers/vhost/vhost.c. */ static bool get_indirect(Vring *vring, struct iovec iov[], struct iovec *iov_end, @@ -160,7 +178,7 @@ static bool get_indirect(Vring *vring, * * Stolen from linux-2.6/drivers/vhost/vhost.c. */ -static unsigned int vring_pop(Vring *vring, +static int vring_pop(Vring *vring, struct iovec iov[], struct iovec *iov_end, unsigned int *out_num, unsigned int *in_num) { @@ -178,9 +196,9 @@ static unsigned int vring_pop(Vring *vring, exit(1); } - /* If there's nothing new since last we looked, return invalid. */ + /* If there's nothing new since last we looked. */ if (avail_idx == last_avail_idx) - return num; + return -EAGAIN; /* Only get avail ring entries after they have been exposed by guest. */ __sync_synchronize(); /* smp_rmb() */ @@ -215,7 +233,7 @@ static unsigned int vring_pop(Vring *vring, desc = vring->vr.desc[i]; if (desc.flags & VRING_DESC_F_INDIRECT) { if (!get_indirect(vring, iov, iov_end, out_num, in_num, &desc)) { - return num; /* not enough iovecs, stop for now */ + return -ENOBUFS; /* not enough iovecs, stop for now */ } continue; } @@ -225,7 +243,7 @@ static unsigned int vring_pop(Vring *vring, * with the current set. */ if (iov >= iov_end) { - return num; + return -ENOBUFS; } iov->iov_base = phys_to_host(vring, desc.addr); diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index efeffa0..f67fdb7 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -202,7 +202,8 @@ static bool handle_notify(EventHandler *handler) * accept more I/O. This is not implemented yet. */ struct iovec iovec[VRING_MAX]; - struct iovec *iov, *end = &iovec[VRING_MAX]; + struct iovec *end = &iovec[VRING_MAX]; + struct iovec *iov = iovec; /* When a request is read from the vring, the index of the first descriptor * (aka head) is returned so that the completed request can be pushed onto @@ -211,19 +212,41 @@ static bool handle_notify(EventHandler *handler) * The number of hypervisor read-only iovecs is out_num. The number of * hypervisor write-only iovecs is in_num. */ - unsigned int head, out_num = 0, in_num = 0; + int head; + unsigned int out_num = 0, in_num = 0; - for (iov = iovec; ; iov += out_num + in_num) { - head = vring_pop(&s->vring, iov, end, &out_num, &in_num); - if (head >= vring_get_num(&s->vring)) { - break; /* no more requests */ - } + for (;;) { + /* Disable guest->host notifies to avoid unnecessary vmexits */ + vring_disable_cb(&s->vring); + + for (;;) { + head = vring_pop(&s->vring, iov, end, &out_num, &in_num); + if (head < 0) { + break; /* no more requests */ + } - /* - fprintf(stderr, "out_num=%u in_num=%u head=%u\n", out_num, in_num, head); - */ + /* + fprintf(stderr, "out_num=%u in_num=%u head=%d\n", out_num, in_num, head); + */ - process_request(&s->ioqueue, iov, out_num, in_num, head); + process_request(&s->ioqueue, iov, out_num, in_num, head); + iov += out_num + in_num; + } + + if (likely(head == -EAGAIN)) { /* vring emptied */ + /* Re-enable guest->host notifies and stop processing the vring. + * But if the guest has snuck in more descriptors, keep processing. + */ + if (likely(vring_enable_cb(&s->vring))) { + break; + } + } else { /* head == -ENOBUFS, cannot continue since iovecs[] is depleted */ + /* Since there are no iovecs[] left, stop processing for now. Do + * not re-enable guest->host notifies since the I/O completion + * handler knows to check for more vring descriptors anyway. + */ + break; + } } /* Submit requests, if any */ @@ -247,7 +270,7 @@ static bool handle_io(EventHandler *handler) * so check again. There should now be enough resources to process more * requests. */ - if (vring_more_avail(&s->vring)) { + if (unlikely(vring_more_avail(&s->vring))) { return handle_notify(&s->notify_handler); } -- 1.7.10.4