[Qemu-devel] [PATCH] virtio: improve virtio devices initialization time

[Qemu-devel] [PATCH] virtio: improve virtio devices initialization time

[Gal Hammer]

The loading time of a VM is quite significant when its virtio
devices uses a large amount of virt-queues (e.g. a virtio-serial
device with max_ports=511). Most of the time is spend in the
creation of all the required event notifiers (ioeventfd and memory
regions).

This patch pack all the changes to the memory regions in a
single memory transaction.

Reported-by: Sitong Liu <siliu@redhat.com>
Reported-by: Xiaoling Gao <xiagao@redhat.com>
Signed-off-by: Gal Hammer <ghammer@redhat.com>
---
 hw/virtio/virtio.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ad564b0..8d8d93d 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2574,6 +2574,7 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
     VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
     int n, r, err;
 
+    memory_region_transaction_begin();
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
         VirtQueue *vq = &vdev->vq[n];
         if (!virtio_queue_get_num(vdev, n)) {
@@ -2596,6 +2597,7 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
         }
         event_notifier_set(&vq->host_notifier);
     }
+    memory_region_transaction_commit();
     return 0;
 
 assign_error:
@@ -2609,6 +2611,7 @@ assign_error:
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
     }
+    memory_region_transaction_commit();
     return err;
 }
 
@@ -2625,6 +2628,7 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
     VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
     int n, r;
 
+    memory_region_transaction_begin();
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
         VirtQueue *vq = &vdev->vq[n];
 
@@ -2632,6 +2636,13 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
             continue;
         }
         event_notifier_set_handler(&vq->host_notifier, NULL);
+    }
+    memory_region_transaction_commit();
+
+    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
     }
-- 
2.7.5

Re: [Qemu-devel] [PATCH] virtio: improve virtio devices initialization time

[Michael S. Tsirkin]

On Thu, Jan 11, 2018 at 12:16:56PM +0200, Gal Hammer wrote:
> The loading time of a VM is quite significant when its virtio
> devices uses a large amount of virt-queues (e.g. a virtio-serial
> device with max_ports=511). Most of the time is spend in the
> creation of all the required event notifiers (ioeventfd and memory
> regions).
> 
> This patch pack all the changes to the memory regions in a
> single memory transaction.
> 
> Reported-by: Sitong Liu <siliu@redhat.com>
> Reported-by: Xiaoling Gao <xiagao@redhat.com>
> Signed-off-by: Gal Hammer <ghammer@redhat.com>

Nice patch! Any timing numbers to share before/after?

> ---
>  hw/virtio/virtio.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ad564b0..8d8d93d 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2574,6 +2574,7 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
>      VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
>      int n, r, err;
>  
> +    memory_region_transaction_begin();
>      for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
>          VirtQueue *vq = &vdev->vq[n];
>          if (!virtio_queue_get_num(vdev, n)) {
> @@ -2596,6 +2597,7 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
>          }
>          event_notifier_set(&vq->host_notifier);
>      }
> +    memory_region_transaction_commit();
>      return 0;
>  
>  assign_error:
> @@ -2609,6 +2611,7 @@ assign_error:
>          r = virtio_bus_set_host_notifier(qbus, n, false);
>          assert(r >= 0);
>      }
> +    memory_region_transaction_commit();
>      return err;
>  }
>  
> @@ -2625,6 +2628,7 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
>      VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
>      int n, r;
>  
> +    memory_region_transaction_begin();
>      for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
>          VirtQueue *vq = &vdev->vq[n];
>  
> @@ -2632,6 +2636,13 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
>              continue;
>          }
>          event_notifier_set_handler(&vq->host_notifier, NULL);
> +    }
> +    memory_region_transaction_commit();
> +
> +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
>          r = virtio_bus_set_host_notifier(qbus, n, false);
>          assert(r >= 0);
>      }
> -- 
> 2.7.5

Re: [Qemu-devel] [PATCH] virtio: improve virtio devices initialization time

[Gal Hammer]

On Thu, Jan 11, 2018 at 9:52 PM, Michael S. Tsirkin <mst@redhat.com> wrote:

> On Thu, Jan 11, 2018 at 12:16:56PM +0200, Gal Hammer wrote:
> > The loading time of a VM is quite significant when its virtio
> > devices uses a large amount of virt-queues (e.g. a virtio-serial
> > device with
> ​​
> max_ports=511). Most of the time is spend in the
> > creation of all the required event notifiers (ioeventfd and memory
> > regions).
> >
> > This patch pack all the changes to the memory regions in a
> > single memory transaction.
> >
> > Reported-by: Sitong Liu <siliu@redhat.com>
> > Reported-by: Xiaoling Gao <xiagao@redhat.com>
> > Signed-off-by: Gal Hammer <ghammer@redhat.com>
>
> Nice patch! Any timing numbers to share before/after?
>

​Thanks. Unfortunately ​I made a mistake in the shutdown code (the
transaction is wrapping the code which doesn't modify the memory regions),
so a V2 is on its way.

​As for the timing. Running a VM with 25 virtio-serial devices,​ each one
with ​max_ports=511, results in a boot time of around 30 minutes. With this
patch (and a another patch to kvm) reduce it to approximately 3 minutes.

This was reported and tracked here:
https://bugzilla.redhat.com/show_bug.cgi?id=1528588

    Gal.


>
> > ---
> >  hw/virtio/virtio.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index ad564b0..8d8d93d 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -2574,6 +2574,7 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice
> *vdev)
> >      VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_
> bus(DEVICE(vdev)));
> >      int n, r, err;
> >
> > +    memory_region_transaction_begin();
> >      for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> >          VirtQueue *vq = &vdev->vq[n];
> >          if (!virtio_queue_get_num(vdev, n)) {
> > @@ -2596,6 +2597,7 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice
> *vdev)
> >          }
> >          event_notifier_set(&vq->host_notifier);
> >      }
> > +    memory_region_transaction_commit();
> >      return 0;
> >
> >  assign_error:
> > @@ -2609,6 +2611,7 @@ assign_error:
> >          r = virtio_bus_set_host_notifier(qbus, n, false);
> >          assert(r >= 0);
> >      }
> > +    memory_region_transaction_commit();
> >      return err;
> >  }
> >
> > @@ -2625,6 +2628,7 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice
> *vdev)
> >      VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_
> bus(DEVICE(vdev)));
> >      int n, r;
> >
> > +    memory_region_transaction_begin();
> >      for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> >          VirtQueue *vq = &vdev->vq[n];
> >
> > @@ -2632,6 +2636,13 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice
> *vdev)
> >              continue;
> >          }
> >          event_notifier_set_handler(&vq->host_notifier, NULL);
> > +    }
> > +    memory_region_transaction_commit();
> > +
> > +    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
> > +        if (!virtio_queue_get_num(vdev, n)) {
> > +            continue;
> > +        }
> >          r = virtio_bus_set_host_notifier(qbus, n, false);
> >          assert(r >= 0);
> >      }
> > --
> > 2.7.5
>

Re: [Qemu-devel] [PATCH] virtio: improve virtio devices initialization time

[Michael S. Tsirkin]

On Fri, Jan 12, 2018 at 10:48:21AM +0200, Gal Hammer wrote:
> 
> On Thu, Jan 11, 2018 at 9:52 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
>     On Thu, Jan 11, 2018 at 12:16:56PM +0200, Gal Hammer wrote:
>     > The loading time of a VM is quite significant when its virtio
>     > devices uses a large amount of virt-queues (e.g. a virtio-serial
>     > device with
>     ​​
>     max_ports=511). Most of the time is spend in the
>     > creation of all the required event notifiers (ioeventfd and memory
>     > regions).
>     >
>     > This patch pack all the changes to the memory regions in a
>     > single memory transaction.
>     >
>     > Reported-by: Sitong Liu <siliu@redhat.com>
>     > Reported-by: Xiaoling Gao <xiagao@redhat.com>
>     > Signed-off-by: Gal Hammer <ghammer@redhat.com>
> 
>     Nice patch! Any timing numbers to share before/after?
> 
> 
> Thanks. Unfortunately I made a mistake in the shutdown code (the transaction
> is wrapping the code which doesn't modify the memory regions), so a V2 is on
> its way.
> 
> As for the timing. Running a VM with 25 virtio-serial devices,​ each one with
> max_ports=511, results in a boot time of around 30 minutes. With this patch
> (and a another patch to kvm) reduce it to approximately 3 minutes.

Good to know, sounds like an important fix to have.
Since you plan to do v2 anyway, please add this info
in the commit log.


> This was reported and tracked here: https://bugzilla.redhat.com/show_bug.cgi?id
> =1528588
> 
>     Gal.