Am 18.09.2018 um 11:27 schrieb Zhu, Rex:
-----Original Message-----
From: Koenig, Christian
Sent: Tuesday, September 18, 2018 4:41 PM
To: Zhu, Rex <rex....@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
Am 18.09.2018 um 10:16 schrieb Zhu, Rex:
-----Original Message-----
From: Christian König <ckoenig.leichtzumer...@gmail.com>
Sent: Tuesday, September 18, 2018 3:14 PM
To: Zhu, Rex <rex....@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-boun...@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.koe...@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.
Yes, the problem is user don't need size check. But user have no way to check
whether the bo is allocated successfully.
Because in size 0 case, the create function also return true.
And as you said below, check bo_ptr is not safe either(the *bo_ptr may be not
modified at all).
That is not correct. When size==0 we call amdgpu_bo_unref(bo_ptr), and
that one sets bo_ptr to NULL.
When size==0 was possible before, the calling code would have needed to
check bo_ptr later on anyway.
In other words what we had before:
calling_function()
{
if (size) {
r = amdgpu_bo_create_kernel(..., size, &bo);
if (r)
goto error_handling;
}
....
if (bo)
....
}
But now that looks like:
calling_function()
{
r = amdgpu_bo_create_kernel(..., size, &bo);
if (r)
goto error_handling;
....
if (bo) {
....
}
So we just removed the extra size check of the calling function. I think
that is a valid usage.
Christian.
Regards
Rex
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.
Yes, You ae right.
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
