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

From: Alex Deucher <alexdeucher@gmail.com>
To: Harry Wentland <harry.wentland@amd.com>
Cc: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>,
	Maling list - DRI developers <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
Date: Tue, 9 Jan 2018 13:13:33 -0500	[thread overview]
Message-ID: <CADnq5_OATUZPsDZXdS5YtTj8Q6BzVrmbAV9GwqQzrYEURUjoJw@mail.gmail.com> (raw)
In-Reply-To: <846a20f2-9453-0b10-614c-7df14c15a25b@amd.com>

On Mon, Jan 8, 2018 at 4:30 PM, Harry Wentland <harry.wentland@amd.com> wrote:
> On 2018-01-04 02:47 PM, sunpeng.li@amd.com wrote:
>> From: "Leo (Sunpeng) Li" <sunpeng.li@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@amd.com>
>
> No expert on this but looks sane to me.
>
> Acked-by: Harry Wentland <harry.wentland@amd.com>

Pushed to drm-misc-next.

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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel