A refcounting leak was found of the original frame buffer attached to the CRTC when using the page_flip ioctl, resulting in the frame buffer never being freed.
This was not obvious initially, as if the page flip subsequently re-attaches the original frame buffer, the refcounts will be balanced. However, if the original frame buffer is freed, then it will be leaked.
Fix this by ensuring that we take a reference on the incoming fb, but rely on the queued work to drop that ref count.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 1f0875c26dc5..ac2d73f4af45 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -945,18 +945,15 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, armada_reg_queue_end(work->regs, i);
/* - * Hold the old framebuffer for the work - DRM appears to drop our - * reference to the old framebuffer in drm_mode_page_flip_ioctl(). + * Ensure that we hold a reference on the new framebuffer. + * This has to match the behaviour in mode_set. */ - drm_framebuffer_reference(work->old_fb); + drm_framebuffer_reference(fb);
ret = armada_drm_crtc_queue_frame_work(dcrtc, work); if (ret) { - /* - * Undo our reference above; DRM does not drop the reference - * to this object on error, so that's okay. - */ - drm_framebuffer_unreference(work->old_fb); + /* Undo our reference above */ + drm_framebuffer_unreference(fb); kfree(work); return ret; }
On Sun, Oct 12, 2014 at 12:01:18AM +0100, Russell King wrote:
A refcounting leak was found of the original frame buffer attached to the CRTC when using the page_flip ioctl, resulting in the frame buffer never being freed.
This was not obvious initially, as if the page flip subsequently re-attaches the original frame buffer, the refcounts will be balanced. However, if the original frame buffer is freed, then it will be leaked.
Fix this by ensuring that we take a reference on the incoming fb, but rely on the queued work to drop that ref count.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk
Yeah, looks like what I've expected, so Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/armada/armada_crtc.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 1f0875c26dc5..ac2d73f4af45 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -945,18 +945,15 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, armada_reg_queue_end(work->regs, i);
/*
* Hold the old framebuffer for the work - DRM appears to drop our* reference to the old framebuffer in drm_mode_page_flip_ioctl().
* Ensure that we hold a reference on the new framebuffer.* This has to match the behaviour in mode_set.
- drm_framebuffer_reference(work->old_fb);
drm_framebuffer_reference(fb);
ret = armada_drm_crtc_queue_frame_work(dcrtc, work); if (ret) {
/** Undo our reference above; DRM does not drop the reference* to this object on error, so that's okay.*/drm_framebuffer_unreference(work->old_fb);
/* Undo our reference above */drm_framebuffer_unreference(fb);-- 1.8.3.1
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Russell King