Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
vhost: fix vhost_dev_enable_notifiers() error case
in vhost_dev_enable_notifiers(), if virtio_bus_set_host_notifier(true)
fails, we call vhost_dev_disable_notifiers() that executes
virtio_bus_set_host_notifier(false) on all queues, even on queues that
have failed to be initialized.

This triggers a core dump in memory_region_del_eventfd():

 virtio_bus_set_host_notifier: unable to init event notifier: Too many open files (-24)
 vhost VQ 1 notifier binding failed: 24
 .../softmmu/memory.c:2611: memory_region_del_eventfd: Assertion `i != mr->ioeventfd_nb' failed.

Fix the problem by providing to vhost_dev_disable_notifiers() the
number of queues to disable.

Fixes: 8771589 ("vhost: simplify vhost_dev_enable_notifiers")
Cc: longpeng2@huawei.com
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20230602162735.3670785-1-lvivier@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
  • Loading branch information
vivier authored and mstsirkin committed Jun 26, 2023
1 parent 4b4a137 commit 92099aa
Showing 1 changed file with 36 additions and 29 deletions.
65 changes: 36 additions & 29 deletions hw/virtio/vhost.c
Expand Up @@ -1530,6 +1530,40 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
memset(hdev, 0, sizeof(struct vhost_dev));
}

static void vhost_dev_disable_notifiers_nvqs(struct vhost_dev *hdev,
VirtIODevice *vdev,
unsigned int nvqs)
{
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 < nvqs; ++i) {
r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
false);
if (r < 0) {
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 < nvqs; ++i) {
virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i);
}
virtio_device_release_ioeventfd(vdev);
}

/* Stop processing guest IO notifications in qemu.
* Start processing them in vhost in kernel.
*/
Expand Down Expand Up @@ -1559,7 +1593,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
if (r < 0) {
error_report("vhost VQ %d notifier binding failed: %d", i, -r);
memory_region_transaction_commit();
vhost_dev_disable_notifiers(hdev, vdev);
vhost_dev_disable_notifiers_nvqs(hdev, vdev, i);
return r;
}
}
Expand All @@ -1576,34 +1610,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
*/
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);
if (r < 0) {
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);
vhost_dev_disable_notifiers_nvqs(hdev, vdev, hdev->nvqs);
}

/* Test and clear event pending status.
Expand Down

0 comments on commit 92099aa

Please sign in to comment.