Hi Leo,

Leo Yan [off-list ref] hat am 13. August 2017 um 05:50 geschrieben:


Hi Stefan,

On Sat, Aug 12, 2017 at 03:19:33PM +0200, Stefan Wahren wrote:

quoted

Hi,

if i additionally enable kmemleak (on top of multi_v7_defconfig) on RPi 3 (4 cores) with 4.13-rc4, i get the following output from kmemleak:

unreferenced object 0xede0dc00 (size 1024):
  comm "swapper/0", pid 1, jiffies 4294937431 (age 744.510s)
  hex dump (first 32 bytes):
    94 9e 0b c1 00 00 00 00 00 00 00 00 00 00 00 00  ................
    57 46 49 00 00 00 00 00 00 00 00 00 00 00 00 00  WFI.............
  backtrace:
    [<c1295f04>] arm_idle_init+0x44/0x1ac
    [<c0301e6c>] do_one_initcall+0x3c/0x16c
    [<c1200d70>] kernel_init_freeable+0x110/0x1d0
    [<c0cb3624>] kernel_init+0x8/0x114
    [<c0307a98>] ret_from_fork+0x14/0x3c
    [<ffffffff>] 0xffffffff

If i revert the last commit in cpuidle-arm d50a7d8acd78 ("ARM: cpuidle: Support asymmetric idle definition") then kmemleak stays calm.

I cannot reproduce the error at my side, I also tried to disable idle
states but cannot trigger the failure. So first thing is to know the
reason for registeration failure and finally introduce the memory
leaking. Could you point out the dts you are using for idle states
binding?

thanks for you quick response. Sorry, i forgot to mention that arch/arm64/boot/dts/broadcom/bcm2837.dtsi doesn't contain any idle states. So the error path of dt_init_idle_driver() is expected. If i get it right, the clean up loop after out_fail only handles registered drivers (better label name or a comment would be helpful). So combined with your fix the following fixes the memleak for me (didn't test the clean up loop):

diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
index 7080c38..827ab25 100644
--- a/drivers/cpuidle/cpuidle-arm.c
+++ b/drivers/cpuidle/cpuidle-arm.c

@@ -104,12 +104,14 @@ static int __init arm_idle_init(void)
 		ret = dt_init_idle_driver(drv, arm_idle_state_match, 1);
 		if (ret <= 0) {
 			ret = ret ? : -ENODEV;
+			kfree(drv);
 			goto out_fail;
 		}
 
 		ret = cpuidle_register_driver(drv);
 		if (ret) {
 			pr_err("Failed to register cpuidle driver\n");
+			kfree(drv);
 			goto out_fail;
 		}
 

@@ -152,11 +154,13 @@ static int __init arm_idle_init(void)
 out_fail:
 	while (--cpu >= 0) {
 		dev = per_cpu(cpuidle_devices, cpu);
-		cpuidle_unregister_device(dev);
-		kfree(dev);
-		drv = cpuidle_get_driver();
+		drv = cpuidle_get_cpu_driver(dev);
+
 		cpuidle_unregister_driver(drv);
 		kfree(drv);
+
+		cpuidle_unregister_device(dev);
+		kfree(dev);
 	}
 
 	return ret;