Re: 2.6.29-rc1 does not boot

[Rusty Russell <rusty@rustcorp.com.au>]

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__