On 6/12/22 09:18, Longpeng(Mike) via wrote:
From: Longpeng <longpeng2@huawei.com>
This allows the vhost device to batch the setup of all its host
notifiers.
This significantly reduces the device starting time, e.g. the time
spend
on enabling notifiers reduce from 376ms to 9.1ms for a VM with 64 vCPUs
and 3 vhost-vDPA generic devices[1] (64vq per device)
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg921541.html
Signed-off-by: Longpeng <longpeng2@huawei.com>
---
hw/virtio/vhost.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 7fb008bc9e..16f8391d86 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1507,7 +1507,7 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
int vhost_dev_enable_notifiers(struct vhost_dev *hdev,
VirtIODevice *vdev)
{
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
- int i, r, e;
+ int i, n, r, e;
/* We will pass the notifiers to the kernel, make sure that QEMU
* doesn't interfere.
@@ -1518,6 +1518,12 @@ int vhost_dev_enable_notifiers(struct
vhost_dev *hdev, VirtIODevice *vdev)
goto fail;
}
+ /*
+ * Batch all the host notifiers in a single transaction to avoid
+ * quadratic time complexity in address_space_update_ioeventfds().
+ */
+ memory_region_transaction_begin();
+
for (i = 0; i < hdev->nvqs; ++i) {
r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus),
hdev->vq_index + i,
true);
@@ -1527,8 +1533,12 @@ int vhost_dev_enable_notifiers(struct
vhost_dev *hdev, VirtIODevice *vdev)
}
}
+ memory_region_transaction_commit();
+
return 0;
fail_vq:
+ /* save i for a second iteration after transaction is
committed. */
+ n = i;
while (--i >= 0) {
e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus),
hdev->vq_index + i,
false);
@@ -1536,8 +1546,18 @@ fail_vq:
error_report("vhost VQ %d notifier cleanup error: %d",
i, -r);
}
assert (e >= 0);
- virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus),
hdev->vq_index + i);
}
+
+ /*
+ * The transaction expects the ioeventfds to be open when it
+ * commits. Do it now, before the cleanup loop.
+ */
+ memory_region_transaction_commit();
+
+ while (--n >= 0) {
+ virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus),
hdev->vq_index + n);
+ }
+
virtio_device_release_ioeventfd(vdev);
fail:
return r;
Similarly to patch #2, removing both goto statement in this function
(as a preliminary patch) will 1/ simplify the code 2/ simplify
reviewing your changes, resulting in something like:
int vhost_dev_enable_notifiers(struct vhost_dev *hdev,
VirtIODevice *vdev)
{
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
int i, r, e;
/* We will pass the notifiers to the kernel, make sure that QEMU
* doesn't interfere.
*/
r = virtio_device_grab_ioeventfd(vdev);
if (r < 0) {
error_report("binding does not support host notifiers");
return r;
}
memory_region_transaction_begin();
for (i = 0; i < hdev->nvqs; ++i) {
r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus),
hdev->vq_index + i,
true);
if (r < 0) {
error_report("vhost VQ %d notifier binding failed: %d",
i, -r);
while (--i >= 0) {
e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus),
hdev->vq_index + i,
false);
if (e < 0) {
error_report(
"vhost VQ %d notifier cleanup error:
%d",
i, -r);
}
assert (e >= 0);
virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus),
hdev->vq_index + i);
}
virtio_device_release_ioeventfd(vdev);
break;
}
}
memory_region_transaction_commit();
return r;
}
What do you think?