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); }
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); }
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
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
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
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
dri-devel@lists.freedesktop.org
-
Christian König -
Grazvydas Ignotas -
zhoucm1