From: Tvrtko Ursulin <[email protected]> If a client instantiates a VMA against an imported object (flink) this VMA will not get unbound when the object is closed.
This happens because the exporter holds a reference count on the object and will also keep a reference to the PPGTT VM. In real life this happens with xorg-driver-intel and fbcon takeover. Latter is copied from via the flink name and when Xorg process exists one VMA remains dangling with a now unreachable VM. Signed-off-by: Tvrtko Ursulin <[email protected]> Cc: Chris Wilson <[email protected]> Testcase: igt/gem_ppgtt/flink-vs-ctx-vm-leak --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_gem.c | 63 ++++++++++++++++++++++++++++++++--------- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f9754c3..16a0b34 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1630,6 +1630,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 6a2528c..e82790b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2635,6 +2635,8 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev, size_t size); void i915_init_vm(struct drm_i915_private *dev_priv, struct i915_address_space *vm); +void i915_gem_close_object(struct drm_gem_object *gem_obj, + 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 f7b8766..a720154 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4523,6 +4523,55 @@ static bool discard_backing_storage(struct drm_i915_gem_object *obj) return atomic_long_read(&obj->base.filp->f_count) == 1; } +static void i915_gem_unbind_vma(struct drm_i915_private *dev_priv, + struct i915_vma *vma) +{ + if (WARN_ON(i915_vma_unbind(vma) == -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; + } +} + +void i915_gem_close_object(struct drm_gem_object *gem_obj, + struct drm_file *file) +{ + struct drm_i915_gem_object *obj = to_intel_bo(gem_obj); + struct drm_device *dev = obj->base.dev; + struct drm_i915_file_private *file_priv = file->driver_priv; + struct i915_vma *vma, *next; + struct i915_hw_ppgtt *ppgtt; + + mutex_lock(&dev->struct_mutex); + + /* + * Release all VMAs associated with this client's PPGTT. + * + * This is to avoid potentially unreachable VMAs since contexts can have + * shorter lifetime than objects. Meaning if a client has a reference to + * an object (flink) and an instantiated VMA, when it exists neither VMA + * will be unbound (since object free won't run), nor the PPGTT VM + * freed (since VMA holds a reference to it). + */ + list_for_each_entry_safe(vma, next, &obj->vma_list, vma_link) { + if (i915_is_ggtt(vma->vm)) + continue; + + ppgtt = (struct i915_hw_ppgtt *)vma->vm; + if (ppgtt->file_priv != file_priv) + continue; + + i915_gem_unbind_vma(dev->dev_private, vma); + } + + mutex_unlock(&dev->struct_mutex); +} + void i915_gem_free_object(struct drm_gem_object *gem_obj) { struct drm_i915_gem_object *obj = to_intel_bo(gem_obj); @@ -4535,20 +4584,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, vma_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_gem_unbind_vma(dev_priv, vma); } /* Stolen objects don't hold a ref, but do hold pin count. Fix that up -- 2.3.5 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
