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

[PATCH 14/40] vdpa: convert iova_tree to ref count based

Posted by Si-Wei Liu 11 months, 3 weeks ago

So that it can be freed from vhost_vdpa_cleanup on
the last deref. The next few patches will try to
make iova tree life cycle not depend on memory
listener, and there's possiblity to keep iova tree
around when memory mapping is not changed across
device reset.

Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
 net/vhost-vdpa.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index a126e5c..7b8f047 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -238,6 +238,8 @@ static void vhost_vdpa_cleanup(NetClientState *nc)
     }
     if (--s->vhost_vdpa.shared->refcnt == 0) {
         qemu_close(s->vhost_vdpa.shared->device_fd);
+        g_clear_pointer(&s->vhost_vdpa.shared->iova_tree,
+                        vhost_iova_tree_delete);
         g_free(s->vhost_vdpa.shared);
     }
     s->vhost_vdpa.shared = NULL;
@@ -461,19 +463,12 @@ static int vhost_vdpa_net_data_load(NetClientState *nc)
 static void vhost_vdpa_net_client_stop(NetClientState *nc)
 {
     VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
-    struct vhost_dev *dev;
 
     assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
 
     if (s->vhost_vdpa.index == 0) {
         migration_remove_notifier(&s->migration_state);
     }
-
-    dev = s->vhost_vdpa.dev;
-    if (dev->vq_index + dev->nvqs == dev->vq_index_end) {
-        g_clear_pointer(&s->vhost_vdpa.shared->iova_tree,
-                        vhost_iova_tree_delete);
-    }
 }
 
 static int vhost_vdpa_net_load_setup(NetClientState *nc, NICState *nic)
-- 
1.8.3.1

Re: [PATCH 14/40] vdpa: convert iova_tree to ref count based

Posted by Jason Wang 10 months, 2 weeks ago

On Fri, Dec 8, 2023 at 2:50 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> So that it can be freed from vhost_vdpa_cleanup on
> the last deref. The next few patches will try to
> make iova tree life cycle not depend on memory
> listener, and there's possiblity to keep iova tree
> around when memory mapping is not changed across
> device reset.
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  net/vhost-vdpa.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index a126e5c..7b8f047 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -238,6 +238,8 @@ static void vhost_vdpa_cleanup(NetClientState *nc)
>      }
>      if (--s->vhost_vdpa.shared->refcnt == 0) {
>          qemu_close(s->vhost_vdpa.shared->device_fd);
> +        g_clear_pointer(&s->vhost_vdpa.shared->iova_tree,
> +                        vhost_iova_tree_delete);

Could be part of the put() as well.

Thanks

>          g_free(s->vhost_vdpa.shared);
>      }
>      s->vhost_vdpa.shared = NULL;
> @@ -461,19 +463,12 @@ static int vhost_vdpa_net_data_load(NetClientState *nc)
>  static void vhost_vdpa_net_client_stop(NetClientState *nc)
>  {
>      VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> -    struct vhost_dev *dev;
>
>      assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
>
>      if (s->vhost_vdpa.index == 0) {
>          migration_remove_notifier(&s->migration_state);
>      }
> -
> -    dev = s->vhost_vdpa.dev;
> -    if (dev->vq_index + dev->nvqs == dev->vq_index_end) {
> -        g_clear_pointer(&s->vhost_vdpa.shared->iova_tree,
> -                        vhost_iova_tree_delete);
> -    }
>  }
>
>  static int vhost_vdpa_net_load_setup(NetClientState *nc, NICState *nic)
> --
> 1.8.3.1
>

Re: [PATCH 14/40] vdpa: convert iova_tree to ref count based

Posted by Eugenio Perez Martin 11 months, 3 weeks ago

On Thu, Dec 7, 2023 at 7:50 PM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
> So that it can be freed from vhost_vdpa_cleanup on
> the last deref. The next few patches will try to
> make iova tree life cycle not depend on memory
> listener, and there's possiblity to keep iova tree
> around when memory mapping is not changed across
> device reset.
>

Title and commit description does not match with the patch, I guess it
is because the reference count was at iova_tree some time in the past
but you decided to move to VhostVDPAShared.

But this code should be merged with previous patches, because we have
an asymmetry here and some bug will arise if the guest reset the
device: allocating at device start, but freeing at cleanup instead of
stop.

> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
>  net/vhost-vdpa.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index a126e5c..7b8f047 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -238,6 +238,8 @@ static void vhost_vdpa_cleanup(NetClientState *nc)
>      }
>      if (--s->vhost_vdpa.shared->refcnt == 0) {
>          qemu_close(s->vhost_vdpa.shared->device_fd);
> +        g_clear_pointer(&s->vhost_vdpa.shared->iova_tree,
> +                        vhost_iova_tree_delete);
>          g_free(s->vhost_vdpa.shared);
>      }
>      s->vhost_vdpa.shared = NULL;
> @@ -461,19 +463,12 @@ static int vhost_vdpa_net_data_load(NetClientState *nc)
>  static void vhost_vdpa_net_client_stop(NetClientState *nc)
>  {
>      VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> -    struct vhost_dev *dev;
>
>      assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
>
>      if (s->vhost_vdpa.index == 0) {
>          migration_remove_notifier(&s->migration_state);
>      }
> -
> -    dev = s->vhost_vdpa.dev;
> -    if (dev->vq_index + dev->nvqs == dev->vq_index_end) {
> -        g_clear_pointer(&s->vhost_vdpa.shared->iova_tree,
> -                        vhost_iova_tree_delete);
> -    }
>  }
>
>  static int vhost_vdpa_net_load_setup(NetClientState *nc, NICState *nic)
> --
> 1.8.3.1
>