| Message ID | 20171207220025.22698-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Chris Wilson (2017-12-07 22:00:25) > When intel_modeset_setup_plane_state() fails drop the local framebuffer > reference before jumping to the error, otherwise we leak the framebuffer. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> I believe, Fixes: edde361711ef ("drm/i915: Use atomic state to obtain load detection crtc, v3.") -Chris
Quoting Patchwork (2017-12-07 22:20:31) > == Series Details == > > Series: drm/i915: Drop fb reference on load_detect_pipe failure path > URL : https://patchwork.freedesktop.org/series/35057/ > State : success > > == Summary == > > Series 35057v1 drm/i915: Drop fb reference on load_detect_pipe failure path > https://patchwork.freedesktop.org/api/1.0/series/35057/revisions/1/mbox/ > > Test debugfs_test: > Subgroup read_all_entries: > dmesg-fail -> DMESG-WARN (fi-elk-e7500) fdo#103989 > Test kms_frontbuffer_tracking: > Subgroup basic: > pass -> FAIL (fi-glk-1) fdo#103167 No impact on the gdg fb leak. Maybe better luck for the others? -Chris
On Thu, Dec 07, 2017 at 10:00:25PM +0000, Chris Wilson wrote: > When intel_modeset_setup_plane_state() fails drop the local framebuffer > reference before jumping to the error, otherwise we leak the framebuffer. Or we could just https://patchwork.freedesktop.org/patch/165486/ This is Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> nonetheless. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/intel_display.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 1f7e312d0d0d..1ba570614dbe 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9940,11 +9940,10 @@ int intel_get_load_detect_pipe(struct drm_connector *connector, > } > > ret = intel_modeset_setup_plane_state(state, crtc, mode, fb, 0, 0); > + drm_framebuffer_put(fb); > if (ret) > goto fail; > > - drm_framebuffer_put(fb); > - > ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode); > if (ret) > goto fail; > -- > 2.15.1
Quoting Ville Syrjälä (2017-12-08 11:14:17) > On Thu, Dec 07, 2017 at 10:00:25PM +0000, Chris Wilson wrote: > > When intel_modeset_setup_plane_state() fails drop the local framebuffer > > reference before jumping to the error, otherwise we leak the framebuffer. > > Or we could just > https://patchwork.freedesktop.org/patch/165486/ Why not both! :) > This is > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > nonetheless. Ta, less controversial patch for more, grand improvement for you :) -Chris
Quoting Chris Wilson (2017-12-08 11:31:01) > Quoting Ville Syrjälä (2017-12-08 11:14:17) > > On Thu, Dec 07, 2017 at 10:00:25PM +0000, Chris Wilson wrote: > > > When intel_modeset_setup_plane_state() fails drop the local framebuffer > > > reference before jumping to the error, otherwise we leak the framebuffer. > > > > Or we could just > > https://patchwork.freedesktop.org/patch/165486/ > > Why not both! :) > > > This is > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > nonetheless. > > Ta, less controversial patch for more, grand improvement for you :) And pushed, thanks for the review. Let's have a gander at the more intelligent load-detect patch. :) -Chris
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1f7e312d0d0d..1ba570614dbe 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9940,11 +9940,10 @@ int intel_get_load_detect_pipe(struct drm_connector *connector, } ret = intel_modeset_setup_plane_state(state, crtc, mode, fb, 0, 0); + drm_framebuffer_put(fb); if (ret) goto fail; - drm_framebuffer_put(fb); - ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode); if (ret) goto fail;
When intel_modeset_setup_plane_state() fails drop the local framebuffer reference before jumping to the error, otherwise we leak the framebuffer. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)