On 2017å¹´04æ??17æ?¥ 22:54, Xie, AlexBin wrote: > > Hi David, > > > Thanks for the comments. However, please have look at > amdgpu_bo_reserve definition. > > static inline int amdgpu_bo_reserve(struct amdgpu_bo *bo, bool no_intr) > Ah, this is a wired wrapper for ttm_bo_reserve. > > When we call this function like the following: > > r = amdgpu_bo_reserve(adev->vram_scratch.robj, false); > The false means interruptible. > > > On the other hand, when amdgpu_bo_reserve function return error, why > do we unref BO without kunmap and unpin the BO? This is wrong > implementation when amdgpu_bo_reserve return any error. > Yeah, I see your mean, it's in driver un-loading, How about changing to no interruptible? Your patch will make a memleak if bo_reserve fails, but it seems not matter. I have no strong preference. Regards, David Zhou > > > Thanks, > Alex Bin Xie > > ------------------------------------------------------------------------ > *From:* Zhou, David(ChunMing) > *Sent:* Friday, April 14, 2017 12:00 AM > *To:* Xie, AlexBin; amd-gfx at lists.freedesktop.org > *Subject:* Re: [PATCH] dmr/amdgpu: Fix wrongly unref of BO > > > On 2017å¹´04æ??14æ?¥ 05:34, Alex Xie wrote: > > According to comment of amdgpu_bo_reserve, amdgpu_bo_reserve > > can return with -ERESTARTSYS. When this function was interrupted > > by a signal, BO should not be unref. Otherwise the BO might be > > released while is kmapped and pinned, or BO MIGHT be deref > > multiple times, etc. > r = amdgpu_bo_reserve(adev->vram_scratch.robj, false); > we have specified interruptible to false, so -ERESTARTSYS isn't possible > here. > > Thanks, > David Zhou > > > > Change-Id: If76071a768950a0d3ad9d5da7fcae04881807621 > > Signed-off-by: Alex Xie <AlexBin.Xie at amd.com> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > index 53996e3..1dcc2d1 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > @@ -355,8 +355,8 @@ static void amdgpu_vram_scratch_fini(struct > amdgpu_device *adev) > > amdgpu_bo_kunmap(adev->vram_scratch.robj); > > amdgpu_bo_unpin(adev->vram_scratch.robj); > > amdgpu_bo_unreserve(adev->vram_scratch.robj); > > + amdgpu_bo_unref(&adev->vram_scratch.robj); > > } > > - amdgpu_bo_unref(&adev->vram_scratch.robj); > > } > > > > /** > -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170418/8616a6e6/attachment.html>
