* [PATCH] drm/amd/display: Don't leak dc_stream_state.
@ 2017-09-08 15:41 Darren Salt
[not found] ` <20170908154134.8657-1-devspam-kUQ7/2e57v/LS5A4iEQ01Vpr/1R2p/CL@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Darren Salt @ 2017-09-08 15:41 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Darren Salt
Noticed while playing “Valley”, which was causing some 8MB of leakage per
second. kmemleak listed many entries looking like this:
unreferenced object 0xffff8802c2951800 (size 1024):
comm "Xorg", pid 2982, jiffies 4297410155 (age 392.787s)
hex dump (first 32 bytes):
00 50 f9 0c 04 88 ff ff 98 08 00 00 00 00 00 00 .P..............
80 07 00 00 00 00 00 00 58 00 00 00 2c 00 00 00 ........X...,...
backtrace:
[<ffffffff810cd4c3>] create_object+0x13c/0x261
[<ffffffff815abdc2>] kmemleak_alloc+0x20/0x3c
[<ffffffff810cad1d>] slab_post_alloc_hook+0x42/0x52
[<ffffffff810cb8e0>] kmem_cache_alloc+0x67/0x76
[<ffffffff813bbb54>] dc_create_stream_for_sink+0x24/0x1cf
[<ffffffff81373aaa>] create_stream_for_sink+0x6f/0x295
[<ffffffff81373dc2>] dm_update_crtcs_state+0xa6/0x268
[<ffffffff8137401e>] amdgpu_dm_atomic_check+0x9a/0x314
[<ffffffff812ac3dd>] drm_atomic_check_only+0x17a/0x42d
[<ffffffff812ac6a3>] drm_atomic_commit+0x13/0x4b
[<ffffffff812ad1a5>] drm_atomic_connector_commit_dpms+0xcb/0xe8
[<ffffffff812b1238>] drm_mode_obj_set_property_ioctl+0xe6/0x1e3
[<ffffffff812b027b>] drm_mode_connector_property_set_ioctl+0x2b/0x2d
[<ffffffff8129f427>] drm_ioctl_kernel+0x64/0x9d
[<ffffffff8129f6a2>] drm_ioctl+0x230/0x316
[<ffffffff812ca4d3>] amdgpu_drm_ioctl+0x4b/0x7d
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 32c75867eaa7..394fc3c03362 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4525,7 +4525,7 @@ static int dm_update_crtcs_state(
if (!drm_atomic_crtc_needs_modeset(crtc_state))
- continue;
+ goto free_new_stream_continue;
DRM_DEBUG_KMS(
"amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
@@ -4543,7 +4543,7 @@ static int dm_update_crtcs_state(
if (!enable) {
if (!old_acrtc_state->stream)
- continue;
+ goto free_new_stream_continue;
DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
crtc->base.id);
@@ -4565,7 +4565,7 @@ static int dm_update_crtcs_state(
} else {/* Add stream for any updated/enabled CRTC */
if (modereset_required(crtc_state))
- continue;
+ goto free_new_stream_continue;
if (modeset_required(crtc_state, new_stream,
old_acrtc_state->stream)) {
@@ -4590,6 +4590,7 @@ static int dm_update_crtcs_state(
}
}
+free_new_stream_continue:
/* Release extra reference */
if (new_stream)
dc_stream_release(new_stream);
--
2.11.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amd/display: Don't leak dc_stream_state.
[not found] ` <20170908154134.8657-1-devspam-kUQ7/2e57v/LS5A4iEQ01Vpr/1R2p/CL@public.gmane.org>
@ 2017-09-12 3:06 ` Alex Deucher
[not found] ` <CADnq5_ON-p=PJq54tdzVG++MtY9u7jXVmwYyG-rOZo-jp1agYA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-09-12 14:37 ` Harry Wentland
1 sibling, 1 reply; 4+ messages in thread
From: Alex Deucher @ 2017-09-12 3:06 UTC (permalink / raw)
To: Darren Salt; +Cc: amd-gfx list
On Fri, Sep 8, 2017 at 11:41 AM, Darren Salt <devspam@moreofthesa.me.uk> wrote:
> Noticed while playing “Valley”, which was causing some 8MB of leakage per
> second. kmemleak listed many entries looking like this:
>
> unreferenced object 0xffff8802c2951800 (size 1024):
> comm "Xorg", pid 2982, jiffies 4297410155 (age 392.787s)
> hex dump (first 32 bytes):
> 00 50 f9 0c 04 88 ff ff 98 08 00 00 00 00 00 00 .P..............
> 80 07 00 00 00 00 00 00 58 00 00 00 2c 00 00 00 ........X...,...
> backtrace:
> [<ffffffff810cd4c3>] create_object+0x13c/0x261
> [<ffffffff815abdc2>] kmemleak_alloc+0x20/0x3c
> [<ffffffff810cad1d>] slab_post_alloc_hook+0x42/0x52
> [<ffffffff810cb8e0>] kmem_cache_alloc+0x67/0x76
> [<ffffffff813bbb54>] dc_create_stream_for_sink+0x24/0x1cf
> [<ffffffff81373aaa>] create_stream_for_sink+0x6f/0x295
> [<ffffffff81373dc2>] dm_update_crtcs_state+0xa6/0x268
> [<ffffffff8137401e>] amdgpu_dm_atomic_check+0x9a/0x314
> [<ffffffff812ac3dd>] drm_atomic_check_only+0x17a/0x42d
> [<ffffffff812ac6a3>] drm_atomic_commit+0x13/0x4b
> [<ffffffff812ad1a5>] drm_atomic_connector_commit_dpms+0xcb/0xe8
> [<ffffffff812b1238>] drm_mode_obj_set_property_ioctl+0xe6/0x1e3
> [<ffffffff812b027b>] drm_mode_connector_property_set_ioctl+0x2b/0x2d
> [<ffffffff8129f427>] drm_ioctl_kernel+0x64/0x9d
> [<ffffffff8129f6a2>] drm_ioctl+0x230/0x316
> [<ffffffff812ca4d3>] amdgpu_drm_ioctl+0x4b/0x7d
Possibly the same issue as this?
https://lists.freedesktop.org/archives/dri-devel/2017-August/150564.html
Alex
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 32c75867eaa7..394fc3c03362 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4525,7 +4525,7 @@ static int dm_update_crtcs_state(
>
>
> if (!drm_atomic_crtc_needs_modeset(crtc_state))
> - continue;
> + goto free_new_stream_continue;
>
> DRM_DEBUG_KMS(
> "amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
> @@ -4543,7 +4543,7 @@ static int dm_update_crtcs_state(
> if (!enable) {
>
> if (!old_acrtc_state->stream)
> - continue;
> + goto free_new_stream_continue;
>
> DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
> crtc->base.id);
> @@ -4565,7 +4565,7 @@ static int dm_update_crtcs_state(
> } else {/* Add stream for any updated/enabled CRTC */
>
> if (modereset_required(crtc_state))
> - continue;
> + goto free_new_stream_continue;
>
> if (modeset_required(crtc_state, new_stream,
> old_acrtc_state->stream)) {
> @@ -4590,6 +4590,7 @@ static int dm_update_crtcs_state(
> }
> }
>
> +free_new_stream_continue:
> /* Release extra reference */
> if (new_stream)
> dc_stream_release(new_stream);
> --
> 2.11.0
>
> _______________________________________________
> 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] 4+ messages in thread
* Re: [PATCH] drm/amd/display: Don't leak dc_stream_state.
[not found] ` <CADnq5_ON-p=PJq54tdzVG++MtY9u7jXVmwYyG-rOZo-jp1agYA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-09-12 3:35 ` Darren Salt
0 siblings, 0 replies; 4+ messages in thread
From: Darren Salt @ 2017-09-12 3:35 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx list
I demand that Alex Deucher may or may not have written...
> On Fri, Sep 8, 2017 at 11:41 AM, Darren Salt <devspam@moreofthesa.me.uk>
wrote:
>> Noticed while playing “Valley”, which was causing some 8MB of leakage per
>> second. kmemleak listed many entries looking like this:
>>
>> unreferenced object 0xffff8802c2951800 (size 1024):
>> comm "Xorg", pid 2982, jiffies 4297410155 (age 392.787s)
>> hex dump (first 32 bytes):
>> 00 50 f9 0c 04 88 ff ff 98 08 00 00 00 00 00 00 .P..............
>> 80 07 00 00 00 00 00 00 58 00 00 00 2c 00 00 00 ........X...,...
>> backtrace:
>> [<ffffffff810cd4c3>] create_object+0x13c/0x261
>> [<ffffffff815abdc2>] kmemleak_alloc+0x20/0x3c
>> [<ffffffff810cad1d>] slab_post_alloc_hook+0x42/0x52
>> [<ffffffff810cb8e0>] kmem_cache_alloc+0x67/0x76
>> [<ffffffff813bbb54>] dc_create_stream_for_sink+0x24/0x1cf
[snip]
> Possibly the same issue as this?
> https://lists.freedesktop.org/archives/dri-devel/2017-August/150564.html
No. I've tested that patch in place of mine, and kmemleak shows the leaks
again.
The offending patch is a5a159d077be / c4e2a09e8414 “drm/amd/display: Refactor
atomic check”: it allocates new_stream but then, in various circumstances,
fails to release it.
If that allocation goes away then I expect that so can this fix.
--
| _ | Darren Salt, using Debian GNU/Linux (and Android)
| ( ) |
| X | ASCII Ribbon campaign against HTML e-mail
| / \ |
Force has no place where there is need of skill.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amd/display: Don't leak dc_stream_state.
[not found] ` <20170908154134.8657-1-devspam-kUQ7/2e57v/LS5A4iEQ01Vpr/1R2p/CL@public.gmane.org>
2017-09-12 3:06 ` Alex Deucher
@ 2017-09-12 14:37 ` Harry Wentland
1 sibling, 0 replies; 4+ messages in thread
From: Harry Wentland @ 2017-09-12 14:37 UTC (permalink / raw)
To: Darren Salt, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Grodzovsky, Andrey
On 2017-09-08 11:41 AM, Darren Salt wrote:
> Noticed while playing “Valley”, which was causing some 8MB of leakage per
> second. kmemleak listed many entries looking like this:
>
> unreferenced object 0xffff8802c2951800 (size 1024):
> comm "Xorg", pid 2982, jiffies 4297410155 (age 392.787s)
> hex dump (first 32 bytes):
> 00 50 f9 0c 04 88 ff ff 98 08 00 00 00 00 00 00 .P..............
> 80 07 00 00 00 00 00 00 58 00 00 00 2c 00 00 00 ........X...,...
> backtrace:
> [<ffffffff810cd4c3>] create_object+0x13c/0x261
> [<ffffffff815abdc2>] kmemleak_alloc+0x20/0x3c
> [<ffffffff810cad1d>] slab_post_alloc_hook+0x42/0x52
> [<ffffffff810cb8e0>] kmem_cache_alloc+0x67/0x76
> [<ffffffff813bbb54>] dc_create_stream_for_sink+0x24/0x1cf
> [<ffffffff81373aaa>] create_stream_for_sink+0x6f/0x295
> [<ffffffff81373dc2>] dm_update_crtcs_state+0xa6/0x268
> [<ffffffff8137401e>] amdgpu_dm_atomic_check+0x9a/0x314
> [<ffffffff812ac3dd>] drm_atomic_check_only+0x17a/0x42d
> [<ffffffff812ac6a3>] drm_atomic_commit+0x13/0x4b
> [<ffffffff812ad1a5>] drm_atomic_connector_commit_dpms+0xcb/0xe8
> [<ffffffff812b1238>] drm_mode_obj_set_property_ioctl+0xe6/0x1e3
> [<ffffffff812b027b>] drm_mode_connector_property_set_ioctl+0x2b/0x2d
> [<ffffffff8129f427>] drm_ioctl_kernel+0x64/0x9d
> [<ffffffff8129f6a2>] drm_ioctl+0x230/0x316
> [<ffffffff812ca4d3>] amdgpu_drm_ioctl+0x4b/0x7d
Please leave a Signed-off-by tag (see part 10 of
https://www.kernel.org/doc/html/v4.11/process/submitting-patches.html).
Otherwise the patch looks good. Please resend with the Signed-off-by.
Looks like we'd have the same problem with the break statements. This
wouldn't be as noticeable since they are error conditions.
Thanks,
Harry
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 32c75867eaa7..394fc3c03362 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4525,7 +4525,7 @@ static int dm_update_crtcs_state(
>
>
> if (!drm_atomic_crtc_needs_modeset(crtc_state))
> - continue;
> + goto free_new_stream_continue;
>
> DRM_DEBUG_KMS(
> "amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, "
> @@ -4543,7 +4543,7 @@ static int dm_update_crtcs_state(
> if (!enable) {
>
> if (!old_acrtc_state->stream)
> - continue;
> + goto free_new_stream_continue;
>
> DRM_DEBUG_KMS("Disabling DRM crtc: %d\n",
> crtc->base.id);
> @@ -4565,7 +4565,7 @@ static int dm_update_crtcs_state(
> } else {/* Add stream for any updated/enabled CRTC */
>
> if (modereset_required(crtc_state))
> - continue;
> + goto free_new_stream_continue;
>
> if (modeset_required(crtc_state, new_stream,
> old_acrtc_state->stream)) {
> @@ -4590,6 +4590,7 @@ static int dm_update_crtcs_state(
> }
> }
>
> +free_new_stream_continue:
> /* Release extra reference */
> if (new_stream)
> dc_stream_release(new_stream);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-09-12 14:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-08 15:41 [PATCH] drm/amd/display: Don't leak dc_stream_state Darren Salt
[not found] ` <20170908154134.8657-1-devspam-kUQ7/2e57v/LS5A4iEQ01Vpr/1R2p/CL@public.gmane.org>
2017-09-12 3:06 ` Alex Deucher
[not found] ` <CADnq5_ON-p=PJq54tdzVG++MtY9u7jXVmwYyG-rOZo-jp1agYA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-09-12 3:35 ` Darren Salt
2017-09-12 14:37 ` Harry Wentland
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.