[PATCH] drm/amd/display: put commit when -ERESTARTSYS received

[Daniel Vetter]

On Wed, Aug 16, 2017 at 7:12 PM, Alex Deucher <alexdeucher at gmail.com> wrote:
> On Wed, Aug 16, 2017 at 10:22 AM, Jerry Zuo <Jerry.Zuo at amd.com> wrote:
>> During page flip atomic_check and atomic_commit can return
>> -ERESTARTSYS to restart the ioctl. When this happens we fail to
>> put the commit object leading to a memory leak.
>>
>> Signed-off-by: Jerry Zuo <Jerry.Zuo at amd.com>
>
> The subject should be:
> drm/atomic: put commit when -ERESTARTSYS received
> Since the change is in the core atomic code.

Do we have an igt testcase to exercise this? This is the kind of error
case handling igt really is made for, and igt has ready-made helpers
to interrupt ioctls. I think Maarten was even working on such a
testcase, adding him.
-Daniel

>
> Alex
>
>> ---
>>  drivers/gpu/drm/drm_atomic.c | 25 +++++++++++++++++++++++--
>>  1 file changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index f32506a7c1d6..f2f623dacf90 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -1642,14 +1642,35 @@ int drm_atomic_nonblocking_commit(struct drm_atomic_state *state)
>>  {
>>         struct drm_mode_config *config = &state->dev->mode_config;
>>         int ret;
>> +       int i;
>> +       struct drm_crtc *crtc;
>> +       struct drm_crtc_state *crtc_state;
>>
>>         ret = drm_atomic_check_only(state);
>> -       if (ret)
>> +       if (ret) {
>> +               if (ret == -ERESTARTSYS)
>> +                       goto fail;
>> +
>>                 return ret;
>> +       }
>>
>>         DRM_DEBUG_ATOMIC("commiting %p nonblocking\n", state);
>>
>> -       return config->funcs->atomic_commit(state->dev, state, true);
>> +       ret = config->funcs->atomic_commit(state->dev, state, true);
>> +       if (ret == -ERESTARTSYS)
>> +               goto fail;
>> +
>> +       return ret;
>> +
>> +       /* cleanup commit object if commit fails with ERESTARTSYS */
>> +fail:
>> +       for_each_crtc_in_state(state, crtc, crtc_state, i) {
>> +               if (state->crtcs[i].commit) {
>> +                       drm_crtc_commit_put(state->crtcs[i].commit);
>> +               }
>> +       }
>> +
>> +       return ret;
>>  }
>>  EXPORT_SYMBOL(drm_atomic_nonblocking_commit);
>>
>> --
>> 2.11.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch