[Intel-gfx] [PATCH 53/55] drm/i915: Release vma when the handle is closed

[Chris Wilson]

On Wed, Jul 27, 2016 at 01:00:59PM +0300, Joonas Lahtinen wrote:
> On ma, 2016-07-25 at 18:32 +0100, Chris Wilson wrote:
> >  /**
> >   * i915_gem_wait_ioctl - implements DRM_IOCTL_I915_GEM_WAIT
> >   * @dev: drm device pointer
> > @@ -2810,26 +2823,32 @@ static int __i915_vma_unbind(struct i915_vma *vma, bool wait)
> >  	if (active && wait) {
> >  		int idx;
> >  
> > +		/* When a closed VMA is retired, it is unbound - eek.
> > +		 * In order to prevent it from being recursively closed,
> > +		 * take a pin on the vma so that the second unbind is
> > +		 * aborted.
> > +		 */
> > +		vma->pin_count++;
> 
> Always smells fishy. But an extra variable probably not worthy.
> 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index ef0dc7131808..bfac2448ba04 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -3339,6 +3339,31 @@ i915_vma_retire(struct i915_gem_active *active,
> >  		return;
> >  
> >  	list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
> > +	if (unlikely(vma->closed && !vma->pin_count))
> > +		WARN_ON(i915_vma_unbind(vma));
> 
> So this is just an optimization to get rid of the VMA ASAP if we're
> lucky?

Not quite. Think of this as the active reference, this may be the last
point at which we see the closed vma. If we don't free it now, we leak
the VMA until the object is closed - where if we meet it we BUG.

> > +void i915_vma_close(struct i915_vma *vma)
> > +{
> > +	GEM_BUG_ON(vma->closed);
> > +	vma->closed = true;
> > +
> > +	list_del_init(&vma->obj_link);
> > +	if (!i915_vma_is_active(vma) && !vma->pin_count)
> > +		WARN_ON(__i915_vma_unbind_no_wait(vma));
> 
> Same here, an optimization?

Same as above. This time we know the vma is inactive, so no outstanding
reference.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre