[PATCH 3/8] cpufreq: powerenv: Fix memory leak

Subsystems: cpu frequency scaling framework, the rest

STALE3736d

6 messages, 2 authors, 2016-06-02 · open the first message on its own page

[PATCH 3/8] cpufreq: powerenv: Fix memory leak

From: Viresh Kumar <viresh.kumar@linaro.org>
Date: 2016-06-01 10:34:50

The policy is copied (unnecessarily) and is never freed. Fix it by just
getting a reference to the existing policy structure and putting it
back.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/powernv-cpufreq.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index 54c45368e3f1..96bb4acd366e 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -756,15 +756,18 @@ void powernv_cpufreq_work_fn(struct work_struct *work)
 
 	chip->restore = false;
 	for_each_cpu(cpu, &mask) {
+		struct cpufreq_policy *policy = cpufreq_cpu_get(cpu)
 		int index;
-		struct cpufreq_policy policy;
 
-		cpufreq_get_policy(&policy, cpu);
-		cpufreq_frequency_table_target(&policy, policy.freq_table,
-					       policy.cur,
+		if (!policy)
+			continue;
+
+		cpufreq_frequency_table_target(policy, policy->freq_table,
+					       policy->cur,
 					       CPUFREQ_RELATION_C, &index);
-		powernv_cpufreq_target_index(&policy, index);
-		cpumask_andnot(&mask, &mask, policy.cpus);
+		powernv_cpufreq_target_index(policy, index);
+		cpumask_andnot(&mask, &mask, policy->cpus);
+		cpufreq_cpu_put(policy);
 	}
 out:
 	put_online_cpus();
-- 
2.7.1.410.g6faf27b

Re: [PATCH 3/8] cpufreq: powerenv: Fix memory leak

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-06-02 11:08:13

On Wed, 2016-06-01 at 16:04 +0530, Viresh Kumar wrote:
The policy is copied (unnecessarily) and is never freed. Fix it by just
getting a reference to the existing policy structure and putting it
back.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
When was it broken, always?

Cc: stable ?

cheers

Re: [PATCH 3/8] cpufreq: powerenv: Fix memory leak

From: Viresh Kumar <viresh.kumar@linaro.org>
Date: 2016-06-02 11:22:10

On 02-06-16, 21:08, Michael Ellerman wrote:
On Wed, 2016-06-01 at 16:04 +0530, Viresh Kumar wrote:
quoted
The policy is copied (unnecessarily) and is never freed. Fix it by just
getting a reference to the existing policy structure and putting it
back.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
When was it broken, always?

Cc: stable ?
Its a small memory leak and its not that we will fail on something. So
didn't bother to add those details, but in case they are required:

Cc: <redacted> # 4.3+
Fixes: 227942809b52 ("cpufreq: powernv: Restore cpu frequency to policy->cur on unthrottling")

-- 
viresh

Re: [PATCH 3/8] cpufreq: powerenv: Fix memory leak

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-06-02 11:37:56

On Thu, 2016-06-02 at 16:52 +0530, Viresh Kumar wrote:
On 02-06-16, 21:08, Michael Ellerman wrote:
quoted
On Wed, 2016-06-01 at 16:04 +0530, Viresh Kumar wrote:
quoted
The policy is copied (unnecessarily) and is never freed. Fix it by just
getting a reference to the existing policy structure and putting it
back.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
When was it broken, always?

Cc: stable ?
Its a small memory leak and its not that we will fail on something. So
didn't bother to add those details, but in case they are required:

Cc: <redacted> # 4.3+
Fixes: 227942809b52 ("cpufreq: powernv: Restore cpu frequency to policy->cur on unthrottling")
OK. I can't actually see where the copy is?

But if we are leaking even a small amount of memory in a loop like that, in a
function that's run semi-regularly, then it's going to add up eventually.

cheers

Re: [PATCH 3/8] cpufreq: powerenv: Fix memory leak

From: Viresh Kumar <viresh.kumar@linaro.org>
Date: 2016-06-02 11:43:24

On 02-06-16, 21:37, Michael Ellerman wrote:
On Thu, 2016-06-02 at 16:52 +0530, Viresh Kumar wrote:
quoted
On 02-06-16, 21:08, Michael Ellerman wrote:
quoted
On Wed, 2016-06-01 at 16:04 +0530, Viresh Kumar wrote:
quoted
The policy is copied (unnecessarily) and is never freed. Fix it by just
getting a reference to the existing policy structure and putting it
back.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
When was it broken, always?

Cc: stable ?
Its a small memory leak and its not that we will fail on something. So
didn't bother to add those details, but in case they are required:

Cc: <redacted> # 4.3+
Fixes: 227942809b52 ("cpufreq: powernv: Restore cpu frequency to policy->cur on unthrottling")
OK. I can't actually see where the copy is?

But if we are leaking even a small amount of memory in a loop like that, in a
function that's run semi-regularly, then it's going to add up eventually.
Urg, it wasn't a memory leak actually. I misread.

I somehow thought that cpufreq_get_policy() is also allocating memory
for the policy, but it just memcpy's it into the callers buffer. So,
that's not a problem really.

This patch should be just dropped. Sorry for the noise.

-- 
viresh

Re: [PATCH 3/8] cpufreq: powerenv: Fix memory leak

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-06-02 12:08:31

On Thu, 2016-06-02 at 17:13 +0530, Viresh Kumar wrote:
On 02-06-16, 21:37, Michael Ellerman wrote:
quoted
On Thu, 2016-06-02 at 16:52 +0530, Viresh Kumar wrote:
quoted
On 02-06-16, 21:08, Michael Ellerman wrote:
quoted
On Wed, 2016-06-01 at 16:04 +0530, Viresh Kumar wrote:
quoted
The policy is copied (unnecessarily) and is never freed. Fix it by just
getting a reference to the existing policy structure and putting it
back.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
When was it broken, always?

Cc: stable ?
Its a small memory leak and its not that we will fail on something. So
didn't bother to add those details, but in case they are required:

Cc: <redacted> # 4.3+
Fixes: 227942809b52 ("cpufreq: powernv: Restore cpu frequency to policy->cur on unthrottling")
OK. I can't actually see where the copy is?

But if we are leaking even a small amount of memory in a loop like that, in a
function that's run semi-regularly, then it's going to add up eventually.
Urg, it wasn't a memory leak actually. I misread.

I somehow thought that cpufreq_get_policy() is also allocating memory
for the policy, but it just memcpy's it into the callers buffer. So,
that's not a problem really.

This patch should be just dropped. Sorry for the noise.
OK, no worries.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help