[CPUFREQ] speedstep-centrino: fix SMP memory leak
Venkatesh Pallipadi made me aware of a memory leak in speedstep-centrino:
centrino_model is allocated for all CPUs. There were two possibilities: either
make the centrino_model data per-CPU, or to share it across CPUs. I chose the
former variant, as ACPI data may be broken and/or different for multiple CPUs.
Additionally, I made centrino_cpu per-CPU, and removed a check whether setting
allowed_cpus to policy->cpus resulted in smp_processor_id() == policy->cpu:
if policy->cpus has more than one bit set, this check will fail very often.
Signed-off-by: Dominik Brodowski <linux@brodo.de>
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c linux/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c 2004-11-27 12:06:14.000000000 +0100
+++ linux/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c 2004-11-27 13:06:32.503538216 +0100
@@ -71,8 +71,8 @@
static int centrino_verify_cpu_id(const struct cpuinfo_x86 *c, const struct cpu_id *x);
/* Operating points for current CPU */
-static struct cpu_model *centrino_model;
-static const struct cpu_id *centrino_cpu;
+static struct cpu_model *centrino_model[NR_CPUS];
+static const struct cpu_id *centrino_cpu[NR_CPUS];
#ifdef CONFIG_X86_SPEEDSTEP_CENTRINO_TABLE
@@ -255,7 +255,7 @@
return -ENOENT;
}
- centrino_model = model;
+ centrino_model[policy->cpu] = model;
dprintk("found \"%s\": max frequency: %dkHz\n",
model->model_name, model->max_freq);
@@ -277,7 +277,7 @@
}
/* To be called only after centrino_model is initialized */
-static unsigned extract_clock(unsigned msr)
+static unsigned extract_clock(unsigned msr, unsigned int cpu)
{
int i;
@@ -286,20 +286,20 @@
* for centrino, as some DSDTs are buggy.
* Ideally, this can be done using the acpi_data structure.
*/
- if ((centrino_cpu == &cpu_ids[CPU_BANIAS]) ||
- (centrino_cpu == &cpu_ids[CPU_DOTHAN_A1]) ||
- (centrino_cpu == &cpu_ids[CPU_DOTHAN_B0])) {
+ if ((centrino_cpu[cpu] == &cpu_ids[CPU_BANIAS]) ||
+ (centrino_cpu[cpu] == &cpu_ids[CPU_DOTHAN_A1]) ||
+ (centrino_cpu[cpu] == &cpu_ids[CPU_DOTHAN_B0])) {
msr = (msr >> 8) & 0xff;
return msr * 100000;
}
- if ((!centrino_model) || (!centrino_model->op_points))
+ if ((!centrino_model[cpu]) || (!centrino_model[cpu]->op_points))
return 0;
msr &= 0xffff;
- for (i=0;centrino_model->op_points[i].frequency != CPUFREQ_TABLE_END; i++) {
- if (msr == centrino_model->op_points[i].index)
- return centrino_model->op_points[i].frequency;
+ for (i=0;centrino_model[cpu]->op_points[i].frequency != CPUFREQ_TABLE_END; i++) {
+ if (msr == centrino_model[cpu]->op_points[i].index)
+ return centrino_model[cpu]->op_points[i].frequency;
}
return 0;
}
@@ -317,7 +317,7 @@
rdmsr(MSR_IA32_PERF_STATUS, l, h);
set_cpus_allowed(current, saved_mask);
- return extract_clock(l);
+ return extract_clock(l, cpu);
}
@@ -339,6 +339,7 @@
struct acpi_object_list arg_list = {1, &arg0};
unsigned long cur_freq;
int result = 0, i;
+ unsigned int cpu = policy->cpu;
/* _PDC settings */
arg0.buffer.length = 12;
@@ -350,7 +351,7 @@
p.pdc = &arg_list;
/* register with ACPI core */
- if (acpi_processor_register_performance(&p, policy->cpu)) {
+ if (acpi_processor_register_performance(&p, cpu)) {
printk(KERN_INFO PFX "obtaining ACPI data failed\n");
return -EIO;
}
@@ -392,49 +393,49 @@
}
}
- centrino_model = kmalloc(sizeof(struct cpu_model), GFP_KERNEL);
- if (!centrino_model) {
+ centrino_model[cpu] = kmalloc(sizeof(struct cpu_model), GFP_KERNEL);
+ if (!centrino_model[cpu]) {
result = -ENOMEM;
goto err_unreg;
}
- memset(centrino_model, 0, sizeof(struct cpu_model));
+ memset(centrino_model[cpu], 0, sizeof(struct cpu_model));
- centrino_model->model_name=NULL;
- centrino_model->max_freq = p.states[0].core_frequency * 1000;
- centrino_model->op_points = kmalloc(sizeof(struct cpufreq_frequency_table) *
+ centrino_model[cpu]->model_name=NULL;
+ centrino_model[cpu]->max_freq = p.states[0].core_frequency * 1000;
+ centrino_model[cpu]->op_points = kmalloc(sizeof(struct cpufreq_frequency_table) *
(p.state_count + 1), GFP_KERNEL);
- if (!centrino_model->op_points) {
+ if (!centrino_model[cpu]->op_points) {
result = -ENOMEM;
goto err_kfree;
}
for (i=0; i<p.state_count; i++) {
- centrino_model->op_points[i].index = p.states[i].control;
- centrino_model->op_points[i].frequency = p.states[i].core_frequency * 1000;
+ centrino_model[cpu]->op_points[i].index = p.states[i].control;
+ centrino_model[cpu]->op_points[i].frequency = p.states[i].core_frequency * 1000;
dprintk("adding state %i with frequency %u and control value %04x\n",
- i, centrino_model->op_points[i].frequency, centrino_model->op_points[i].index);
+ i, centrino_model[cpu]->op_points[i].frequency, centrino_model[cpu]->op_points[i].index);
}
- centrino_model->op_points[p.state_count].frequency = CPUFREQ_TABLE_END;
+ centrino_model[cpu]->op_points[p.state_count].frequency = CPUFREQ_TABLE_END;
- cur_freq = get_cur_freq(policy->cpu);
+ cur_freq = get_cur_freq(cpu);
for (i=0; i<p.state_count; i++) {
if (!p.states[i].core_frequency) {
dprintk("skipping state %u\n", i);
- centrino_model->op_points[i].frequency = CPUFREQ_ENTRY_INVALID;
+ centrino_model[cpu]->op_points[i].frequency = CPUFREQ_ENTRY_INVALID;
continue;
}
- if (extract_clock(centrino_model->op_points[i].index) !=
- (centrino_model->op_points[i].frequency)) {
+ if (extract_clock(centrino_model[cpu]->op_points[i].index, cpu) !=
+ (centrino_model[cpu]->op_points[i].frequency)) {
dprintk("Invalid encoded frequency (%u vs. %u)\n",
- extract_clock(centrino_model->op_points[i].index),
- centrino_model->op_points[i].frequency);
+ extract_clock(centrino_model[cpu]->op_points[i].index, cpu),
+ centrino_model[cpu]->op_points[i].frequency);
result = -EINVAL;
goto err_kfree_all;
}
- if (cur_freq == centrino_model->op_points[i].frequency)
+ if (cur_freq == centrino_model[cpu]->op_points[i].frequency)
p.state = i;
}
@@ -444,11 +445,11 @@
return 0;
err_kfree_all:
- kfree(centrino_model->op_points);
+ kfree(centrino_model[cpu]->op_points);
err_kfree:
- kfree(centrino_model);
+ kfree(centrino_model[cpu]);
err_unreg:
- acpi_processor_unregister_performance(&p, policy->cpu);
+ acpi_processor_unregister_performance(&p, cpu);
printk(KERN_INFO PFX "invalid ACPI data\n");
return (result);
}
@@ -473,13 +474,13 @@
break;
if (i != N_IDS)
- centrino_cpu = &cpu_ids[i];
+ centrino_cpu[policy->cpu] = &cpu_ids[i];
if (centrino_cpu_init_acpi(policy)) {
if (policy->cpu != 0)
return -ENODEV;
- if (!centrino_cpu) {
+ if (!centrino_cpu[policy->cpu]) {
printk(KERN_INFO PFX "found unsupported CPU with "
"Enhanced SpeedStep: send /proc/cpuinfo to "
MAINTAINER "\n");
@@ -516,32 +517,34 @@
dprintk("centrino_cpu_init: cur=%dkHz\n", policy->cur);
- ret = cpufreq_frequency_table_cpuinfo(policy, centrino_model->op_points);
+ ret = cpufreq_frequency_table_cpuinfo(policy, centrino_model[policy->cpu]->op_points);
if (ret)
return (ret);
- cpufreq_frequency_table_get_attr(centrino_model->op_points, policy->cpu);
+ cpufreq_frequency_table_get_attr(centrino_model[policy->cpu]->op_points, policy->cpu);
return 0;
}
static int centrino_cpu_exit(struct cpufreq_policy *policy)
{
- if (!centrino_model)
+ unsigned int cpu = policy->cpu;
+
+ if (!centrino_model[cpu])
return -ENODEV;
- cpufreq_frequency_table_put_attr(policy->cpu);
+ cpufreq_frequency_table_put_attr(cpu);
#ifdef CONFIG_X86_SPEEDSTEP_CENTRINO_ACPI
- if (!centrino_model->model_name) {
+ if (!centrino_model[cpu]->model_name) {
dprintk("unregistering and freeing ACPI data\n");
- acpi_processor_unregister_performance(&p, policy->cpu);
- kfree(centrino_model->op_points);
- kfree(centrino_model);
+ acpi_processor_unregister_performance(&p, cpu);
+ kfree(centrino_model[cpu]->op_points);
+ kfree(centrino_model[cpu]);
}
#endif
- centrino_model = NULL;
+ centrino_model[cpu] = NULL;
return 0;
}
@@ -555,7 +558,7 @@
*/
static int centrino_verify (struct cpufreq_policy *policy)
{
- return cpufreq_frequency_table_verify(policy, centrino_model->op_points);
+ return cpufreq_frequency_table_verify(policy, centrino_model[policy->cpu]->op_points);
}
/**
@@ -571,12 +574,12 @@
unsigned int relation)
{
unsigned int newstate = 0;
- unsigned int msr, oldmsr, h;
+ unsigned int msr, oldmsr, h, cpu = policy->cpu;
struct cpufreq_freqs freqs;
cpumask_t saved_mask;
int retval;
- if (centrino_model == NULL)
+ if (centrino_model[cpu] == NULL)
return -ENODEV;
/*
@@ -585,18 +588,14 @@
*/
saved_mask = current->cpus_allowed;
set_cpus_allowed(current, policy->cpus);
- if (smp_processor_id() != policy->cpu) {
- dprintk("couldn't limit to CPUs in this domain\n");
- return(-EAGAIN);
- }
- if (cpufreq_frequency_table_target(policy, centrino_model->op_points, target_freq,
+ if (cpufreq_frequency_table_target(policy, centrino_model[cpu]->op_points, target_freq,
relation, &newstate)) {
retval = -EINVAL;
goto migrate_end;
}
- msr = centrino_model->op_points[newstate].index;
+ msr = centrino_model[cpu]->op_points[newstate].index;
rdmsr(MSR_IA32_PERF_CTL, oldmsr, h);
if (msr == (oldmsr & 0xffff)) {
@@ -605,9 +604,9 @@
goto migrate_end;
}
- freqs.cpu = policy->cpu;
- freqs.old = extract_clock(oldmsr);
- freqs.new = extract_clock(msr);
+ freqs.cpu = cpu;
+ freqs.old = extract_clock(oldmsr, cpu);
+ freqs.new = extract_clock(msr, cpu);
dprintk("target=%dkHz old=%d new=%d msr=%04x\n",
target_freq, freqs.old, freqs.new, msr);