Skip to contentSkip to conversation

vhost: Release memory references on cleanup

PATCH In review Alex Williamson 2 participants 2 messages Sep 7, 2017 – Sep 9, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 vhost: Release memory references on cleanup

vhost registers a MemoryListener where it adds and removes references to MemoryRegions as the MemoryRegionSections pass through. The region_add callback is invoked for each existing section when the MemoryListener is registered, but unregistering the MemoryListener performs no reciprocal region_del callback. It's therefore the owner of the MemoryListener's responsibility to cleanup any persistent changes, such as these memory references, after unregistering.

The consequence of this bug is that if we have both a vhost device and a vfio device, the vhost device will reference any mmap'd MMIO of the vfio device via this MemoryListener. If the vhost device is then removed, those references remain outstanding. If we then attempt to remove the vfio device, it never gets finalized and the only way to release the kernel file descriptors is to terminate the QEMU process.

Fixes dfde4e6e1a86 ("memory: add ref/unref calls")
Cc Michael S. Tsirkin <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc [email protected] # v1.6.0+
---
 hw/virtio/vhost.c |    4 ++++
 1 file changed, 4 insertions(+)
hw/virtio/vhost.c +4 -0
index 6eddb099b02f..b737ca915b06 100644
@@ -1356,6 +1356,10 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
     if (hdev->mem) {
         /* those are only safe after successful init */
         memory_listener_unregister(&hdev->memory_listener);
+        for (i = 0; i < hdev->n_mem_sections; ++i) {
+            MemoryRegionSection *section = &hdev->mem_sections[i];
+            memory_region_unref(section->mr);
+        }
         QLIST_REMOVE(hdev, entry);
     }
     if (hdev->migration_blocker) {
#1 lore raw
Quoted 15 lines · Alex Williamson

vhost registers a MemoryListener where it adds and removes references to MemoryRegions as the MemoryRegionSections pass through. The region_add callback is invoked for each existing section when the MemoryListener is registered, but unregistering the MemoryListener performs no reciprocal region_del callback. It's therefore the owner of the MemoryListener's responsibility to cleanup any persistent changes, such as these memory references, after unregistering.

The consequence of this bug is that if we have both a vhost device and a vfio device, the vhost device will reference any mmap'd MMIO of the vfio device via this MemoryListener. If the vhost device is then removed, those references remain outstanding. If we then attempt to remove the vfio device, it never gets finalized and the only way to release the kernel file descriptors is to terminate the QEMU process.

Quoted 5 lines
Fixes dfde4e6e1a86 ("memory: add ref/unref calls")
Cc Michael S. Tsirkin <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc [email protected] # v1.6.0+

This fix is correct. It's surprising that it was only found now!

Thanks,

Paolo

Quoted 20 lines
---
 hw/virtio/vhost.c |    4 ++++
 1 file changed, 4 insertions(+)
hw/virtio/vhost.c +4 -0
index 6eddb099b02f..b737ca915b06 100644
@@ -1356,6 +1356,10 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
     if (hdev->mem) {
         /* those are only safe after successful init */
         memory_listener_unregister(&hdev->memory_listener);
+        for (i = 0; i < hdev->n_mem_sections; ++i) {
+            MemoryRegionSection *section = &hdev->mem_sections[i];
+            memory_region_unref(section->mr);
+        }
         QLIST_REMOVE(hdev, entry);
     }
     if (hdev->migration_blocker) {