On Fri, Nov 18, 2022 at 10:49 PM Longpeng(Mike) <longpe...@huawei.com> wrote: > > From: Longpeng <longpe...@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 vhost-vDPA > generic device [1] start time reduce from 376ms to 9.1ms for a VM with > 64 vCPUs and 3 vDPA device(64vq per device).
Great, I think we need to do this for host_notifiers_mr as well. This helps for the case when the notification area could be mapped directly to guests. > > [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg921541.html > > Signed-off-by: Longpeng <longpe...@huawei.com> > --- > hw/virtio/vhost.c | 39 ++++++++++++++++++++++++++++++++++++++- > 1 file changed, 38 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index d1c4c20b8c..bf82d9b176 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1507,6 +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 vq_init_count = 0; > int i, r, e; > > /* We will pass the notifiers to the kernel, make sure that QEMU > @@ -1518,6 +1519,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); > @@ -1525,19 +1532,33 @@ int vhost_dev_enable_notifiers(struct vhost_dev > *hdev, VirtIODevice *vdev) > error_report("vhost VQ %d notifier binding failed: %d", i, -r); > goto fail_vq; > } > + > + vq_init_count++; > } Nit, the name needs some tweak, it's actually the number of the host notifiers that is initialized. And we can count it out of the loop. > > + memory_region_transaction_commit(); > + > return 0; > fail_vq: > - while (--i >= 0) { > + for (i = 0; i < vq_init_count; i++) { It looks to me there's no need for this change. Others look good. Thanks > 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); > + } > + > + /* > + * The transaction expects the ioeventfds to be open when it > + * commits. Do it now, before the cleanup loop. > + */ > + memory_region_transaction_commit(); > + > + for (i = 0; i < vq_init_count; i++) { > virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + > i); > } > + > virtio_device_release_ioeventfd(vdev); > fail: > return r; > @@ -1553,6 +1574,12 @@ void vhost_dev_disable_notifiers(struct vhost_dev > *hdev, VirtIODevice *vdev) > BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); > int i, r; > > + /* > + * 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, > false); > @@ -1560,8 +1587,18 @@ void vhost_dev_disable_notifiers(struct vhost_dev > *hdev, VirtIODevice *vdev) > error_report("vhost VQ %d notifier cleanup failed: %d", i, -r); > } > assert (r >= 0); > + } > + > + /* > + * The transaction expects the ioeventfds to be open when it > + * commits. Do it now, before the cleanup loop. > + */ > + memory_region_transaction_commit(); > + > + for (i = 0; i < hdev->nvqs; ++i) { > virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + > i); > } > + > virtio_device_release_ioeventfd(vdev); > } > > -- > 2.23.0 >
