[PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits

[Daniel Vetter]

On Tue, Jan 09, 2018 at 01:13:33PM -0500, Alex Deucher wrote:
> On Mon, Jan 8, 2018 at 4:30 PM, Harry Wentland <harry.wentland at amd.com> wrote:
> > On 2018-01-04 02:47 PM, sunpeng.li at amd.com wrote:
> >> From: "Leo (Sunpeng) Li" <sunpeng.li at amd.com>
> >>
> >> During a non-blocking commit, it is possible to return before the
> >> commit_tail work is queued (-ERESTARTSYS, for example).
> >>
> >> Since a reference on the crtc commit object is obtained for the pending
> >> vblank event when preparing the commit, the above situation will leave
> >> us with an extra reference.
> >>
> >> Therefore, if the commit_tail worker has not consumed the event at the
> >> end of a commit, release it's reference.
> >>
> >> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li at amd.com>
> >
> > No expert on this but looks sane to me.
> >
> > Acked-by: Harry Wentland <harry.wentland at amd.com>
> 
> 
> Pushed to drm-misc-next.

This blows up all over the place in our CI:

https://bugs.freedesktop.org/show_bug.cgi?id=104566

I think as a leak fix it should also be pushed to -fixes. And we didn't
really figure out what's wrong, so we're going with the quick revert until
you folks are all awake.

Cheers, Daniel

> 
> Thanks,
> 
> Alex
> 
> >
> > Harry
> >
> >> ---
> >>  drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> >> index ab40321..4253f57 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -3421,6 +3421,15 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
> >>  void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
> >>  {
> >>       if (state->commit) {
> >> +             /*
> >> +              * In the event that a non-blocking commit returns
> >> +              * -ERESTARTSYS before the commit_tail work is queued, we will
> >> +              * have an extra reference to the commit object. Release it, if
> >> +              * the event has not been consumed by the worker.
> >> +              */
> >> +             if (state->event)
> >> +                     drm_crtc_commit_put(state->commit);
> >> +
> >>               kfree(state->commit->event);
> >>               state->commit->event = NULL;
> >>               drm_crtc_commit_put(state->commit);
> >>
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch