[Intel-gfx] [PATCH] drm/i915: Don't leak VBT mode data

Daniel Vetter daniel at ffwll.ch
Wed Sep 23 06:15:45 PDT 2015


On Wed, Sep 23, 2015 at 02:02:03PM +0300, Jani Nikula wrote:
> On Wed, 23 Sep 2015, Daniel Vetter <daniel at ffwll.ch> wrote:
> > On Tue, Sep 15, 2015 at 10:50:45AM +0300, Jani Nikula wrote:
> >> On Tue, 15 Sep 2015, Matt Roper <matthew.d.roper at intel.com> wrote:
> >> > We allocate memory for LVDS modes while parsing the VBT at startup, but
> >> > never free this memory when the driver is unloaded, causing a small
> >> > leak.
> >> >
> >> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> >> 
> >> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
> >> 
> >> We could probably use devm_*alloc functions for this kind of stuff more.
> >
> > The problem with devm_* is that the lifetime of the underlying hw
> > device doesn't stricltly match the lifetime of the drm device.
> 
> Is it not enough that the lifetime of the driver is bounded by the
> lifetime of the drm device? What is the problem you're referring to,
> apart from handing the hardware devm allocated memory and unreferencing
> the drm device?

For udl if you unplug the physical device the struct device _will_ go
away.

The drm_device otoh might be referenced by piles of other internal things
(like open file descriptors, shared dma-bufs and what else) and hence very
much must stick around. Of course we /should/ try to not call down into
lower-level hw handling code in that state. Which all very much doesn't
happen (at least in a race-free manner).

So the drm_device is not bounded by the lifetime of the underlying
physical device in full generality. Which is why we can't really do any
devm_* stuff in drm core.

> > Not that anything in drm gets that right (pretty far from it), but
> > that's why I haven't gone ballastic yet with rolling out devm_* all
> > over our init code.
> 
> I think you mean "ballistic", but, in this case, I think your typo
> "ballastic" is actually a more accurate description of the change. :)

Either way I think we should be ok with fully abusing devm_* ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the Intel-gfx mailing list