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'.
Right. I think I like the suggestion to add a 'virtio_gpu_cleanup_mapping' (with a comment to explain it) to avoid this kind of question again. Feel free to add a patch on top if you want.
thanks
>
> Btw, for each patch, it would be nice to have a reference to the original fix in virtio-gpu.
>
> Thanks!
>