Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

["Christian König" <christian.koenig-5C7GfCeVMHo@public.gmane.org>]

Am 18.09.2018 um 10:16 schrieb Zhu, Rex:
>
>> -----Original Message-----
>> From: Christian König <ckoenig.leichtzumerken@gmail.com>
>> Sent: Tuesday, September 18, 2018 3:14 PM
>> To: Zhu, Rex <Rex.Zhu@amd.com>; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
>>
>> Am 18.09.2018 um 08:16 schrieb Zhu, Rex:
>>>> -----Original Message-----
>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>>> Christian König
>>>> Sent: Tuesday, September 18, 2018 2:07 AM
>>>> To: amd-gfx@lists.freedesktop.org
>>>> Subject: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
>>>>
>>>> Don't try to unreserve a BO we doesn't allocated.
>>>>
>>>> Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel BOs
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> index 84d82d5382f9..c1387efc0c91 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> @@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct
>> amdgpu_device
>>>> *adev,
>>>> 	if (r)
>>>>    		return r;
>>>> -	amdgpu_bo_unreserve(*bo_ptr);
>>>> +	if (*bo_ptr)
>>>> +		amdgpu_bo_unreserve(*bo_ptr);
>>>>
>>>>    	return 0;
>>>>    }
>>> It is weird.
>>> If we return true for allocate bo with size  0.
>>> Does that mean we need to check all the bo_ptr before we use them.
>> No, allocating a BO with zero size doesn't make much sense and was
>> essentially undefined behavior previously.
>>
>> So now we get a defined behavior, but not necessary the one you expected.
>>
>> Is that only a rhetorical question or really a problem somewhere?
> Logically, the code is trick.

Yeah, that is not a bad argument.

But it also makes the function more useful, e.g. we don't need size 
checks any more whenever an optional BO is allocated.

>   It also make the  code
> if (r)
> 	return r;
> redundant.

Actually that is not correct.

When an error happens the *bo_ptr is not modified at all. So we could 
then try to unreserve a BO which was never reserved.

Christian.

>
> Regards
> Rex
>
>> Regards,
>> Christian.
>>
>>> Best Regards
>>> Rex
>>>
>>>> --
>>>> 2.14.1
>>>>
>>>> _______________________________________________
>>>> 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