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

[Eugenio Perez Martin <eperezma@redhat.com>]

On Thu, Nov 10, 2022 at 6:22 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Wed, Nov 9, 2022 at 1:08 AM Eugenio Pérez <eperezma@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].

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?

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 <eperezma@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
> >
>