Messages in this thread Patch in this message | | | From | Rusty Russell <> | Subject | Re: 2.6.29-rc1 does not boot | Date | Wed, 14 Jan 2009 11:46:03 +1030 |
| |
On Monday 12 January 2009 21:56:08 Ingo Molnar wrote: > work_on_cpu() needs to be reworked to be more generally usable.
Yes, but how? If we don't take the hotplug lock, we are relying on the callers to "prove" the cpu can't go away. But we've already shown that we can't find this by inspection :(
> The commit introduces work_on_cpu() use into the cpufreq code, > but that is subtly problematic from a lock hierarchy POV: the > hotplug-cpu lock is an highlevel lock that is taken before > lowlevel locks, and in this codepath we are called with the > policy lock taken.
The easiest thing to do in this case is to use smp_call_function_single(), since the thing we want to do is just rdmsr and wrmsr.
How about this instead of reverting? Rusty.
fix: use smp_call_function_single() not work_on_cpu in acpi-cpufreq.c
get_measured_perf() gets called through twisty chains, and work_on_cpu() has issues being called here as it takes the cpu hotplug lock.
So we just use smp_call_function_single() which should work fine for this case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c @@ -255,7 +255,7 @@ struct perf_cur { }; -static long read_measured_perf_ctrs(void *_cur) +static void read_measured_perf_ctrs(void *_cur) { struct perf_cur *cur = _cur; @@ -264,8 +264,6 @@ static long read_measured_perf_ctrs(void wrmsr(MSR_IA32_APERF, 0, 0); wrmsr(MSR_IA32_MPERF, 0, 0); - - return 0; } /* @@ -288,7 +286,7 @@ static unsigned int get_measured_perf(st unsigned int perf_percent; unsigned int retval; - if (!work_on_cpu(cpu, read_measured_perf_ctrs, &cur)) + if (smp_call_function_single(cpu, read_measured_perf_ctrs, &cur, 1)) return 0; #ifdef __i386__
| |