Re: [PATCH] vhost-vdpa: cleanup memory maps when closing vdpa fds

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2023/2/15 10:00, Jason Wang 写道:
On Tue, Feb 14, 2023 at 2:28 PM Longpeng (Mike, Cloud Infrastructure
Service Product Dept.) <longpeng2@xxxxxxxxxx> wrote:



在 2023/2/14 14:16, Jason Wang 写道:

在 2023/1/31 22:53, Longpeng(Mike) 写道:
From: Longpeng <longpeng2@xxxxxxxxxx>

We must cleanup all memory maps when closing the vdpa fds, otherwise
some critical resources (e.g. memory, iommu map) will leaked if the
userspace exits unexpectedly (e.g. kill -9).


Sounds like a bug of the kernel, should we fix there?


For example, the iommu map is setup when QEMU calls VHOST_IOTLB_UPDATE
ioctl and it'll be freed if QEMU calls VHOST_IOTLB_INVALIDATE ioctl.

So maybe we release these resources in vdpa framework in kernel is a
suitable choice?

I think I need understand what does "resources" mean here:

For iommu mapping, it should be freed by vhost_vdpa_free_domain() in
vhost_vdpa_release()?


Please consider the following lifecycle of the vdpa device:

1. vhost_vdpa_open
    vhost_vdpa_alloc_domain

2. vhost_vdpa_pa_map
    pin_user_pages
    vhost_vdpa_map
      iommu_map

3. kill QEMU

4. vhost_vdpa_release
    vhost_vdpa_free_domain

In this case, we have no opportunity to invoke unpin_user_pages or iommu_unmap to free the memory.

static int vhost_vdpa_release(struct inode *inode, struct file *filep)
{
         struct vhost_vdpa *v = filep->private_data;
         struct vhost_dev *d = &v->vdev;

         mutex_lock(&d->mutex);
         filep->private_data = NULL;
         vhost_vdpa_clean_irq(v);
         vhost_vdpa_reset(v);
         vhost_dev_stop(&v->vdev);
         vhost_vdpa_free_domain(v);
         vhost_vdpa_config_put(v);
         vhost_vdpa_cleanup(v);
         mutex_unlock(&d->mutex);

         atomic_dec(&v->opened);
         complete(&v->completion);

         return 0;
}


By the way, Jason, can you reproduce the problem in your machine?


Haven't got time in doing this but it should be the responsibility of
the author to validate this anyhow.

Thanks

Thanks



Signed-off-by: Longpeng <longpeng2@xxxxxxxxxx>
---
   drivers/vhost/vdpa.c | 13 +++++++++++++
   1 file changed, 13 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index a527eeeac637..37477cffa5aa 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -823,6 +823,18 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
           vhost_vdpa_remove_as(v, asid);
   }
+static void vhost_vdpa_clean_map(struct vhost_vdpa *v)
+{
+    struct vhost_vdpa_as *as;
+    u32 asid;
+
+    for (asid = 0; asid < v->vdpa->nas; asid++) {
+        as = asid_to_as(v, asid);
+        if (as)
+            vhost_vdpa_unmap(v, &as->iotlb, 0ULL, 0ULL - 1);
+    }
+}
+
   static int vhost_vdpa_va_map(struct vhost_vdpa *v,
                    struct vhost_iotlb *iotlb,
                    u64 iova, u64 size, u64 uaddr, u32 perm)
@@ -1247,6 +1259,7 @@ static int vhost_vdpa_release(struct inode
*inode, struct file *filep)
       vhost_vdpa_clean_irq(v);
       vhost_vdpa_reset(v);
       vhost_dev_stop(&v->vdev);
+    vhost_vdpa_clean_map(v);
       vhost_vdpa_free_domain(v);
       vhost_vdpa_config_put(v);
       vhost_vdpa_cleanup(v);

.


.



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux