* [PATCH] drm/rockchip: Fix memory leak in rockchip_drm_sys_resume()
@ 2017-09-27 12:26 Jeffy Chen
2017-09-27 20:27 ` Sean Paul
0 siblings, 1 reply; 4+ messages in thread
From: Jeffy Chen @ 2017-09-27 12:26 UTC (permalink / raw)
To: linux-arm-kernel
Free the drm_atomic_state allocated by drm_atomic_helper_suspend().
Fixes: 5a5873830972 ("drm/rockchip: Use atomic PM helpers")
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 76d63de5921d..80235b672deb 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -15,6 +15,7 @@
*/
#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
#include <drm/drm_crtc_helper.h>
#include <drm/drm_fb_helper.h>
#include <drm/drm_gem_cma_helper.h>
@@ -299,6 +300,7 @@ static int rockchip_drm_sys_resume(struct device *dev)
priv = drm->dev_private;
drm_atomic_helper_resume(drm, priv->state);
+ drm_atomic_state_put(priv->state);
rockchip_drm_fb_resume(drm);
drm_kms_helper_poll_enable(drm);
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] drm/rockchip: Fix memory leak in rockchip_drm_sys_resume()
2017-09-27 12:26 [PATCH] drm/rockchip: Fix memory leak in rockchip_drm_sys_resume() Jeffy Chen
@ 2017-09-27 20:27 ` Sean Paul
2017-09-28 3:16 ` jeffy
0 siblings, 1 reply; 4+ messages in thread
From: Sean Paul @ 2017-09-27 20:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 27, 2017 at 08:26:42PM +0800, Jeffy Chen wrote:
> Free the drm_atomic_state allocated by drm_atomic_helper_suspend().
>
> Fixes: 5a5873830972 ("drm/rockchip: Use atomic PM helpers")
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 76d63de5921d..80235b672deb 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -15,6 +15,7 @@
> */
>
> #include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_gem_cma_helper.h>
> @@ -299,6 +300,7 @@ static int rockchip_drm_sys_resume(struct device *dev)
>
> priv = drm->dev_private;
> drm_atomic_helper_resume(drm, priv->state);
> + drm_atomic_state_put(priv->state);
Won't this be freed for you eventually in commit_tail()?
Sean
> rockchip_drm_fb_resume(drm);
> drm_kms_helper_poll_enable(drm);
>
> --
> 2.11.0
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] drm/rockchip: Fix memory leak in rockchip_drm_sys_resume()
2017-09-27 20:27 ` Sean Paul
@ 2017-09-28 3:16 ` jeffy
2017-10-09 6:49 ` jeffy
0 siblings, 1 reply; 4+ messages in thread
From: jeffy @ 2017-09-28 3:16 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sean,
On 09/28/2017 04:27 AM, Sean Paul wrote:
>> >@@ -299,6 +300,7 @@ static int rockchip_drm_sys_resume(struct device *dev)
>> >
>> > priv = drm->dev_private;
>> > drm_atomic_helper_resume(drm, priv->state);
>> >+ drm_atomic_state_put(priv->state);
> Won't this be freed for you eventually in commit_tail()?
>
i think the drm_atomic_state_put in commit_tail is paired to the
drm_atomic_state_get in drm_atomic_helper_commit.
and the kmemleak shows(after a few suspend/resume):
unreferenced object 0xffffffc0ce0fa400 (size 256):
...
backtrace:
[<ffffffc00034a9a8>] __save_stack_trace+0x48/0x6c
[<ffffffc00034b088>] create_object+0x138/0x254
[<ffffffc0009d5504>] kmemleak_alloc+0x58/0x8c
[<ffffffc000345f78>] __kmalloc+0x1d4/0x2a0
[<ffffffc0006765bc>] usb_alloc_urb+0x30/0x60
[<ffffffbffc128598>] alloc_ctrl_urb+0x38/0x120 [btusb]
[<ffffffbffc129e7c>] btusb_send_frame+0x64/0xf8 [btusb]
checking the current code, i saw only i915/intel_display.c has this
drm_atomic_state_put for the state allocated by
drm_atomic_helper_suspend(), there're many drivers missing that(or maybe
they free it in some other way?)
> Sean
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] drm/rockchip: Fix memory leak in rockchip_drm_sys_resume()
2017-09-28 3:16 ` jeffy
@ 2017-10-09 6:49 ` jeffy
0 siblings, 0 replies; 4+ messages in thread
From: jeffy @ 2017-10-09 6:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sean,
On 09/28/2017 11:16 AM, jeffy wrote:
> Hi Sean,
>
> On 09/28/2017 04:27 AM, Sean Paul wrote:
>>> >@@ -299,6 +300,7 @@ static int rockchip_drm_sys_resume(struct device
>>> *dev)
>>> >
>>> > priv = drm->dev_private;
>>> > drm_atomic_helper_resume(drm, priv->state);
>>> >+ drm_atomic_state_put(priv->state);
>> Won't this be freed for you eventually in commit_tail()?
>>
> i think the drm_atomic_state_put in commit_tail is paired to the
> drm_atomic_state_get in drm_atomic_helper_commit.
>
> and the kmemleak shows(after a few suspend/resume):
>
> unreferenced object 0xffffffc0ce0fa400 (size 256):
> ...
> backtrace:
> [<ffffffc00034a9a8>] __save_stack_trace+0x48/0x6c
> [<ffffffc00034b088>] create_object+0x138/0x254
> [<ffffffc0009d5504>] kmemleak_alloc+0x58/0x8c
> [<ffffffc000345f78>] __kmalloc+0x1d4/0x2a0
> [<ffffffc0006765bc>] usb_alloc_urb+0x30/0x60
> [<ffffffbffc128598>] alloc_ctrl_urb+0x38/0x120 [btusb]
> [<ffffffbffc129e7c>] btusb_send_frame+0x64/0xf8 [btusb]
oops, the log is wrong...
it should be:
unreferenced object 0xffffffc0c75fe600 (size 256):
comm "bash", pid 168, jiffies 4294748032 (age 557.932s)
hex dump (first 32 bytes):
e0 4a 02 ce c0 ff ff ff 00 98 f2 c8 c0 ff ff ff .J..............
00 98 f2 c8 c0 ff ff ff 00 45 9f cb c0 ff ff ff .........E......
backtrace:
[<ffffff9008c76ba0>] kmemleak_alloc+0x58/0x8c
[<ffffff900828fe70>] __kmalloc+0x1dc/0x2d4
[<ffffff9008647698>] drm_atomic_state_init+0xb8/0x114
[<ffffff9008649ed8>] drm_atomic_state_alloc+0x70/0xa4
[<ffffff90086209f8>] drm_atomic_helper_duplicate_state+0x94/0x210
[<ffffff9008621184>] drm_atomic_helper_suspend+0xb4/0x160
[<ffffff9008666f18>] rockchip_drm_sys_suspend+0x5c/0xa0
[<ffffff9008697f18>] platform_pm_suspend+0x58/0x74
[<ffffff90086a7f98>] dpm_run_callback+0x13c/0x2f0
[<ffffff90086a9064>] __device_suspend+0x28c/0x3fc
[<ffffff90086ab9ec>] dpm_suspend+0x1cc/0x50c
[<ffffff90086ac5a4>] dpm_suspend_start+0x8c/0xa4
[<ffffff90081226a0>] suspend_devices_and_enter+0x12c/0xd84
[<ffffff9008123d30>] pm_suspend+0xa38/0xb98
[<ffffff9008120f94>] state_store+0xe0/0x108
[<ffffff9008c63e7c>] kobj_attr_store+0x48/0x58
since it's allocated in the drm_atomic_helper_suspend(), maybe it would
make more sense to unref it in drm_atomic_helper_resume(), i'll send a
v2 patch for it :)
>
>
>
>
> checking the current code, i saw only i915/intel_display.c has this
> drm_atomic_state_put for the state allocated by
> drm_atomic_helper_suspend(), there're many drivers missing that(or maybe
> they free it in some other way?)
>
>> Sean
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-09 6:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-27 12:26 [PATCH] drm/rockchip: Fix memory leak in rockchip_drm_sys_resume() Jeffy Chen
2017-09-27 20:27 ` Sean Paul
2017-09-28 3:16 ` jeffy
2017-10-09 6:49 ` jeffy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).