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

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

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.

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

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