Re: [PATCH v3] drm/i915: Clean up associated VMAs on context destruction

[Michel Thierry <michel.thierry@intel.com>]

On 9/22/2015 2:16 PM, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Prevent leaking VMAs and PPGTT VMs when objects are imported
> via flink.
>
> Scenario is that any VMAs created by the importer will be left
> dangling after the importer exits, or destroys the PPGTT context
> with which they are associated.
>
> This is caused by object destruction not running when the
> importer closes the buffer object handle due the reference held
> by the exporter. This also leaks the VM since the VMA has a
> reference on it.
>
> In practice these leaks can be observed by stopping and starting
> the X server on a kernel with fbcon compiled in. Every time
> X server exits another VMA will be leaked against the fbcon's
> frame buffer object.
>
> Also on systems where flink buffer sharing is used extensively,
> like Android, this leak has even more serious consequences.
>
> This version is takes a general approach from the  earlier work
> by Rafael Barabalho (drm/i915: Clean-up PPGTT on context
              ^Barbalho
> destruction) and tries to incorporate the subsequent discussion
> between Chris Wilson and Daniel Vetter.
>
> v2:
>
> Removed immediate cleanup on object retire - it was causing a
> recursive VMA unbind via i915_gem_object_wait_rendering. And
> it is in fact not even needed since by definition context
> cleanup worker runs only after the last context reference has
> been dropped, hence all VMAs against the VM belonging to the
> context are already on the inactive list.
>
> v3:
>
> Previous version could deadlock since VMA unbind waits on any
> rendering on an object to complete. Objects can be busy in a
> different VM which would mean that the cleanup loop would do
> the wait with the struct mutex held.
>
> This is an even simpler approach where we just unbind VMAs
> without waiting since we know all VMAs belonging to this VM
> are idle, and there is nothing in flight, at the point
> context destructor runs.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Testcase: igt/gem_ppgtt.c/flink-and-exit-vma-leak

I guess we'll have to wait a bit more to have a fix for 
flink-and-close-vma-leak subtest... Anyway, this addresses the flink 
leak (also seen in bug 87729 - igt/gem_close_race)

> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rafael Barbalho <rafael.barbalho@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h         |  5 +++++
>   drivers/gpu/drm/i915/i915_gem.c         | 20 ++++++++++++++++----
>   drivers/gpu/drm/i915/i915_gem_context.c | 24 ++++++++++++++++++++++++
>   3 files changed, 45 insertions(+), 4 deletions(-)
>
> @@ -3274,6 +3276,16 @@ int i915_vma_unbind(struct i915_vma *vma)
>          return 0;
>   }
>
> +int i915_vma_unbind(struct i915_vma *vma)
> +{
> +       return __i915_vma_unbind(vma, true);
> +}
> +
> +int _i915_vma_unbind_no_wait(struct i915_vma *vma)
        ^ it needs one more underscore (__i915_vma_unbind_no_wait).
> +{
> +       return __i915_vma_unbind(vma, false);
> +}
> +

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

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