In order to prevent a leak of the vma on shared objects, we need to
hook into the object_close callback to destroy the vma on the object for
this file. However, if we destroyed that vma immediately we may cause
unexpected application stalls as we try to unbind a busy vma - hence we
defer the unbind to when we retire the vma.

Testcase: igt/gem_ppggtt/flink-and-close-vma-leak
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.c     |  1 +
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_gem.c     | 41 ++++++++++++++++++++++---------------
 drivers/gpu/drm/i915/i915_gem_gtt.c |  2 ++
 drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
 5 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e7eef5fd6918..70979339d58a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1656,6 +1656,7 @@ static struct drm_driver driver = {
        .debugfs_init = i915_debugfs_init,
        .debugfs_cleanup = i915_debugfs_cleanup,
 #endif
+       .gem_close_object = i915_gem_close_object,
        .gem_free_object = i915_gem_free_object,
        .gem_vm_ops = &i915_gem_vm_ops,
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eb775eb1c693..696469a06715 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2686,6 +2686,7 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct 
drm_device *dev,
                                                  size_t size);
 struct drm_i915_gem_object *i915_gem_object_create_from_data(
                struct drm_device *dev, const void *data, size_t size);
+void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file);
 void i915_gem_free_object(struct drm_gem_object *obj);
 void i915_gem_vma_destroy(struct i915_vma *vma);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1d21c5b79215..7c13c27a6470 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2367,6 +2367,30 @@ i915_gem_object_flush_active(struct drm_i915_gem_object 
*obj)
        return 0;
 }
 
+static void i915_vma_close(struct i915_vma *vma)
+{
+       RQ_BUG_ON(vma->closed);
+       vma->closed = true;
+
+       list_del_init(&vma->obj_link);
+       if (!vma->active)
+               WARN_ON(i915_vma_unbind(vma));
+}
+
+void i915_gem_close_object(struct drm_gem_object *gem,
+                          struct drm_file *file)
+{
+       struct drm_i915_gem_object *obj = to_intel_bo(gem);
+       struct drm_i915_file_private *fpriv = file->driver_priv;
+       struct i915_vma *vma, *vn;
+
+       mutex_lock(&obj->base.dev->struct_mutex);
+       list_for_each_entry_safe(vma, vn, &obj->vma_list, obj_link)
+               if (vma->vm->file == fpriv)
+                       i915_vma_close(vma);
+       mutex_unlock(&obj->base.dev->struct_mutex);
+}
+
 /**
  * i915_gem_wait_ioctl - implements DRM_IOCTL_I915_GEM_WAIT
  * @DRM_IOCTL_ARGS: standard ioctl arguments
@@ -2577,9 +2601,6 @@ static int __i915_vma_unbind(struct i915_vma *vma, bool 
wait)
                RQ_BUG_ON(vma->active);
        }
 
-       if (list_empty(&vma->obj_link))
-               return 0;
-
        if (!drm_mm_node_allocated(&vma->node)) {
                i915_gem_vma_destroy(vma);
                return 0;
@@ -3792,20 +3813,8 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
        trace_i915_gem_object_destroy(obj);
 
        list_for_each_entry_safe(vma, next, &obj->vma_list, obj_link) {
-               int ret;
-
                vma->pin_count = 0;
-               ret = i915_vma_unbind(vma);
-               if (WARN_ON(ret == -ERESTARTSYS)) {
-                       bool was_interruptible;
-
-                       was_interruptible = dev_priv->mm.interruptible;
-                       dev_priv->mm.interruptible = false;
-
-                       WARN_ON(i915_vma_unbind(vma));
-
-                       dev_priv->mm.interruptible = was_interruptible;
-               }
+               i915_vma_close(vma);
        }
 
        /* Stolen objects don't hold a ref, but do hold pin count. Fix that up
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5505603f52af..9f594c33bd0a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3240,6 +3240,8 @@ i915_vma_retire(struct drm_i915_gem_request_active 
*active,
                return;
 
        list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
+       if (unlikely(vma->closed))
+               WARN_ON(i915_vma_unbind(vma));
 }
 
 static struct i915_vma *
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index c2f2c62ac88d..be7e8526b219 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -190,6 +190,7 @@ struct i915_vma {
        unsigned int bound : 4;
        unsigned int active : I915_NUM_RINGS;
        bool is_ggtt : 1;
+       bool closed : 1;
 
        /**
         * Support different GGTT views into the same object.
-- 
2.6.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to