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

[zhoucm1 <david1.zhou-5C7GfCeVMHo@public.gmane.org>]

On 2017年04月07日 16:55, Christian König wrote:
> 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@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.
I see your mean, will send V2 soon.

Regards,
David Zhou
>
> 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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx