[Intel-gfx] [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits, v2.

[Alex Deucher]

On Wed, Jan 10, 2018 at 6:00 AM, Maarten Lankhorst
<maarten.lankhorst at linux.intel.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.
>
> Changes since v1:
> - Also check for state->event->base.completion being set, to
>   handle the case where stall_checks() fails in setup_crtc_commit().
>
> Fixes: 24835e442f28 ("drm: reference count event->completion")

Thanks for fixing this up.  You mentioned on IRC that this version
still caused problems.  What were those?

Thanks,

Alex

> Cc: <stable at vger.kernel.org> # v4.11+
> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li at amd.com>
> Acked-by: Harry Wentland <harry.wentland at amd.com> #v1
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
>  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 b16f1d69a0bb..1d43f3e85a7d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3327,6 +3327,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 && state->event->base.completion)
> +                       drm_crtc_commit_put(state->commit);
> +
>                 kfree(state->commit->event);
>                 state->commit->event = NULL;
>                 drm_crtc_commit_put(state->commit);
> --
> 2.15.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel