[Intel-gfx] [RFC 2/2] drm/i915: Clean-up PPGTT on context destruction

[Daniel Vetter]

On Fri, Feb 13, 2015 at 10:34:51AM +0000, Chris Wilson wrote:
> On Fri, Feb 13, 2015 at 10:55:46AM +0100, Daniel Vetter wrote:
> > On Thu, Feb 12, 2015 at 09:03:06PM +0000, Chris Wilson wrote:
> > > On Thu, Feb 12, 2015 at 08:05:02PM +0000, rafael.barbalho at intel.com wrote:
> > > > From: Rafael Barbalho <rafael.barbalho at intel.com>
> > > > 
> > > > With full PPGTT enabled an object's VMA entry into a PPGTT VM needs to be
> > > > cleaned up so that the PPGTT PDE & PTE allocations can be freed.
> > > > 
> > > > This problem only shows up with full PPGTT because an object's VMA is
> > > > only cleaned-up when the object is destroyed. However, if the object has
> > > > been shared between multiple processes this may not happen, which leads to
> > > > references to the PPGTT still being kept the object was shared.
> > > > 
> > > > Under android the sharing of GEM objects is a fairly common operation, thus
> > > > the clean-up has to be more agressive.
> > > 
> > > Not quite. You need an active refcount as we do not expect close(fd) to
> > > stall. The trick is to "simply" use requests to retire vma (as well as
> > > the object management it does today, though that just becomes a second
> > > layer for GEM API management, everything else goes through vma).
> > 
> > Linking into the ctx unref should give us that for free since requests do
> > hold a reference on the context. So this will only be run when the buffers
> > are idle.
> > 
> > Well except that our unbind code is too dense to do that correctly for
> > shared buffers, so we need to move obj->active to vma->active first.
> 
> We unbind vma, so what do you mean?

The unbind of the vma will block since we track active per-obj instead of
per-vma. Which is kinda not that cool for a kref_put cleanup function.

But yeah the below is what I had in mind too, with the mentioned nuisance
fixed.
-Daniel

> 
> This is how I forsee the code:
> 
> static int context_idr_cleanup(int id, void *p, void *data)
> {
>         struct intel_context *ctx = p;
> 
>         if (ctx->ppgtt && !i915_gem_context_is_default(ctx)) {
>                 struct list_head *list;
>                 struct i915_vma *vma;
> 
>                 /* Decouple the remaining vma to keep the next lookup fast */
>                 list = &ctx->ppgtt->base.vma_list;
>                 while (!list_empty(list)) {
>                         vma = list_first_entry(list, typeof(*vma), vm_link);
>                         list_del_init(&vma->vm_link);
>                         list_del_init(&vma->obj_link);
>                         i915_vma_put(vma);
>                 }
> 
>                 /* Drop active references to this vm upon retire */
>                 ctx->ppgtt->base.closed = true;
> 
>                 /* Drop all inactive references (via vma->vm reference) */
>                 list = &ctx->ppgtt->base.inactive_list;
>                 while (!list_empty(list)) {
>                         struct drm_i915_gem_object *obj;
>                         int ret;
> 
>                         vma = list_first_entry(list, typeof(*vma), mm_list);
>                         obj = vma->obj;
> 
>                         drm_gem_object_reference(&obj->base);
>                         ret = i915_vma_unbind(vma);
>                         drm_gem_object_unreference(&obj->base);
>                         if (WARN_ON(ret))
>                                 break;
>                 }
>         }
> 
>         ctx->file_priv = NULL;
>         i915_gem_context_unreference(ctx);
> 
>         return 0;
> }
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch