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

Chris Wilson
2015-11-24T12:50:48Z

On Tue, Nov 24, 2015 at 11:57:22AM +0100, Daniel Vetter wrote:
> 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@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/
>
> 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.

Just revert the incomplete patch, rather than the warning that the
patch is suspect.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre