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@xxxxxxxxx> > >> > >>In the following commit: > >> > >> commit e9f24d5fb7cf3628b195b18ff3ac4e37937ceeae > >> Author: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > >> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
