[Bug 206695] New: kmemleak reports leaks in drivers/macintosh/windfarm

https://bugzilla.kernel.org/show_bug.cgi?id=206695

            Bug ID: 206695
           Summary: kmemleak reports leaks in drivers/macintosh/windfarm
           Product: Platform Specific/Hardware
           Version: 2.5
    Kernel Version: 5.6-rc3
          Hardware: PPC-64
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: PPC-64
          Assignee: platform_ppc-64@kernel-bugs.osdl.org
          Reporter: erhard_f@mailbox.org
        Regression: No

Created attachment 287687
  --> https://bugzilla.kernel.org/attachment.cgi?id=287687&action=edit
kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2)

kmemleak reports leaks from the windfarm module of my PowerMac G5 11,2:

[...]
unreferenced object 0xc00000047081f840 (size 32):
  comm "kwindfarm", pid 203, jiffies 4294880630 (age 5552.877s)
  hex dump (first 32 bytes):
    c8 06 02 7f ff 02 ff 01 fb bf 00 41 00 20 00 00  ...........A. ..
    00 07 89 37 00 a0 00 00 00 00 00 00 00 00 00 00  ...7............
  backtrace:
    [<0000000083f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0
[windfarm_smu_sat]
    [<000000003010fcb7>] .pm112_wf_notify+0x104c/0x13bc [windfarm_pm112]
    [<00000000b958b2dd>] .notifier_call_chain+0xa8/0x180
    [<0000000070490868>] .blocking_notifier_call_chain+0x64/0x90
    [<00000000131d8149>] .wf_thread_func+0x114/0x1a0
    [<000000000d54838d>] .kthread+0x13c/0x190
    [<00000000669b72bc>] .ret_from_kernel_thread+0x58/0x64
unreferenced object 0xc0000004737089f0 (size 16):
  comm "kwindfarm", pid 203, jiffies 4294880879 (age 5552.050s)
  hex dump (first 16 bytes):
    c4 04 01 7f 22 11 e0 e6 ff 55 7b 12 ec 11 00 00  ...."....U{.....
  backtrace:
    [<0000000083f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0
[windfarm_smu_sat]
    [<00000000b94ef7e1>] .pm112_wf_notify+0x1294/0x13bc [windfarm_pm112]
    [<00000000b958b2dd>] .notifier_call_chain+0xa8/0x180
    [<0000000070490868>] .blocking_notifier_call_chain+0x64/0x90
    [<00000000131d8149>] .wf_thread_func+0x114/0x1a0
    [<000000000d54838d>] .kthread+0x13c/0x190
    [<00000000669b72bc>] .ret_from_kernel_thread+0x58/0x64
unreferenced object 0xc00000047081fdc0 (size 32):
  comm "kwindfarm", pid 203, jiffies 4294881067 (age 5551.427s)
  hex dump (first 32 bytes):
    c9 06 02 7f ff 02 ff 01 fb bf 00 41 00 20 00 00  ...........A. ..
    00 07 89 37 00 a0 00 00 00 00 00 00 00 00 00 00  ...7............
  backtrace:
    [<0000000083f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0
[windfarm_smu_sat]
    [<000000003010fcb7>] .pm112_wf_notify+0x104c/0x13bc [windfarm_pm112]
    [<00000000b958b2dd>] .notifier_call_chain+0xa8/0x180
    [<0000000070490868>] .blocking_notifier_call_chain+0x64/0x90
    [<00000000131d8149>] .wf_thread_func+0x114/0x1a0
    [<000000000d54838d>] .kthread+0x13c/0x190
    [<00000000669b72bc>] .ret_from_kernel_thread+0x58/0x64
unreferenced object 0xc000000473708b60 (size 16):
  comm "kwindfarm", pid 203, jiffies 4294881320 (age 5550.587s)
  hex dump (first 16 bytes):
    c5 04 01 7f 22 11 e0 e6 ff 55 7b 12 ec 11 00 00  ...."....U{.....
  backtrace:
    [<0000000083f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0
[windfarm_smu_sat]
    [<00000000b94ef7e1>] .pm112_wf_notify+0x1294/0x13bc [windfarm_pm112]
    [<00000000b958b2dd>] .notifier_call_chain+0xa8/0x180
    [<0000000070490868>] .blocking_notifier_call_chain+0x64/0x90
    [<00000000131d8149>] .wf_thread_func+0x114/0x1a0
    [<000000000d54838d>] .kthread+0x13c/0x190
    [<00000000669b72bc>] .ret_from_kernel_thread+0x58/0x64

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=206695

--- Comment #1 from Erhard F. (erhard_f@mailbox.org) ---

Created attachment 287689
  --> https://bugzilla.kernel.org/attachment.cgi?id=287689&action=edit
dmesg (kernel 5.6-rc3, PowerMac G5 11,2)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=206695

--- Comment #2 from Erhard F. (erhard_f@mailbox.org) ---

Created attachment 287691
  --> https://bugzilla.kernel.org/attachment.cgi?id=287691&action=edit
kernel .config (kernel 5.6-rc3, PowerMac G5 11,2)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

Can you try this patch?

diff --git a/drivers/macintosh/windfarm_pm112.c b/drivers/macintosh/windfarm_pm112.c
index 4150301a89a5..a16f43a1def9 100644
--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c

@@ -125,7 +125,7 @@ static int create_cpu_loop(int cpu)
 {
 	int chip = cpu / 2;
 	int core = cpu & 1;
-	struct smu_sdbp_header *hdr;
+	struct smu_sdbp_header *hdr, *hdr2;
 	struct smu_sdbp_cpupiddata *piddata;
 	struct wf_cpu_pid_param pid;
 	struct wf_control *main_fan = cpu_fans[0];

@@ -141,9 +141,9 @@ static int create_cpu_loop(int cpu)
 	piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];
 
 	/* Get FVT params to get Tmax; if not found, assume default */
-	hdr = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
-	if (hdr) {
-		struct smu_sdbp_fvt *fvt = (struct smu_sdbp_fvt *)&hdr[1];
+	hdr2 = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
+	if (hdr2) {
+		struct smu_sdbp_fvt *fvt = (struct smu_sdbp_fvt *)&hdr2[1];
 		tmax = fvt->maxtemp << 16;
 	} else
 		tmax = 95 << 16;	/* default to 95 degrees C */

@@ -174,6 +174,10 @@ static int create_cpu_loop(int cpu)
 		pid.min = fmin;
 
 	wf_cpu_pid_init(&cpu_pid[cpu], &pid);
+
+	kfree(hdr);
+	kfree(hdr2);
+
 	return 0;
 }
 

https://bugzilla.kernel.org/show_bug.cgi?id=206695

--- Comment #3 from mpe@ellerman.id.au ---

Can you try this patch?

diff --git a/drivers/macintosh/windfarm_pm112.c

b/drivers/macintosh/windfarm_pm112.c
index 4150301a89a5..a16f43a1def9 100644

--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c

@@ -125,7 +125,7 @@ static int create_cpu_loop(int cpu)
 {
        int chip = cpu / 2;
        int core = cpu & 1;
-       struct smu_sdbp_header *hdr;
+       struct smu_sdbp_header *hdr, *hdr2;
        struct smu_sdbp_cpupiddata *piddata;
        struct wf_cpu_pid_param pid;
        struct wf_control *main_fan = cpu_fans[0];

@@ -141,9 +141,9 @@ static int create_cpu_loop(int cpu)
        piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];

        /* Get FVT params to get Tmax; if not found, assume default */
-       hdr = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
-       if (hdr) {
-               struct smu_sdbp_fvt *fvt = (struct smu_sdbp_fvt *)&hdr[1];
+       hdr2 = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
+       if (hdr2) {
+               struct smu_sdbp_fvt *fvt = (struct smu_sdbp_fvt *)&hdr2[1];
                tmax = fvt->maxtemp << 16;
        } else
                tmax = 95 << 16;        /* default to 95 degrees C */

@@ -174,6 +174,10 @@ static int create_cpu_loop(int cpu)
                pid.min = fmin;

        wf_cpu_pid_init(&cpu_pid[cpu], &pid);
+
+       kfree(hdr);
+       kfree(hdr2);
+
        return 0;
 }

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=206695

--- Comment #4 from Erhard F. (erhard_f@mailbox.org) ---

(In reply to mpe from comment #3)

Can you try this patch?

Applied your patch on top of 5.6-rc4 +
https://patchwork.ozlabs.org/patch/1248350/ and let the G5 do a few hours
compiling.

Only getting those nice memleaks from bug #206203 but no windfarm_pm112 memleak
any longer. So your patch works well it seems. Thanks!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

bugzilla-daemon@bugzilla.kernel.org writes:

Thanks.

Can you try this one instead, it changes the order of operations to make
the code flow a bit nicer.

cheers

diff --git a/drivers/macintosh/windfarm_pm112.c b/drivers/macintosh/windfarm_pm112.c
index 4150301a89a5..e8377ce0a95a 100644
--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c

@@ -132,14 +132,6 @@ static int create_cpu_loop(int cpu)
 	s32 tmax;
 	int fmin;
 
-	/* Get PID params from the appropriate SAT */
-	hdr = smu_sat_get_sdb_partition(chip, 0xC8 + core, NULL);
-	if (hdr == NULL) {
-		printk(KERN_WARNING"windfarm: can't get CPU PID fan config\n");
-		return -EINVAL;
-	}
-	piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];
-
 	/* Get FVT params to get Tmax; if not found, assume default */
 	hdr = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
 	if (hdr) {

@@ -152,6 +144,16 @@ static int create_cpu_loop(int cpu)
 	if (tmax < cpu_all_tmax)
 		cpu_all_tmax = tmax;
 
+	kfree(hdr);
+
+	/* Get PID params from the appropriate SAT */
+	hdr = smu_sat_get_sdb_partition(chip, 0xC8 + core, NULL);
+	if (hdr == NULL) {
+		printk(KERN_WARNING"windfarm: can't get CPU PID fan config\n");
+		return -EINVAL;
+	}
+	piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];
+
 	/*
 	 * Darwin has a minimum fan speed of 1000 rpm for the 4-way and
 	 * 515 for the 2-way.  That appears to be overkill, so for now,

@@ -174,6 +176,9 @@ static int create_cpu_loop(int cpu)
 		pid.min = fmin;
 
 	wf_cpu_pid_init(&cpu_pid[cpu], &pid);
+
+	kfree(hdr);
+
 	return 0;
 }
 

https://bugzilla.kernel.org/show_bug.cgi?id=206695

--- Comment #5 from mpe@ellerman.id.au ---

bugzilla-daemon@bugzilla.kernel.org writes:

Thanks.

Can you try this one instead, it changes the order of operations to make
the code flow a bit nicer.

cheers

diff --git a/drivers/macintosh/windfarm_pm112.c

b/drivers/macintosh/windfarm_pm112.c
index 4150301a89a5..e8377ce0a95a 100644

--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c

@@ -132,14 +132,6 @@ static int create_cpu_loop(int cpu)
        s32 tmax;
        int fmin;

-       /* Get PID params from the appropriate SAT */
-       hdr = smu_sat_get_sdb_partition(chip, 0xC8 + core, NULL);
-       if (hdr == NULL) {
-               printk(KERN_WARNING"windfarm: can't get CPU PID fan config\n");
-               return -EINVAL;
-       }
-       piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];
-
        /* Get FVT params to get Tmax; if not found, assume default */
        hdr = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
        if (hdr) {

@@ -152,6 +144,16 @@ static int create_cpu_loop(int cpu)
        if (tmax < cpu_all_tmax)
                cpu_all_tmax = tmax;

+       kfree(hdr);
+
+       /* Get PID params from the appropriate SAT */
+       hdr = smu_sat_get_sdb_partition(chip, 0xC8 + core, NULL);
+       if (hdr == NULL) {
+               printk(KERN_WARNING"windfarm: can't get CPU PID fan config\n");
+               return -EINVAL;
+       }
+       piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];
+
        /*
         * Darwin has a minimum fan speed of 1000 rpm for the 4-way and
         * 515 for the 2-way.  That appears to be overkill, so for now,

@@ -174,6 +176,9 @@ static int create_cpu_loop(int cpu)
                pid.min = fmin;

        wf_cpu_pid_init(&cpu_pid[cpu], &pid);
+
+       kfree(hdr);
+
        return 0;
 }

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=206695

--- Comment #6 from Erhard F. (erhard_f@mailbox.org) ---

(In reply to mpe from comment #5)

Can you try this one instead, it changes the order of operations to make
the code flow a bit nicer.

2nd patch works equally well.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=206695

Michael Ellerman (michael@ellerman.id.au) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |michael@ellerman.id.au

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=206695

Michael Ellerman (michael@ellerman.id.au) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |CODE_FIX

--- Comment #7 from Michael Ellerman (michael@ellerman.id.au) ---

Patch posted:

https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200423060038.3308530-1-mpe@ellerman.id.au/

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=206695

Dennis Clarke (dclarke@blastwave.org) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dclarke@blastwave.org

--- Comment #8 from Dennis Clarke (dclarke@blastwave.org) ---

123456789+123456789+123456789+123456789+123456789+123456789+123456789+
I will apply the patch and try with Linux 5.7-rc2 and post any results
seen.

Also this does close off : https://bugzilla.kernel.org/show_bug.cgi?id=199471

I see Wolfram Sang has commented there. OKay ... good stuff.

Dennis Clarke

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=206695

--- Comment #9 from Dennis Clarke (dclarke@blastwave.org) ---

I see this has not gone upstream to 5.7-rc3 and thus I am applying
it manually and building now.  

Shall report on the kmem leak shortly. I hope. 

Dennis

-- 
You are receiving this mail because:
You are watching the assignee of the bug.