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

[Jason Wang <jasowang@redhat.com>]

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