[PATCH 1/3] drm/atomic-helper: Fix leak in disable_all

Chris Wilson chris at chris-wilson.co.uk
Fri Jul 14 19:27:32 UTC 2017


Quoting Daniel Vetter (2017-07-14 20:14:37)
> The legacy plane->fb pointer is refcounted by calling
> drm_atomic_clean_old_fb().
> 
> In practice this isn't a real problem because:
> - The caller in the i915 gpu reset code restores the original state
>   again, which means the plane->fb pointer won't change, hence can't
>   leak.
> - Drivers using drm_atomic_helper_shutdown call the fbdev cleanup
>   first, and that usually cleans up the fb through
>   drm_remove_framebuffer, which does this correctly.
> - Without fbdev the only framebuffers are from userspace, and those
>   get cleaned up (again using drm_remove_framebuffer) befor the driver
>   can even be unloaded.
> 
> But in i915 I've switched the cleanup sequence around so that the
> _shutdown() calls happens after the drm_remove_framebuffer(), which is
> how I discovered this issue.

If only we have refcounted fb now and didn't need every caller manually
tracking the old pointers. </s>

Given the requirement for the caller to cleanup old_fb around
drm_atomic_commit(), this looks correct to me.

> Cc: martin.peres at free.fr
> Cc: chris at chris-wilson.co.uk
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris


More information about the dri-devel mailing list