1. Patchew
  2. QEMU
  3. [PATCH v6 00/10] ASID support in vhost-vdpa net
  4. View patch

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

Eugenio Pérez posted 10 patches 3 years, 3 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
There is a newer version of this series
[PATCH v6 01/10] vdpa: Use v->shadow_vqs_enabled in vhost_vdpa_svqs_start & stop
Posted by Eugenio Pérez 3 years, 3 months ago 
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).

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
Re: [PATCH v6 01/10] vdpa: Use v->shadow_vqs_enabled in vhost_vdpa_svqs_start & stop
Posted by Jason Wang 3 years, 2 months ago 
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?

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
>
Re: [PATCH v6 01/10] vdpa: Use v->shadow_vqs_enabled in vhost_vdpa_svqs_start & stop
Posted by Eugenio Perez Martin 3 years, 2 months ago 
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
> >
>
Re: [PATCH v6 01/10] vdpa: Use v->shadow_vqs_enabled in vhost_vdpa_svqs_start & stop
Posted by Jason Wang 3 years, 2 months ago 
在 2022/11/10 20:54, Eugenio Perez Martin 写道:
> 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].


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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.