Re: [PATCH v6 01/10] vdpa: Use v->shadow_vqs_enabled in vhost_vdpa_svqs_start & stop

[Jason Wang]

在 2022/11/10 20:54, Eugenio Perez Martin 写道:

On Thu, Nov 10, 2022 at 6:22 AM Jason Wang <jasow...@redhat.com> wrote:

On Wed, Nov 9, 2022 at 1:08 AM Eugenio Pérez <epere...@redhat.com> wrote:

This function used to trust in v->shadow_vqs != NULL to know if it must
start svq or not.

This is not going to be valid anymore, as qemu is going to allocate svq
unconditionally (but it will only start them conditionally).

It might be a waste of memory if we did this. Any reason for this?

Well, it's modelled after vhost_vdpa notifier member [1].

Right, this could be optimized in the future as well.

But sure we can reduce the memory usage if SVQ is not used. The first
function that needs it is vhost_set_vring_kick. But I think it is not
a good function to place the delayed allocation.

Would it work to move the allocation to vhost_set_features vhost op?
It seems unlikely to me to call callbacks that can affect SVQ earlier
than that one. Or maybe to create a new one and call it the first on
vhost.c:vhost_dev_start?

Rethink about this, so I think we can leave this in the future.

Thanks

Thanks!

[1] The notifier member already allocates VIRTIO_QUEUE_MAX
VhostVDPAHostNotifier for each vhost_vdpa, It is easy to reduce at
least to the number of virtqueues on a vhost_vdpa. Should I send a
patch for this one?

Thanks

Signed-off-by: Eugenio Pérez <epere...@redhat.com>
---
  hw/virtio/vhost-vdpa.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 7468e44b87..7f0ff4df5b 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1029,7 +1029,7 @@ static bool vhost_vdpa_svqs_start(struct vhost_dev *dev)
      Error *err = NULL;
      unsigned i;

-    if (!v->shadow_vqs) {
+    if (!v->shadow_vqs_enabled) {
          return true;
      }

@@ -1082,7 +1082,7 @@ static void vhost_vdpa_svqs_stop(struct vhost_dev *dev)
  {
      struct vhost_vdpa *v = dev->opaque;

-    if (!v->shadow_vqs) {
+    if (!v->shadow_vqs_enabled) {
          return;
      }

--
2.31.1