Skip to content

Commit 12e88c0

Browse files
davidhildenbrandMichael Tokarev
authored andcommitted
vhost: Fix used memslot tracking when destroying a vhost device
When we unplug a vhost device, we end up calling vhost_dev_cleanup() where we do a memory_listener_unregister(). This memory_listener_unregister() call will end up disconnecting the listener from the address space through listener_del_address_space(). In that process, we effectively communicate the removal of all memory regions from that listener, resulting in region_del() + commit() callbacks getting triggered. So in case of vhost, we end up calling vhost_commit() with no remaining memory slots (0). In vhost_commit() we end up overwriting the global variables used_memslots / used_shared_memslots, used for detecting the number of free memslots. With used_memslots / used_shared_memslots set to 0 by vhost_commit() during device removal, we'll later assume that the other vhost devices still have plenty of memslots left when calling vhost_get_free_memslots(). Let's fix it by simply removing the global variables and depending only on the actual per-device count. Easy to reproduce by adding two vhost-user devices to a VM and then hot-unplugging one of them. While at it, detect unexpected underflows in vhost_get_free_memslots() and issue a warning. Reported-by: yuanminghao <[email protected]> Link: https://lore.kernel.org/qemu-devel/[email protected]/ Fixes: 2ce68e4 ("vhost: add vhost_has_free_slot() interface") Cc: Igor Mammedov <[email protected]> Cc: Michael S. Tsirkin <[email protected]> Cc: Stefano Garzarella <[email protected]> Signed-off-by: David Hildenbrand <[email protected]> Message-Id: <[email protected]> Reviewed-by: Igor Mammedov <[email protected]> Reviewed-by: Michael S. Tsirkin <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]> (cherry picked from commit 9f749129e2629b19f424df106c92c5a5647e396c) Signed-off-by: Michael Tokarev <[email protected]>
1 parent 2533500 commit 12e88c0

File tree

1 file changed

+10
-27
lines changed

1 file changed

+10
-27
lines changed

hw/virtio/vhost.c

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,6 @@ static struct vhost_log *vhost_log[VHOST_BACKEND_TYPE_MAX];
4747
static struct vhost_log *vhost_log_shm[VHOST_BACKEND_TYPE_MAX];
4848
static QLIST_HEAD(, vhost_dev) vhost_log_devs[VHOST_BACKEND_TYPE_MAX];
4949

50-
/* Memslots used by backends that support private memslots (without an fd). */
51-
static unsigned int used_memslots;
52-
53-
/* Memslots used by backends that only support shared memslots (with an fd). */
54-
static unsigned int used_shared_memslots;
55-
5650
static QLIST_HEAD(, vhost_dev) vhost_devices =
5751
QLIST_HEAD_INITIALIZER(vhost_devices);
5852

@@ -74,15 +68,15 @@ unsigned int vhost_get_free_memslots(void)
7468

7569
QLIST_FOREACH(hdev, &vhost_devices, entry) {
7670
unsigned int r = hdev->vhost_ops->vhost_backend_memslots_limit(hdev);
77-
unsigned int cur_free;
71+
unsigned int cur_free = r - hdev->mem->nregions;
7872

79-
if (hdev->vhost_ops->vhost_backend_no_private_memslots &&
80-
hdev->vhost_ops->vhost_backend_no_private_memslots(hdev)) {
81-
cur_free = r - used_shared_memslots;
73+
if (unlikely(r < hdev->mem->nregions)) {
74+
warn_report_once("used (%u) vhost backend memory slots exceed"
75+
" the device limit (%u).", hdev->mem->nregions, r);
76+
free = 0;
8277
} else {
83-
cur_free = r - used_memslots;
78+
free = MIN(free, cur_free);
8479
}
85-
free = MIN(free, cur_free);
8680
}
8781
return free;
8882
}
@@ -666,13 +660,6 @@ static void vhost_commit(MemoryListener *listener)
666660
dev->mem = g_realloc(dev->mem, regions_size);
667661
dev->mem->nregions = dev->n_mem_sections;
668662

669-
if (dev->vhost_ops->vhost_backend_no_private_memslots &&
670-
dev->vhost_ops->vhost_backend_no_private_memslots(dev)) {
671-
used_shared_memslots = dev->mem->nregions;
672-
} else {
673-
used_memslots = dev->mem->nregions;
674-
}
675-
676663
for (i = 0; i < dev->n_mem_sections; i++) {
677664
struct vhost_memory_region *cur_vmr = dev->mem->regions + i;
678665
struct MemoryRegionSection *mrs = dev->mem_sections + i;
@@ -1619,15 +1606,11 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
16191606
QLIST_INSERT_HEAD(&vhost_devices, hdev, entry);
16201607

16211608
/*
1622-
* The listener we registered properly updated the corresponding counter.
1623-
* So we can trust that these values are accurate.
1609+
* The listener we registered properly setup the number of required
1610+
* memslots in vhost_commit().
16241611
*/
1625-
if (hdev->vhost_ops->vhost_backend_no_private_memslots &&
1626-
hdev->vhost_ops->vhost_backend_no_private_memslots(hdev)) {
1627-
used = used_shared_memslots;
1628-
} else {
1629-
used = used_memslots;
1630-
}
1612+
used = hdev->mem->nregions;
1613+
16311614
/*
16321615
* We assume that all reserved memslots actually require a real memslot
16331616
* in our vhost backend. This might not be true, for example, if the

0 commit comments

Comments
 (0)