Re: [PATCH 6/7] vhost-user-gpu: fix memory leak in 'virgl_resource_attach_backing'

[Li Qiang]

Marc-André Lureau <marcandre.lureau@gmail.com> 于2021年5月5日周三 下午5:08写道：
>
> Hi
>
> On Wed, May 5, 2021 at 12:03 PM P J P <ppandit@redhat.com> wrote:
>>
>> +-- On Tue, 4 May 2021, Li Qiang wrote --+
>> | diff --git a/contrib/vhost-user-gpu/virgl.c 
>> b/contrib/vhost-user-gpu/virgl.c
>> | index c669d73a1d..a16a311d80 100644
>> | --- a/contrib/vhost-user-gpu/virgl.c
>> | +++ b/contrib/vhost-user-gpu/virgl.c
>> | @@ -287,8 +287,11 @@ virgl_resource_attach_backing(VuGpu *g,
>> |          return;
>> |      }
>> |
>> | -    virgl_renderer_resource_attach_iov(att_rb.resource_id,
>> | +    ret = virgl_renderer_resource_attach_iov(att_rb.resource_id,
>> |                                         res_iovs, att_rb.nr_entries);
>> | +    if (ret != 0) {
>> | +        g_free(res_iovs);
>> | +    }
>> |  }
>>
>> * Similar to earlier,
>>   hw/display/virtio-gpu-3d.c:virgl_resource_attach_backing() calls
>>   'virtio_gpu_cleanup_mapping_iov'
>>
>> * should it be same for vhost-user-gpu?
>>
>>
>
> Good question, that's also what you did when you fixed it for virtio-gpu in:
>
> commit 33243031dad02d161225ba99d782616da133f689
> Author: Li Qiang <liq3ea@gmail.com>
> Date:   Thu Dec 29 03:11:26 2016 -0500
>
>     virtio-gpu-3d: fix memory leak in resource attach backing
>

Do you mean this;
-->https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg01156.html

I think there is no need for this.

The virtio_gpu_cleanup_mapping_iov is needed in virtio-gpu is because
it need map guest memory.
But in vhost-user-gpu case, the 'vg_create_mapping_iov' calls
'vu_gpa_to_va' and this function don't need
do map memory.

But for the beauty of symmetry we can abstract a function called
'vg_destroy_mapping_iov' and it just calls g_free(res->iov).
Like the pair 'virtio_gpu_create_mapping_iov' and 'virtio_gpu_cleanup_mapping'.

Thanks,
Li Qiang

>
> Btw, for each patch, it would be nice to have a reference to the original fix 
> in virtio-gpu.
>
> Thanks!
>