* [PATCH] drm/amdgpu: release parent fence reference
@ 2016-10-23 18:31 Grazvydas Ignotas
[not found] ` <1477247507-11378-4-git-send-email-notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Grazvydas Ignotas @ 2016-10-23 18:31 UTC (permalink / raw)
To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
It's done in amd_sched_hw_job_reset(), but not in normal job processing.
Leak reported by CONFIG_SLUB_DEBUG.
Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
CONFIG_SLUB_DEBUG reports more leaks related to ioctls,
but I was unable to track them down...
drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 910b8d5..cfb686e 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -522,6 +522,8 @@ static void amd_sched_process_job(struct fence *f, struct fence_cb *cb)
trace_amd_sched_process_job(s_fence);
fence_put(&s_fence->finished);
+ fence_put(s_fence->parent);
+ s_fence->parent = NULL;
wake_up_interruptible(&sched->wake_up_worker);
}
--
2.7.4
_______________________________________________
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: <1477247507-11378-4-git-send-email-notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: release parent fence reference [not found] ` <1477247507-11378-4-git-send-email-notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2016-10-24 2:25 ` zhoucm1 [not found] ` <580D70FE.4010404-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: zhoucm1 @ 2016-10-24 2:25 UTC (permalink / raw) To: Grazvydas Ignotas, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2016年10月24日 02:31, Grazvydas Ignotas wrote: > It's done in amd_sched_hw_job_reset(), but not in normal job processing. > Leak reported by CONFIG_SLUB_DEBUG. > > Signed-off-by: Grazvydas Ignotas <notasas@gmail.com> > --- > CONFIG_SLUB_DEBUG reports more leaks related to ioctls, > but I was unable to track them down... > > drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c > index 910b8d5..cfb686e 100644 > --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c > +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c > @@ -522,6 +522,8 @@ static void amd_sched_process_job(struct fence *f, struct fence_cb *cb) > > trace_amd_sched_process_job(s_fence); > fence_put(&s_fence->finished); > + fence_put(s_fence->parent); If I remember correctly, parent was put in sched fence release. Regards, David Zhou > + s_fence->parent = NULL; > wake_up_interruptible(&sched->wake_up_worker); > } > _______________________________________________ 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: <580D70FE.4010404-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: release parent fence reference [not found] ` <580D70FE.4010404-5C7GfCeVMHo@public.gmane.org> @ 2016-10-24 9:13 ` Christian König 2016-10-24 9:43 ` Grazvydas Ignotas 0 siblings, 1 reply; 6+ messages in thread From: Christian König @ 2016-10-24 9:13 UTC (permalink / raw) To: zhoucm1, Grazvydas Ignotas, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 24.10.2016 um 04:25 schrieb zhoucm1: > > > On 2016年10月24日 02:31, Grazvydas Ignotas wrote: >> It's done in amd_sched_hw_job_reset(), but not in normal job processing. >> Leak reported by CONFIG_SLUB_DEBUG. >> >> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com> >> --- >> CONFIG_SLUB_DEBUG reports more leaks related to ioctls, >> but I was unable to track them down... >> >> drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >> index 910b8d5..cfb686e 100644 >> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >> @@ -522,6 +522,8 @@ static void amd_sched_process_job(struct fence >> *f, struct fence_cb *cb) >> trace_amd_sched_process_job(s_fence); >> fence_put(&s_fence->finished); >> + fence_put(s_fence->parent); > If I remember correctly, parent was put in sched fence release. Yes, exactly. It's only released in amd_sched_hw_job_reset() because we want to assign a new parent fence. So this is a clear NAK. Regards, Christian. > > Regards, > David Zhou >> + s_fence->parent = NULL; >> wake_up_interruptible(&sched->wake_up_worker); >> } > > _______________________________________________ > 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
* Re: [PATCH] drm/amdgpu: release parent fence reference 2016-10-24 9:13 ` Christian König @ 2016-10-24 9:43 ` Grazvydas Ignotas 2016-10-28 11:47 ` Christian König 0 siblings, 1 reply; 6+ messages in thread From: Grazvydas Ignotas @ 2016-10-24 9:43 UTC (permalink / raw) To: Christian König; +Cc: amd-gfx, dri-devel On Mon, Oct 24, 2016 at 12:13 PM, Christian König <deathsimple@vodafone.de> wrote: > Am 24.10.2016 um 04:25 schrieb zhoucm1: >> >> >> >> On 2016年10月24日 02:31, Grazvydas Ignotas wrote: >>> >>> It's done in amd_sched_hw_job_reset(), but not in normal job processing. >>> Leak reported by CONFIG_SLUB_DEBUG. >>> >>> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com> >>> --- >>> CONFIG_SLUB_DEBUG reports more leaks related to ioctls, >>> but I was unable to track them down... >>> >>> drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>> index 910b8d5..cfb686e 100644 >>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>> @@ -522,6 +522,8 @@ static void amd_sched_process_job(struct fence *f, >>> struct fence_cb *cb) >>> trace_amd_sched_process_job(s_fence); >>> fence_put(&s_fence->finished); >>> + fence_put(s_fence->parent); >> >> If I remember correctly, parent was put in sched fence release. > > > Yes, exactly. It's only released in amd_sched_hw_job_reset() because we want > to assign a new parent fence. Well then it doesn't work, SLUB_DEBUG detects leaks on teardown, and with my patch they're gone. Perhaps the problem is that when new parent fence is assigned, the old one is not put? I won't be able to check until the weekend, so if anybody else can do it, please go ahead. Gražvydas _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: release parent fence reference 2016-10-24 9:43 ` Grazvydas Ignotas @ 2016-10-28 11:47 ` Christian König [not found] ` <35ee2288-e34c-071c-38af-42e6d35ce48e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Christian König @ 2016-10-28 11:47 UTC (permalink / raw) To: Grazvydas Ignotas; +Cc: amd-gfx, dri-devel Am 24.10.2016 um 11:43 schrieb Grazvydas Ignotas: > On Mon, Oct 24, 2016 at 12:13 PM, Christian König > <deathsimple@vodafone.de> wrote: >> Am 24.10.2016 um 04:25 schrieb zhoucm1: >>> >>> >>> On 2016年10月24日 02:31, Grazvydas Ignotas wrote: >>>> It's done in amd_sched_hw_job_reset(), but not in normal job processing. >>>> Leak reported by CONFIG_SLUB_DEBUG. >>>> >>>> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com> >>>> --- >>>> CONFIG_SLUB_DEBUG reports more leaks related to ioctls, >>>> but I was unable to track them down... >>>> >>>> drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>>> index 910b8d5..cfb686e 100644 >>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>>> @@ -522,6 +522,8 @@ static void amd_sched_process_job(struct fence *f, >>>> struct fence_cb *cb) >>>> trace_amd_sched_process_job(s_fence); >>>> fence_put(&s_fence->finished); >>>> + fence_put(s_fence->parent); >>> If I remember correctly, parent was put in sched fence release. >> >> Yes, exactly. It's only released in amd_sched_hw_job_reset() because we want >> to assign a new parent fence. > Well then it doesn't work, SLUB_DEBUG detects leaks on teardown, and > with my patch they're gone. > Perhaps the problem is that when new parent fence is assigned, the old > one is not put? I won't be able to check until the weekend, so if > anybody else can do it, please go ahead. Mhm, what steps do you do to reproduce this? I'm looking into this a bit and it sounds like we just doesn't correctly tear down the scheduler on driver unload. Regards, Christian. > > Gražvydas _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <35ee2288-e34c-071c-38af-42e6d35ce48e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: release parent fence reference [not found] ` <35ee2288-e34c-071c-38af-42e6d35ce48e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> @ 2016-10-28 12:21 ` Christian König 0 siblings, 0 replies; 6+ messages in thread From: Christian König @ 2016-10-28 12:21 UTC (permalink / raw) To: Grazvydas Ignotas Cc: zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 28.10.2016 um 13:47 schrieb Christian König: > Am 24.10.2016 um 11:43 schrieb Grazvydas Ignotas: >> On Mon, Oct 24, 2016 at 12:13 PM, Christian König >> <deathsimple@vodafone.de> wrote: >>> Am 24.10.2016 um 04:25 schrieb zhoucm1: >>>> >>>> >>>> On 2016年10月24日 02:31, Grazvydas Ignotas wrote: >>>>> It's done in amd_sched_hw_job_reset(), but not in normal job >>>>> processing. >>>>> Leak reported by CONFIG_SLUB_DEBUG. >>>>> >>>>> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com> >>>>> --- >>>>> CONFIG_SLUB_DEBUG reports more leaks related to ioctls, >>>>> but I was unable to track them down... >>>>> >>>>> drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>>>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>>>> index 910b8d5..cfb686e 100644 >>>>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>>>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c >>>>> @@ -522,6 +522,8 @@ static void amd_sched_process_job(struct fence >>>>> *f, >>>>> struct fence_cb *cb) >>>>> trace_amd_sched_process_job(s_fence); >>>>> fence_put(&s_fence->finished); >>>>> + fence_put(s_fence->parent); >>>> If I remember correctly, parent was put in sched fence release. >>> >>> Yes, exactly. It's only released in amd_sched_hw_job_reset() because >>> we want >>> to assign a new parent fence. >> Well then it doesn't work, SLUB_DEBUG detects leaks on teardown, and >> with my patch they're gone. >> Perhaps the problem is that when new parent fence is assigned, the old >> one is not put? I won't be able to check until the weekend, so if >> anybody else can do it, please go ahead. > > Mhm, what steps do you do to reproduce this? Never mind, I was able to figure out how to trigger it. You actually have to create a fence to leak it :) Regards, Christian. > > I'm looking into this a bit and it sounds like we just doesn't > correctly tear down the scheduler on driver unload. > > Regards, > Christian. > >> >> Gražvydas > > _______________________________________________ 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:[~2016-10-28 12:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-23 18:31 [PATCH] drm/amdgpu: release parent fence reference Grazvydas Ignotas
[not found] ` <1477247507-11378-4-git-send-email-notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-10-24 2:25 ` zhoucm1
[not found] ` <580D70FE.4010404-5C7GfCeVMHo@public.gmane.org>
2016-10-24 9:13 ` Christian König
2016-10-24 9:43 ` Grazvydas Ignotas
2016-10-28 11:47 ` Christian König
[not found] ` <35ee2288-e34c-071c-38af-42e6d35ce48e-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2016-10-28 12:21 ` Christian König
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.