drm/vmwgfx: Fix dumb buffer leak

Dumb buffers were not being freed because the GEM reference that was
acquired in gb_surface_define was not dropped like it is in the 2D case.
Dropping this ref uncovered a few additional issues with freeing the
resources associated with dirty tracking in vmw_bo_free/release.

Additionally the TTM object associated with the surface were also leaking
which meant that when the ttm_object_file was closed at process exit the
destructor unreferenced an already destroyed surface.

The solution is to remove the destructor from the vmw_user_surface
associated with the dumb_buffer and immediately unreferencing the TTM
object which his removes it from the ttm_object_file.

This also allows the early return in vmw_user_surface_base_release for the
dumb buffer case to be removed as it should no longer occur.

The chain of references now has the GEM handle(s) owning the dumb buffer.
The GEM handles have a singular GEM reference to the vmw_bo which is
dropped when all handles are closed. When the GEM reference count hits
zero the vmw_bo is freed which then unreferences the surface via
vmw_resource_release in vmw_bo_release.

Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
Reviewed-by: Zack Rusin <zack.rusin@broadcom.com>
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250123204424.836896-1-ian.forbes@broadcom.com

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 8832e4de86f1c12573f613e141f3e10fb199a2a2..6d48aacf6d012d5ab45bd338dee4c20488e70026 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -51,11 +51,13 @@ static void vmw_bo_release(struct vmw_bo *vbo)
                        mutex_lock(&res->dev_priv->cmdbuf_mutex);
                        (void)vmw_resource_reserve(res, false, true);
                        vmw_resource_mob_detach(res);
+                       if (res->dirty)
+                               res->func->dirty_free(res);
                        if (res->coherent)
                                vmw_bo_dirty_release(res->guest_memory_bo);
                        res->guest_memory_bo = NULL;
                        res->guest_memory_offset = 0;
-                       vmw_resource_unreserve(res, false, false, false, NULL,
+                       vmw_resource_unreserve(res, true, false, false, NULL,
                                               0);
                        mutex_unlock(&res->dev_priv->cmdbuf_mutex);
                }
@@ -73,9 +75,9 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 {
        struct vmw_bo *vbo = to_vmw_bo(&bo->base);
 
-       WARN_ON(vbo->dirty);
        WARN_ON(!RB_EMPTY_ROOT(&vbo->res_tree));
        vmw_bo_release(vbo);
+       WARN_ON(vbo->dirty);
        kfree(vbo);
 }
 

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index a73af8a355fbf5086df5e41f330bd106ab8be61c..c4d5fe5f330f98c1d0e19030e3f07c2dfffff373 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -273,7 +273,7 @@ int vmw_user_resource_lookup_handle(struct vmw_private *dev_priv,
                goto out_bad_resource;
 
        res = converter->base_obj_to_res(base);
-       kref_get(&res->kref);
+       vmw_resource_reference(res);
 
        *p_res = res;
        ret = 0;

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 02ab65cc63ecadb6ee95c976af59e2631d89345f..a9c14b8389cb03603b98cb0147818687856b32ad 100644 (file)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -639,7 +639,7 @@ static void vmw_user_surface_free(struct vmw_resource *res)
        struct vmw_user_surface *user_srf =
            container_of(srf, struct vmw_user_surface, srf);
 
-       WARN_ON_ONCE(res->dirty);
+       WARN_ON(res->dirty);
        if (user_srf->master)
                drm_master_put(&user_srf->master);
        kfree(srf->offsets);
@@ -670,8 +670,7 @@ static void vmw_user_surface_base_release(struct ttm_base_object **p_base)
         * Dumb buffers own the resource and they'll unref the
         * resource themselves
         */
-       if (res && res->guest_memory_bo && res->guest_memory_bo->is_dumb)
-               return;
+       WARN_ON(res && res->guest_memory_bo && res->guest_memory_bo->is_dumb);
 
        vmw_resource_unreference(&res);
 }
@@ -2337,12 +2336,19 @@ int vmw_dumb_create(struct drm_file *file_priv,
        vbo = res->guest_memory_bo;
        vbo->is_dumb = true;
        vbo->dumb_surface = vmw_res_to_srf(res);
-
+       drm_gem_object_put(&vbo->tbo.base);
+       /*
+        * Unset the user surface dtor since this in not actually exposed
+        * to userspace. The suface is owned via the dumb_buffer's GEM handle
+        */
+       struct vmw_user_surface *usurf = container_of(vbo->dumb_surface,
+                                               struct vmw_user_surface, srf);
+       usurf->prime.base.refcount_release = NULL;
 err:
        if (res)
                vmw_resource_unreference(&res);
-       if (ret)
-               ttm_ref_object_base_unref(tfile, arg.rep.handle);
+
+       ttm_ref_object_base_unref(tfile, arg.rep.handle);
 
        return ret;
 }