[Intel-gfx] [PATCH] drm/i915: Ensure associated VMAs are inactive when contexts are destroyed

[Chris Wilson]

On Mon, Oct 26, 2015 at 12:00:06PM +0000, Tvrtko Ursulin wrote:
> 
> On 26/10/15 11:23, Chris Wilson wrote:
> >On Mon, Oct 26, 2015 at 11:05:03AM +0000, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>
> >>In the following commit:
> >>
> >>     commit e9f24d5fb7cf3628b195b18ff3ac4e37937ceeae
> >>     Author: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>     Date:   Mon Oct 5 13:26:36 2015 +0100
> >>
> >>         drm/i915: Clean up associated VMAs on context destruction
> >>
> >>I added a WARN_ON assertion that VM's active list must be empty
> >>at the time of owning context is getting freed, but that turned
> >>out to be a wrong assumption.
> >>
> >>Due ordering of operations in i915_gem_object_retire__read, where
> >>contexts are unreferenced before VMAs are moved to the inactive
> >>list, the described situation can in fact happen.
> >
> >The context is being unreferenced indirectly. Adding a direct reference
> >here is even more bizarre.
> 
> Perhaps is not the prettiest, but it sounds logical to me to ensure
> that order of destruction of involved object hierarchy goes from the
> bottom-up and is not interleaved.
> 
> If you consider the active/inactive list position as part of the
> retire process, doing it at the very place in code, and the very
> object that looked to be destroyed out of sequence, to me sounded
> logical.
> 
> How would you do it, can you think of a better way?

The reference is via the request. We are handling requests, it makes
more sense that you take the reference on the request.

I would just revert the patch, it doesn't fix the problem you tried to
solve and just adds more.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre