vdpa-net: improve migration downtime through descriptor ASID and persistent IOTLB

[PATCH 18/40] vdpa: unregister listener on last dev cleanup

Posted by Si-Wei Liu 2 years, 2 months ago

So that the free of iova tree struct can be safely deferred to
until the last vq referencing it goes away.

Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 hw/virtio/vhost-vdpa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 4f026db..ea2dfc8 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -815,7 +815,10 @@ static int vhost_vdpa_cleanup(struct vhost_dev *dev)
     }
 
     vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
-    memory_listener_unregister(&v->shared->listener);
+    if (vhost_vdpa_last_dev(dev) && v->shared->listener_registered) {
+        memory_listener_unregister(&v->shared->listener);
+        v->shared->listener_registered = false;
+    }
     vhost_vdpa_svq_cleanup(dev);
 
     dev->opaque = NULL;
-- 
1.8.3.1

Re: [PATCH 18/40] vdpa: unregister listener on last dev cleanup

Posted by Jason Wang 2 years ago

On Fri, Dec 8, 2023 at 2:50 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> So that the free of iova tree struct can be safely deferred to
> until the last vq referencing it goes away.
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  hw/virtio/vhost-vdpa.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 4f026db..ea2dfc8 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -815,7 +815,10 @@ static int vhost_vdpa_cleanup(struct vhost_dev *dev)
>      }
>
>      vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
> -    memory_listener_unregister(&v->shared->listener);
> +    if (vhost_vdpa_last_dev(dev) && v->shared->listener_registered) {
> +        memory_listener_unregister(&v->shared->listener);
> +        v->shared->listener_registered = false;
> +    }

Can we move this to the put() (refcnt decreasing helper) of shared?

Thanks

>      vhost_vdpa_svq_cleanup(dev);
>
>      dev->opaque = NULL;
> --
> 1.8.3.1
>

Re: [PATCH 18/40] vdpa: unregister listener on last dev cleanup

Posted by Eugenio Perez Martin 2 years, 1 month ago

On Thu, Dec 7, 2023 at 7:50 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> So that the free of iova tree struct can be safely deferred to
> until the last vq referencing it goes away.
>

I think this patch message went out of sync too.

> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  hw/virtio/vhost-vdpa.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 4f026db..ea2dfc8 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -815,7 +815,10 @@ static int vhost_vdpa_cleanup(struct vhost_dev *dev)
>      }
>
>      vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
> -    memory_listener_unregister(&v->shared->listener);
> +    if (vhost_vdpa_last_dev(dev) && v->shared->listener_registered) {
> +        memory_listener_unregister(&v->shared->listener);
> +        v->shared->listener_registered = false;
> +    }

I think this version is more correct, but it should not matter as the
device cleanup implies the device will not be used anymore, isn't it?
Or am I missing something?

>      vhost_vdpa_svq_cleanup(dev);
>
>      dev->opaque = NULL;
> --
> 1.8.3.1
>