[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] virtio: don't enable notifications during polling
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH] virtio: don't enable notifications during polling |
Date: |
Fri, 13 Dec 2019 11:20:39 +0000 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
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 <address@hidden>
> ---
> 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