On Mon, Dec 09, 2019 at 09:09:57PM +0000, Stefan Hajnoczi wrote: > Virtqueue notifications are not necessary during polling, so we disable > them. This allows the guest driver to avoid MMIO vmexits. > Unfortunately the virtio-blk and virtio-scsi handler functions re-enable > notifications, defeating this optimization. > > Fix virtio-blk and virtio-scsi emulation so they leave notifications > disabled. The key thing to remember for correctness is that polling > always checks one last time after ending its loop, therefore it's safe > to lose the race when re-enabling notifications at the end of polling. > > There is a measurable performance improvement of 5-10% with the null-co > block driver. Real-life storage configurations will see a smaller > improvement because the MMIO vmexit overhead contributes less to > latency. > > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
Thanks! I'll queue it for merge after the release. If possible please ping me after the release to help make sure it didn't get dropped. > --- > hw/block/virtio-blk.c | 9 +++++++-- > hw/scsi/virtio-scsi.c | 9 +++++++-- > hw/virtio/virtio.c | 12 ++++++------ > include/hw/virtio/virtio.h | 1 + > 4 files changed, 21 insertions(+), 10 deletions(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 4c357d2928..c4e55fb3de 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -764,13 +764,16 @@ bool virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) > { > VirtIOBlockReq *req; > MultiReqBuffer mrb = {}; > + bool suppress_notifications = virtio_queue_get_notification(vq); > bool progress = false; > > aio_context_acquire(blk_get_aio_context(s->blk)); > blk_io_plug(s->blk); > > do { > - virtio_queue_set_notification(vq, 0); > + if (suppress_notifications) { > + virtio_queue_set_notification(vq, 0); > + } > > while ((req = virtio_blk_get_request(s, vq))) { > progress = true; > @@ -781,7 +784,9 @@ bool virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) > } > } > > - virtio_queue_set_notification(vq, 1); > + if (suppress_notifications) { > + virtio_queue_set_notification(vq, 1); > + } > } while (!virtio_queue_empty(vq)); > > if (mrb.num_reqs) { > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index e8b2b64d09..f080545f48 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -597,12 +597,15 @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue > *vq) > { > VirtIOSCSIReq *req, *next; > int ret = 0; > + bool suppress_notifications = virtio_queue_get_notification(vq); > bool progress = false; > > QTAILQ_HEAD(, VirtIOSCSIReq) reqs = QTAILQ_HEAD_INITIALIZER(reqs); > > do { > - virtio_queue_set_notification(vq, 0); > + if (suppress_notifications) { > + virtio_queue_set_notification(vq, 0); > + } > > while ((req = virtio_scsi_pop_req(s, vq))) { > progress = true; > @@ -622,7 +625,9 @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue > *vq) > } > } > > - virtio_queue_set_notification(vq, 1); > + if (suppress_notifications) { > + virtio_queue_set_notification(vq, 1); > + } > } while (ret != -EINVAL && !virtio_queue_empty(vq)); > > QTAILQ_FOREACH_SAFE(req, &reqs, next, next) { > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 762df12f4c..78e5852296 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -431,6 +431,11 @@ static void > virtio_queue_packed_set_notification(VirtQueue *vq, int enable) > } > } > > +bool virtio_queue_get_notification(VirtQueue *vq) > +{ > + return vq->notification; > +} > + > void virtio_queue_set_notification(VirtQueue *vq, int enable) > { > vq->notification = enable; > @@ -3382,17 +3387,12 @@ static bool virtio_queue_host_notifier_aio_poll(void > *opaque) > { > EventNotifier *n = opaque; > VirtQueue *vq = container_of(n, VirtQueue, host_notifier); > - bool progress; > > if (!vq->vring.desc || virtio_queue_empty(vq)) { > return false; > } > > - progress = virtio_queue_notify_aio_vq(vq); > - > - /* In case the handler function re-enabled notifications */ > - virtio_queue_set_notification(vq, 0); > - return progress; > + return virtio_queue_notify_aio_vq(vq); > } > > static void virtio_queue_host_notifier_aio_poll_end(EventNotifier *n) > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index 3448d67d2a..8ee93873a4 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -224,6 +224,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int > version_id); > > void virtio_notify_config(VirtIODevice *vdev); > > +bool virtio_queue_get_notification(VirtQueue *vq); > void virtio_queue_set_notification(VirtQueue *vq, int enable); > > int virtio_queue_ready(VirtQueue *vq); > -- > 2.23.0