* [PATCH] drm/amdgpu: fix memory leak in wait_all_fence
@ 2017-04-07 3:36 Chunming Zhou
[not found] ` <1491536178-5978-1-git-send-email-David1.Zhou-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Chunming Zhou @ 2017-04-07 3:36 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ken.Wang-5C7GfCeVMHo
Cc: Chunming Zhou
Change-Id: Ib3e271e00e49f10152c1b3eace981a6bf78820de
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
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;
}
/**
--
1.9.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread[parent not found: <1491536178-5978-1-git-send-email-David1.Zhou-5C7GfCeVMHo@public.gmane.org>]
* 答复: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence [not found] ` <1491536178-5978-1-git-send-email-David1.Zhou-5C7GfCeVMHo@public.gmane.org> @ 2017-04-07 6:45 ` Wang, Ken 2017-04-07 8:36 ` Christian König 1 sibling, 0 replies; 6+ messages in thread From: Wang, Ken @ 2017-04-07 6:45 UTC (permalink / raw) To: Zhou, David(ChunMing), amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW [-- Attachment #1.1: Type: text/plain, Size: 2594 bytes --] Reviewed-by: Ken Wang <Qingqing.Wang@amd.com> ________________________________ 发件人: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> 代表 Chunming Zhou <David1.Zhou@amd.com> 发送时间: 2017年4月7日 11:36:18 收件人: amd-gfx@lists.freedesktop.org; Wang, Ken 抄送: Zhou, David(ChunMing) 主题: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence Change-Id: Ib3e271e00e49f10152c1b3eace981a6bf78820de Signed-off-by: Chunming Zhou <David1.Zhou@amd.com> --- 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; } /** -- 1.9.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx [-- Attachment #1.2: Type: text/html, Size: 6743 bytes --] [-- Attachment #2: Type: text/plain, Size: 154 bytes --] _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence [not found] ` <1491536178-5978-1-git-send-email-David1.Zhou-5C7GfCeVMHo@public.gmane.org> 2017-04-07 6:45 ` 答复: " Wang, Ken @ 2017-04-07 8:36 ` Christian König [not found] ` <058491a9-73a6-eddf-73fb-aaac066f9b3e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 1 sibling, 1 reply; 6+ messages in thread From: Christian König @ 2017-04-07 8:36 UTC (permalink / raw) To: Chunming Zhou, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ken.Wang-5C7GfCeVMHo 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. We should just drop the fence reference directly after waiting for it. 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <058491a9-73a6-eddf-73fb-aaac066f9b3e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence [not found] ` <058491a9-73a6-eddf-73fb-aaac066f9b3e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> @ 2017-04-07 8:46 ` zhoucm1 [not found] ` <58E751D5.50707-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: zhoucm1 @ 2017-04-07 8:46 UTC (permalink / raw) To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ken.Wang-5C7GfCeVMHo 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <58E751D5.50707-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence [not found] ` <58E751D5.50707-5C7GfCeVMHo@public.gmane.org> @ 2017-04-07 8:55 ` Christian König [not found] ` <6aa1afcf-b2d3-ea84-b08c-3520fffb0b7c-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Christian König @ 2017-04-07 8:55 UTC (permalink / raw) To: zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ken.Wang-5C7GfCeVMHo Am 07.04.2017 um 10:46 schrieb zhoucm1: > > > 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. Yeah, but that is unnecessary. We created this function to just avoid that. > >> >> We should just drop the fence reference directly after waiting for it. > How to handle err case in the middle. just put a fence_put() directly after fence_wait_timeout(). We don't need to keep a reference to the fence till the end. Regards, Christian. > > 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 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <6aa1afcf-b2d3-ea84-b08c-3520fffb0b7c-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence [not found] ` <6aa1afcf-b2d3-ea84-b08c-3520fffb0b7c-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> @ 2017-04-07 9:03 ` zhoucm1 0 siblings, 0 replies; 6+ messages in thread From: zhoucm1 @ 2017-04-07 9:03 UTC (permalink / raw) To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ken.Wang-5C7GfCeVMHo On 2017年04月07日 16:55, Christian König wrote: > Am 07.04.2017 um 10:46 schrieb zhoucm1: >> >> >> 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. > > Yeah, but that is unnecessary. We created this function to just avoid > that. > >> >>> >>> We should just drop the fence reference directly after waiting for it. >> How to handle err case in the middle. > > just put a fence_put() directly after fence_wait_timeout(). We don't > need to keep a reference to the fence till the end. I see your mean, will send V2 soon. Regards, David Zhou > > Regards, > Christian. > >> >> 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 > > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-07 9:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-07 3:36 [PATCH] drm/amdgpu: fix memory leak in wait_all_fence Chunming Zhou
[not found] ` <1491536178-5978-1-git-send-email-David1.Zhou-5C7GfCeVMHo@public.gmane.org>
2017-04-07 6:45 ` 答复: " Wang, Ken
2017-04-07 8:36 ` Christian König
[not found] ` <058491a9-73a6-eddf-73fb-aaac066f9b3e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-04-07 8:46 ` zhoucm1
[not found] ` <58E751D5.50707-5C7GfCeVMHo@public.gmane.org>
2017-04-07 8:55 ` Christian König
[not found] ` <6aa1afcf-b2d3-ea84-b08c-3520fffb0b7c-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-04-07 9:03 ` zhoucm1
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.