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 <[email protected]> > --- > 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(-)
Post-release ping :)
> 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
>
signature.asc
Description: PGP signature
