On Tue, Oct 25, 2016 at 3:46 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > drm_property_lookup_blob() returns a reference to the returned blob, and > drm_atomic_replace_property_blob() takes a references to the blob it > stores, so afterwards we are left owning a reference to the new_blob that > we never release, and thus leak memory every time we update a property > such as during drm_atomic_helper_legacy_gamma_set(). > > Based on a patch by Felix Monninger <felix.monninger@xxxxxxxxx> > > Reported-by: Felix Monninger <felix.monninger@xxxxxxxxx> > References: https://bugs.freedesktop.org/show_bug.cgi?id=98420 > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/drm_atomic.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 1b5a32df9a9a..3b35ab793100 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -416,19 +416,24 @@ drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc, > ssize_t expected_size, > bool *replaced) > { > - struct drm_device *dev = crtc->dev; > struct drm_property_blob *new_blob = NULL; > > if (blob_id != 0) { > - new_blob = drm_property_lookup_blob(dev, blob_id); > + new_blob = drm_property_lookup_blob(crtc->dev, blob_id); I think this could be further simplified by making use of drm_property_lookup_blob() returning NULL for blob_id == 0 Then you could do something like: static int drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc, struct drm_property_blob **old_blob, uint64_t blob_id, ssize_t expected_size, bool *replaced) { struct drm_property_blob *blob = NULL; int ret = 0; blob = drm_property_lookup_blob(crtc->dev, blob_id); if (blob && expected_size > 0 && expected_size != blob->length) { ret = -EINVAL; goto out; } } drm_atomic_replace_property_blob(blob, blob, replaced); out: drm_property_unreference_blob(blob); return 0; } > if (new_blob == NULL) > return -EINVAL; > - if (expected_size > 0 && expected_size != new_blob->length) > + > + if (expected_size > 0 && expected_size != new_blob->length) { > + drm_property_unreference_blob(new_blob); > return -EINVAL; > + } > } > > drm_atomic_replace_property_blob(blob, new_blob, replaced); > > + if (new_blob) > + drm_property_unreference_blob(new_blob); > + > return 0; > } > > -- > 2.10.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
