Re: [PATCH v1] drm/i915: Fix a false alert of memory leak when free LRC

Tvrtko Ursulin
2015-11-23T10:35:01Z

On 20/11/15 08:31, Daniel Vetter wrote:
> On Thu, Nov 19, 2015 at 04:10:26PM -0800, yu.dai@intel.com wrote:
>> From: Alex Dai <yu.dai@intel.com>
>>
>> There is a memory leak warning message from i915_gem_context_clean
>> when GuC submission is enabled. The reason is that when LRC is
>> released, its ppgtt could be still referenced. The assumption that
>> all VMAs are unbound during release of LRC is not true.
>>
>> v1: Move the code inside i915_gem_context_clean() to where ppgtt is
>> released because it is not cleaning context anyway but ppgtt.
>>
>> Signed-off-by: Alex Dai <yu.dai@intel.com>
>
> retire__read drops the ctx (and hence ppgtt) reference too early,
> resulting in us hitting the WARNING. See the giant thread with Tvrtko,
> Chris and me:
>
> http://www.spinics.net/lists/intel-gfx/msg78918.html
>
> Would be great if someone could test the diff I posted in there.

It doesn't work - I have posted my IGT snippet which I thought explained it.

Problem req unreference in obj->active case. When it hits that path it
will not move the VMA to inactive and the
intel_execlists_retire_requests will be the last unreference from the
retire worker which will trigger the WARN.

I posted an IGT which hits that ->
http://patchwork.freedesktop.org/patch/65369/

And posted one give up on the active VMA mem leak patch ->
http://patchwork.freedesktop.org/patch/65529/

I have no idea yet of GuC implications, I just spotted this parallel thread.

And Mika has proposed something interesting - that we could just clean
up the active VMA in context cleanup since we know it is a false one.

However, again I don't know how that interacts with the GuC. Surely it
cannot be freeing the context with stuff genuinely still active in the GuC?

Regards,

Tvrtko