Re: [PATCH] drm/i915: Remove incorrect warning in context cleanup

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>,
	Intel-gfx@lists.freedesktop.org,
	Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Michel Thierry <michel.thierry@intel.com>
Subject: Re: [PATCH] drm/i915: Remove incorrect warning in context cleanup
Date: Tue, 24 Nov 2015 13:17:57 +0000	[thread overview]
Message-ID: <56546385.3000700@linux.intel.com> (raw)
In-Reply-To: <20151124125357.GL22980@nuc-i3427.alporthouse.com>

On 24/11/15 12:53, Chris Wilson wrote:
> On Tue, Nov 24, 2015 at 11:58:22AM +0100, Daniel Vetter wrote:
>> On Fri, Nov 20, 2015 at 01:23:36PM +0000, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> Commit e9f24d5fb7cf3628b195b18ff3ac4e37937ceeae
>>> Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Date:   Mon Oct 5 13:26:36 2015 +0100
>>>
>>>      drm/i915: Clean up associated VMAs on context destruction
>>>
>>> Added a warning based on an incorrect assumption that all VMAs
>>> in a VM will be on the inactive list at the point last reference
>>> to a context and VM is dropped.
>>>
>>> This is not true because i915_gem_object_retire__read will not
>>> put VMA on the inactive list until all activities on the object
>>> in question (in all VMs) have been retired.
>>>
>>> As a consequence, whether or not a context/VM will be destroyed
>>> with its VMAs still on the active list, can depend on completely
>>> unrelated activities using the same object from a different
>>> context or engine.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92638
>>> Testcase: igt/gem_request_retire/retire-vma-not-inactive
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Michel Thierry <michel.thierry@intel.com>
>>
>> Queued for -next, thanks for the patch.
>
> The WARN_ON is accurate though. The original patch fails to fix even the
> limited aspect of the bug it claimed to.

That is not true. It only makes it a bit more limited, and not by its 
fault even. Even with that it makes things a bit better, not worse.

And does not impede your VMA rewrite at all. For which I did offer help 
to review as you send out in manageable chunks.

If it is not realistically possible to split it out and do in 
increments, then it would be more constructive to discuss how to do it 
than to keep it in limbo for 15 months, as you say, and use it as a 
reason to shoot down everything else.

Regards,

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