[PATCH 0/4] Fix virtio-gpu deinitialization paths.

[PATCH 0/4] Fix virtio-gpu deinitialization paths.

[Damir Shaikhutdinov]

This series of patches aims to fix some bugs and memory leaks 
in virtio-gpu deinitialization paths.

While denitialization paths is not usually executed in any virtio-gpu
usecase, it is useful for testing during implementation of virtio-gpu
device part.

Damir Shaikhutdinov (4):
  drm/virtio: Fix memory leak during framebuffer destruction.
  drm/virtio: Remove incorrect kfree during connector destruction.
  drm/virtio: Fix virtio gpu fbdev deinitialization.
  drm/virtio: Fix connector leak during virtio-gpu deinitialization.

 drivers/gpu/drm/virtio/virtgpu_display.c | 6 +-----
 drivers/gpu/drm/virtio/virtgpu_fb.c      | 7 +++++--
 drivers/gpu/drm/virtio/virtgpu_kms.c     | 2 ++
 3 files changed, 8 insertions(+), 7 deletions(-)

-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/virtio: Fix memory leak during framebuffer destruction.

[Damir Shaikhutdinov]

In function virtio_gpufb_create, a virtio_gpu_object is allocated for
framebuffer using virtio_gpu_alloc_object.

In virtio_gpu_fbdev_destroy, instead of freeing the object, pointer to
it is set to NULL. This leads to memory leak during framebuffer
destruction, which is reported to kmesg with a message like this:

	Memory manager not clean during takedown.

With DRM_DEBUG_MM enabled, following additional information is printed
about the leak:
	node [00100000 + 000001d5]: inserted at
	save_stack.isra.9+0x67/0xc0
	drm_mm_insert_node_in_range+0x325/0x4f0
	drm_vma_offset_add+0x46/0x60
	ttm_bo_init_reserved+0x2c9/0x400
	ttm_bo_init+0x2a/0x80
	virtio_gpu_object_create+0x139/0x180
	virtio_gpu_alloc_object+0x2f/0x60
	virtio_gpufb_create+0xac/0x2a0

Correctly freeing virtio_gpu_object during framebuffer destruction
fixes the issue.

Signed-off-by: Damir Shaikhutdinov <damir.shaikhutdinov@opensynergy.com>
Signed-off-by: Kiran Pawar <kiran.pawar@opensynergy.com>
---
 drivers/gpu/drm/virtio/virtgpu_fb.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c b/drivers/gpu/drm/virtio/virtgpu_fb.c
index 15d18fd0c64b..10a66a387bfb 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fb.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fb.c
@@ -301,11 +301,14 @@ static int virtio_gpu_fbdev_destroy(struct drm_device *dev,
 
 	drm_fb_helper_unregister_fbi(&vgfbdev->helper);
 
-	if (vgfb->obj)
-		vgfb->obj = NULL;
 	drm_fb_helper_fini(&vgfbdev->helper);
 	drm_framebuffer_cleanup(&vgfb->base);
 
+	if (vgfb->obj) {
+		virtio_gpu_gem_free_object(vgfb->obj);
+		vgfb->obj = NULL;
+	}
+
 	return 0;
 }
 static const struct drm_fb_helper_funcs virtio_gpu_fb_helper_funcs = {
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/virtio: Remove incorrect kfree during connector destruction.

[Damir Shaikhutdinov]

In function virtio_gpu_conn_destroy a pointer to a containing structure
virtio_gpu_output is received using drm_connector_to_virtio_gpu_output
(container_of), and then it is passed to kfree function.

But this pointer points to a member of array (vgdev->outputs + index)
(see vgdev_output_init):

	struct virtio_gpu_output *output = vgdev->outputs + index;
	struct drm_connector *connector = &output->conn;
...
	drm_connector_init(dev, connector, &virtio_gpu_connector_funcs,
			   DRM_MODE_CONNECTOR_VIRTUAL);

Signed-off-by: Damir Shaikhutdinov <damir.shaikhutdinov@opensynergy.com>
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index b6d52055a11f..d211d4e98b46 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -243,12 +243,8 @@ static enum drm_connector_status virtio_gpu_conn_detect(
 
 static void virtio_gpu_conn_destroy(struct drm_connector *connector)
 {
-	struct virtio_gpu_output *virtio_gpu_output =
-		drm_connector_to_virtio_gpu_output(connector);
-
 	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
-	kfree(virtio_gpu_output);
 }
 
 static const struct drm_connector_funcs virtio_gpu_connector_funcs = {
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/virtio: Fix virtio gpu fbdev deinitialization.

[Damir Shaikhutdinov]

Module parameter virtio_gpu_fbdev is used to enable or disable fbdev in
virtio. It is checked during fbdev initialization, but is not checked
during deinitialization.

Moving fbdev destruction to virtgpu_kms.c instead of virtgpu_display.c
places deinitialization to the same file as initialization, and allows
checking for virtio_gpu_fbdev module parameter.

Signed-off-by: Damir Shaikhutdinov <damir.shaikhutdinov@opensynergy.com>
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 1 -
 drivers/gpu/drm/virtio/virtgpu_kms.c     | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index d211d4e98b46..d314e3c672f2 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -377,6 +377,5 @@ int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev)
 
 void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev)
 {
-	virtio_gpu_fbdev_fini(vgdev);
 	drm_mode_config_cleanup(vgdev->ddev);
 }
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 6400506a06b0..b994ecfdd378 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -259,6 +259,8 @@ void virtio_gpu_driver_unload(struct drm_device *dev)
 	flush_work(&vgdev->config_changed_work);
 	vgdev->vdev->config->del_vqs(vgdev->vdev);
 
+	if (virtio_gpu_fbdev)
+		virtio_gpu_fbdev_fini(vgdev);
 	virtio_gpu_modeset_fini(vgdev);
 	virtio_gpu_ttm_fini(vgdev);
 	virtio_gpu_free_vbufs(vgdev);
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/virtio: Fix connector leak during virtio-gpu deinitialization.

[Damir Shaikhutdinov]

Attaching CRTC to a connector increases its reference count, preventing
it from correct deinitialization. Following kernel log is printed when
the leak is found:

	Console: switching to colour VGA+ 80x25
	WARNING: at drivers/gpu/drm/drm_mode_config.c:431
	...
	Call Trace:
	 drm_mode_config_cleanup
	 virtio_gpu_modeset_fini
	 virtio_gpu_driver_unload
	 drm_dev_unregister
	 drm_put_dev
	 virtio_gpu_remove
	 virtio_dev_remove
	 device_release_driver_internal
	 device_release_driver
	 bus_remove_device
	 device_del
	 device_unregister
	 unregister_virtio_device
	...
	[drm:drm_mode_config_cleanup] ERROR connector Virtual-1 leaked!

Calling drm_atomic_helper_shutdown disconnects CRTCs from connectors,
allowing them to be freed during drm_mode_config_cleanup.

Signed-off-by: Damir Shaikhutdinov <damir.shaikhutdinov@opensynergy.com>
Signed-off-by: Kiran Pawar <kiran.pawar@opensynergy.com>
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index d314e3c672f2..088a751a35e9 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -377,5 +377,6 @@ int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev)
 
 void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev)
 {
+	drm_atomic_helper_shutdown(vgdev->ddev);
 	drm_mode_config_cleanup(vgdev->ddev);
 }
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/4] Fix virtio-gpu deinitialization paths.

[Damir Shaikhutdinov]

This series of patches aims to fix some bugs and memory leaks 
in virtio-gpu deinitialization paths.

While denitialization paths is not usually executed in any virtio-gpu
usecase, it is useful for testing during implementation of virtio-gpu
device part.

Damir Shaikhutdinov (4):
  drm/virtio: Fix memory leak during framebuffer destruction.
  drm/virtio: Remove incorrect kfree during connector destruction.
  drm/virtio: Fix virtio gpu fbdev deinitialization.
  drm/virtio: Fix connector leak during virtio-gpu deinitialization.

 drivers/gpu/drm/virtio/virtgpu_display.c | 6 +-----
 drivers/gpu/drm/virtio/virtgpu_fb.c      | 7 +++++--
 drivers/gpu/drm/virtio/virtgpu_kms.c     | 2 ++
 3 files changed, 8 insertions(+), 7 deletions(-)

-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel