hw/virtio/virtio.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
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
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
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 >
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.
© 2016 - 2024 Red Hat, Inc.