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