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

[Harry Wentland]

On 2017-08-16 01:22 PM, Daniel Vetter wrote:
> 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.

I'm not aware of an IGT test for this. We triggered this scenario when
running mode changes while glxgears is running. We observed no
user-visible issue, only a memory leak. Is IGT able to capture those
(semi-)reliably?

I agree that it would be great to have an IGT test for these corner cases.

Harry

> -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
> 
> 
>