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

[Gal Hammer]

On Thu, Jan 11, 2018 at 9:52 PM, Michael S. Tsirkin <address@hidden> 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 <address@hidden>
> > Reported-by: Xiaoling Gao <address@hidden>
> > Signed-off-by: Gal Hammer <address@hidden>
>
> 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
>