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