Am 18.09.2018 um 10:16 schrieb Zhu, Rex: > >> -----Original Message----- >> From: Christian König <ckoenig.leichtzumerken at gmail.com> >> Sent: Tuesday, September 18, 2018 3:14 PM >> To: Zhu, Rex <Rex.Zhu at amd.com>; amd-gfx at 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 at lists.freedesktop.org> On Behalf Of >>>> Christian König >>>> Sent: Tuesday, September 18, 2018 2:07 AM >>>> To: amd-gfx at 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 at 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 at lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx