[PATCH] drm/amdgpu: fix memory leak in wait_all_fence

[Christian König]

Am 07.04.2017 um 10:46 schrieb zhoucm1:
>
>
> On 2017年04月07日 16:36, Christian König wrote:
>> Am 07.04.2017 um 05:36 schrieb Chunming Zhou:
>>> Change-Id: Ib3e271e00e49f10152c1b3eace981a6bf78820de
>>> Signed-off-by: Chunming Zhou <David1.Zhou at amd.com>
>>
>> NAK, that will allocate an array for the fence again, which we wanted 
>> to avoid.
> I don't got your means, the **array just to store fence temporary, 
> freed at the end.

Yeah, but that is unnecessary. We created this function to just avoid that.

>
>>
>> We should just drop the fence reference directly after waiting for it.
> How to handle err case in the middle.

just put a fence_put() directly after fence_wait_timeout(). We don't 
need to keep a reference to the fence till the end.

Regards,
Christian.

>
> Regards,
> David Zhou
>>
>> Regards,
>> Christian.
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 25 +++++++++++++++++++------
>>>   1 file changed, 19 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> index de1c4c3..d842452 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> @@ -1216,22 +1216,28 @@ static int amdgpu_cs_wait_all_fences(struct 
>>> amdgpu_device *adev,
>>>                        struct drm_amdgpu_fence *fences)
>>>   {
>>>       uint32_t fence_count = wait->in.fence_count;
>>> +    struct fence **array;
>>>       unsigned int i;
>>>       long r = 1;
>>>   +    array = kcalloc(fence_count, sizeof(struct fence *), 
>>> GFP_KERNEL);
>>> +
>>> +    if (array == NULL)
>>> +        return -ENOMEM;
>>>       for (i = 0; i < fence_count; i++) {
>>>           struct fence *fence;
>>>           unsigned long timeout = 
>>> amdgpu_gem_timeout(wait->in.timeout_ns);
>>>             fence = amdgpu_cs_get_fence(adev, filp, &fences[i]);
>>> -        if (IS_ERR(fence))
>>> -            return PTR_ERR(fence);
>>> -        else if (!fence)
>>> +        if (IS_ERR(fence)) {
>>> +            r = PTR_ERR(fence);
>>> +            goto err;
>>> +        } else if (!fence)
>>>               continue;
>>> -
>>> +        array[i] = fence;
>>>           r = kcl_fence_wait_timeout(fence, true, timeout);
>>>           if (r < 0)
>>> -            return r;
>>> +            goto err;
>>>             if (r == 0)
>>>               break;
>>> @@ -1240,7 +1246,14 @@ static int amdgpu_cs_wait_all_fences(struct 
>>> amdgpu_device *adev,
>>>       memset(wait, 0, sizeof(*wait));
>>>       wait->out.status = (r > 0);
>>>   -    return 0;
>>> +    r = 0;
>>> +
>>> +err:
>>> +    for (i = 0; i < fence_count; i++)
>>> +        fence_put(array[i]);
>>> +    kfree(array);
>>> +
>>> +    return r;
>>>   }
>>>     /**
>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx