virtio_gpu_device's capsets field is allocated with the DRM memory
allocator but never freed. Add the appropriate freeing call inside
virtio_gpu_deinit.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 drivers/gpu/drm/virtio/virtgpu_kms.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 7b3c4d314f8eee692e2842a7056d6dc64936fc2f..a8b751179332b9ec2fbba1392a6ee0e638a5192e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -286,6 +286,10 @@ void virtio_gpu_deinit(struct drm_device *dev)
 	flush_work(&vgdev->cursorq.dequeue_work);
 	flush_work(&vgdev->config_changed_work);
 	virtio_reset_device(vgdev->vdev);
+	spin_lock(&vgdev->display_info_lock);
+	drmm_kfree(dev, vgdev->capsets);
+	vgdev->capsets = NULL;
+	spin_unlock(&vgdev->display_info_lock);
 	virtio_gpu_fence_cleanup(vgdev);
 	virtio_gpu_queue_cleanup(vgdev);
 	vgdev->vdev->config->del_vqs(vgdev->vdev);

-- 
2.47.2

On 5/5/25 11:59, Manos Pitsidianakis wrote:
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 7b3c4d314f8eee692e2842a7056d6dc64936fc2f..a8b751179332b9ec2fbba1392a6ee0e638a5192e 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -286,6 +286,10 @@ void virtio_gpu_deinit(struct drm_device *dev)
>  	flush_work(&vgdev->cursorq.dequeue_work);
>  	flush_work(&vgdev->config_changed_work);
>  	virtio_reset_device(vgdev->vdev);
> +	spin_lock(&vgdev->display_info_lock);
> +	drmm_kfree(dev, vgdev->capsets);
> +	vgdev->capsets = NULL;
> +	spin_unlock(&vgdev->display_info_lock);

Isn't this lock superfluous?

-- 
Best regards,
Dmitry

On 5/5/25 18:58, Dmitry Osipenko wrote:
> On 5/5/25 11:59, Manos Pitsidianakis wrote:
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
>> index 7b3c4d314f8eee692e2842a7056d6dc64936fc2f..a8b751179332b9ec2fbba1392a6ee0e638a5192e 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
>> @@ -286,6 +286,10 @@ void virtio_gpu_deinit(struct drm_device *dev)
>>  	flush_work(&vgdev->cursorq.dequeue_work);
>>  	flush_work(&vgdev->config_changed_work);
>>  	virtio_reset_device(vgdev->vdev);
>> +	spin_lock(&vgdev->display_info_lock);
>> +	drmm_kfree(dev, vgdev->capsets);
>> +	vgdev->capsets = NULL;
>> +	spin_unlock(&vgdev->display_info_lock);
> 
> Isn't this lock superfluous?

Wait a minute, vgdev->capsets is allocated using drmm, hence it's
auto-freed when DRM device is freed. This patch shouldn't be needed.

-- 
Best regards,
Dmitry

On Mon, May 05, 2025 at 07:22:35PM +0300, Dmitry Osipenko wrote:
>On 5/5/25 18:58, Dmitry Osipenko wrote:
>> On 5/5/25 11:59, Manos Pitsidianakis wrote:
>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
>>> index 7b3c4d314f8eee692e2842a7056d6dc64936fc2f..a8b751179332b9ec2fbba1392a6ee0e638a5192e 100644
>>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
>>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
>>> @@ -286,6 +286,10 @@ void virtio_gpu_deinit(struct drm_device *dev)
>>>  	flush_work(&vgdev->cursorq.dequeue_work);
>>>  	flush_work(&vgdev->config_changed_work);
>>>  	virtio_reset_device(vgdev->vdev);
>>> +	spin_lock(&vgdev->display_info_lock);
>>> +	drmm_kfree(dev, vgdev->capsets);
>>> +	vgdev->capsets = NULL;
>>> +	spin_unlock(&vgdev->display_info_lock);
>>
>> Isn't this lock superfluous?
>
>Wait a minute, vgdev->capsets is allocated using drmm, hence it's
>auto-freed when DRM device is freed. This patch shouldn't be needed.

Yep, good point. I mean the patch is not wrong, but I think we can avoid 
it.

Thanks,
Stefano