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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 10, 2018 at 6:00 AM, Maarten Lankhorst
<maarten.lankhorst@xxxxxxxxxxxxxxx> wrote:
> From: "Leo (Sunpeng) Li" <sunpeng.li@xxxxxxx>
>
> 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@xxxxxxxxxxxxxxx> # v4.11+
> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@xxxxxxx>
> Acked-by: Harry Wentland <harry.wentland@xxxxxxx> #v1
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]