Re: [Intel-gfx] [PATCH] drm/i915/display: cleanup intel_bw_state on i915 module removal

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: David Airlie <airlied@linux.ie>,
	dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: cleanup intel_bw_state on i915 module removal
Date: Thu, 12 Dec 2019 20:01:08 +0200	[thread overview]
Message-ID: <20191212180108.GR1208@intel.com> (raw)
In-Reply-To: <20191212174910.GH85422@mdroper-desk1.amr.corp.intel.com>

On Thu, Dec 12, 2019 at 09:49:10AM -0800, Matt Roper wrote:
> On Mon, Dec 09, 2019 at 08:09:02PM +0530, Pankaj Bharadiya wrote:
> > intel_bw_state allocated memory is not getting freed even after
> > module removal.
> > 
> > kmemleak reported backtrace:
> > 
> >     [<0000000079019739>] kmemdup+0x17/0x40
> >     [<00000000d58c1b9d>] intel_bw_duplicate_state+0x1b/0x40 [i915]
> >     [<000000007423ed0c>] drm_atomic_get_private_obj_state+0xca/0x140
> >     [<00000000100e3533>] intel_bw_atomic_check+0x133/0x350 [i915]
> >     [<00000000126d0e0c>] intel_atomic_check+0x1ab7/0x20d0 [i915]
> >     [<00000000d5dfc004>] drm_atomic_check_only+0x563/0x810
> >     [<00000000c9379611>] drm_atomic_commit+0xe/0x50
> >     [<00000000ec82b765>] drm_atomic_helper_disable_all+0x133/0x160
> >     [<000000003c44760c>] drm_atomic_helper_shutdown+0x65/0xc0
> >     [<00000000414e3e5c>] i915_driver_remove+0xcb/0x130 [i915]
> >     [<00000000f8544c2a>] i915_pci_remove+0x19/0x40 [i915]
> >     [<000000002dcbd148>] pci_device_remove+0x36/0xb0
> >     [<000000003c8c6b0a>] device_release_driver_internal+0xe0/0x1c0
> >     [<00000000580e9566>] unbind_store+0xc3/0x120
> >     [<00000000869d0df5>] kernfs_fop_write+0x104/0x190
> >     [<000000004dc1a355>] vfs_write+0xb9/0x1d0
> > 
> > Call the drm_atomic_private_obj_fini(), which inturn calls the
> > intel_bw_destroy_state() to make sure the intel_bw_state memory is
> > freed properly.
> > 
> > Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> 
> It looks like we'd also leak this object if intel_modeset_init() bails
> out early due to failure to init a crtc.  I.e., we call
> drm_mode_config_cleanup() which cleans up any regular state objects that
> may have been initialized by this point, but never destroy the bw_state
> which was allocated earlier in the function.

The question is why isn't the core cleaning those up for us? It already
puts them onto a list so it could easily do it. Looks like komeda has
already hand rolled exactly that.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx