Re: [PATCH] drm/i915: Don't cleanup plane state in intel_plane_destroy()

From: Daniel Vetter <daniel@ffwll.ch>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Don't cleanup plane state in intel_plane_destroy()
Date: Mon, 19 Jan 2015 10:05:33 +0100	[thread overview]
Message-ID: <20150119090533.GJ26519@phenom.ffwll.local> (raw)
In-Reply-To: <87d26eu2dp.fsf@intel.com>

On Fri, Jan 16, 2015 at 06:16:50PM +0200, Jani Nikula wrote:
> On Fri, 16 Jan 2015, Matt Roper <matthew.d.roper@intel.com> wrote:
> > When we transitioned to the atomic plane helpers in commit:
> >
> >         commit ea2c67bb4affa84080c616920f3899f123786e56
> >         Author: Matt Roper <matthew.d.roper@intel.com>
> >         Date:   Tue Dec 23 10:41:52 2014 -0800
> >
> >             drm/i915: Move to atomic plane helpers (v9)
> >
> > one of the changes was to call intel_plane_destroy_state() while tearing
> > down a plane to prevent leaks when unloading the driver.  That made
> > sense when the patches were first written, but before they were merged,
> >
> >         commit 3009c0377f25c29852b218a6933a969d02cbdc5d
> >         Author:     Thierry Reding <treding@nvidia.com>
> >         Date:       Tue Nov 25 12:09:49 2014 +0100
> >
> >             drm: Free atomic state during cleanup
> >
> > had already landed, which made this the responsibility of the DRM core.
> > The result was that we were kfree()'ing the state twice, and also
> > possibly double-unref'ing a framebuffer, leading to memory corruption
> > when the driver was unloaded.
> >
> > The fix is to simply not try to cleanup the state in the i915 teardown
> > code now that the core handles this for us.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88433
> > Testcase: igt/drv_module_reload
> > Root-cause-analysis-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> 
> Thanked-by-and-good-weekend-wished-by-and-
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> 
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 91d8ada..cc3b9d8 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11937,7 +11937,6 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)
> >  void intel_plane_destroy(struct drm_plane *plane)
> >  {
> >  	struct intel_plane *intel_plane = to_intel_plane(plane);
> > -	intel_plane_destroy_state(plane, plane->state);
> >  	drm_plane_cleanup(plane);
> >  	kfree(intel_plane);
> >  }
> > -- 
> > 1.8.5.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx