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

[Daniel Vetter]

On Mon, Nov 23, 2015 at 10:34:59AM +0000, Tvrtko Ursulin wrote:
> 
> On 20/11/15 08:31, Daniel Vetter wrote:
> >On Thu, Nov 19, 2015 at 04:10:26PM -0800, yu.dai at intel.com wrote:
> >>From: Alex Dai <yu.dai at 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 at 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/

Ok, I get things now. Fundamentally the problem is that we track active
per-obj, but we want it per-vma. In a way this patch just diggs us deeper
into that hole, which doesn't make me all too happy. Oh well.

I'll pull in the warning removal.
-Daniel

> 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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch