Re: spinlock lockup on CPU#0

[Hugh Dickins <hugh@veritas.com>]

On Sat, 26 Apr 2008, Ingo Molnar wrote:
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > > Hopefully the numbers are right, and hopefully this provides enough 
> > > info to help the kernel out
> > 
> > Well that's cute.  At a guess I'd say that acpi_processor_idle() 
> > managed to call sched_clock_idle_wakeup_event() with local interrupts 
> > enabled.  We took an interrupt with rq->lock held and things went 
> > downhill from there.
> > 
> > Can you add this please, see if it triggers?
> 
> there's fixes pending in this area. The main fix would be the one below.
> 
> 	Ingo
> 
> ---------------->
> Subject: idle (arch, acpi and apm) and lockdep
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Fri, 25 Apr 2008 17:39:01 +0200

Oh good, thanks, I see you've just asked Linus to pull that idle fix.

Thanks to Peter for the fix, and to Justin for calling attention to it:
this freeze has been plaguing me on one x86_32 box since I put -git8
on, and I've just verified that it's the same as what's been plaguing
me on that machine since 2.6.25-mm1 came out.

Andrew, here's a belated hotfix for 2.6.25-mm1, running nicely at last:

Peter Zijlstra's "idle (arch, acpi and apm) and lockdep" fix to
recent x86 freezes - adapted to 2.6.25-mm1 by omitting merges
from process_32.c and process_64.c into process.c.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

 arch/x86/kernel/apm_32.c      |    3 +++
 arch/x86/kernel/process_32.c  |   27 +++++++++++++++------------
 arch/x86/kernel/process_64.c  |    8 +++-----
 drivers/acpi/processor_idle.c |   19 +++++++++----------
 include/asm-x86/processor.h   |    1 +
 5 files changed, 31 insertions(+), 27 deletions(-)

--- 2.6.25-mm1/arch/x86/kernel/apm_32.c	2008-04-18 12:18:09.000000000 +0100
+++ linux/arch/x86/kernel/apm_32.c	2008-04-26 22:17:06.000000000 +0100
@@ -904,6 +904,7 @@ recalc:
 			original_pm_idle();
 		else
 			default_idle();
+		local_irq_disable();
 		jiffies_since_last_check = jiffies - last_jiffies;
 		if (jiffies_since_last_check > idle_period)
 			goto recalc;
@@ -911,6 +912,8 @@ recalc:
 
 	if (apm_idle_done)
 		apm_do_busy();
+
+	local_irq_enable();
 }
 
 /**
--- 2.6.25-mm1/arch/x86/kernel/process_32.c	2008-04-18 12:18:09.000000000 +0100
+++ linux/arch/x86/kernel/process_32.c	2008-04-26 22:29:36.000000000 +0100
@@ -111,12 +111,10 @@ void default_idle(void)
 		 */
 		smp_mb();
 
-		local_irq_disable();
-		if (!need_resched()) {
+		if (!need_resched())
 			safe_halt();	/* enables interrupts racelessly */
-			local_irq_disable();
-		}
-		local_irq_enable();
+		else
+			local_irq_enable();
 		current_thread_info()->status |= TS_POLLING;
 	} else {
 		local_irq_enable();
@@ -196,6 +194,7 @@ void cpu_idle(void)
 			if (cpu_is_offline(cpu))
 				play_dead();
 
+			local_irq_disable();
 			__get_cpu_var(irq_stat).idle_timestamp = jiffies;
 			/* Don't trace irqs off for idle */
 			stop_critical_timings();
@@ -245,18 +244,22 @@ void mwait_idle_with_hints(unsigned long
 		__monitor((void *)&current_thread_info()->flags, 0, 0);
 		smp_mb();
 		if (!need_resched())
-			__sti_mwait(ax, cx);
-		else
-			local_irq_enable();
-	} else
-		local_irq_enable();
+			__mwait(ax, cx);
+	}
 }
 
 /* Default MONITOR/MWAIT with no hints, used for default C1 state */
 static void mwait_idle(void)
 {
-	local_irq_enable();
-	mwait_idle_with_hints(0, 0);
+	if (!need_resched()) {
+		__monitor((void *)&current_thread_info()->flags, 0, 0);
+		smp_mb();
+		if (!need_resched())
+			__sti_mwait(0, 0);
+		else
+			local_irq_enable();
+	} else
+		local_irq_enable();
 }
 
 static int __cpuinit mwait_usable(const struct cpuinfo_x86 *c)
--- 2.6.25-mm1/arch/x86/kernel/process_64.c	2008-04-18 12:18:09.000000000 +0100
+++ linux/arch/x86/kernel/process_64.c	2008-04-26 22:19:29.000000000 +0100
@@ -106,12 +106,10 @@ void default_idle(void)
 	 * test NEED_RESCHED:
 	 */
 	smp_mb();
-	local_irq_disable();
-	if (!need_resched()) {
+	if (!need_resched())
 		safe_halt();	/* enables interrupts racelessly */
-		local_irq_disable();
-	}
-	local_irq_enable();
+	else
+		local_irq_enable();
 	current_thread_info()->status |= TS_POLLING;
 }
 
--- 2.6.25-mm1/drivers/acpi/processor_idle.c	2008-04-18 12:18:10.000000000 +0100
+++ linux/drivers/acpi/processor_idle.c	2008-04-26 22:17:06.000000000 +0100
@@ -436,13 +436,12 @@ static void acpi_processor_idle(void)
 
 	cx = pr->power.state;
 	if (!cx || acpi_idle_suspend) {
-		if (pm_idle_save)
-			pm_idle_save();
-		else
+		if (pm_idle_save) {
+			pm_idle_save(); /* enables IRQs */
+		} else {
 			acpi_safe_halt();
-
-		if (irqs_disabled())
 			local_irq_enable();
+		}
 
 		return;
 	}
@@ -538,10 +537,12 @@ static void acpi_processor_idle(void)
 		 * Use the appropriate idle routine, the one that would
 		 * be used without acpi C-states.
 		 */
-		if (pm_idle_save)
-			pm_idle_save();
-		else
+		if (pm_idle_save) {
+			pm_idle_save(); /* enables IRQs */
+		} else {
 			acpi_safe_halt();
+			local_irq_enable();
+		}
 
 		/*
 		 * TBD: Can't get time duration while in C1, as resumes
@@ -552,8 +553,6 @@ static void acpi_processor_idle(void)
 		 *       skew otherwise.
 		 */
 		sleep_ticks = 0xFFFFFFFF;
-		if (irqs_disabled())
-			local_irq_enable();
 
 		break;
 
--- 2.6.25-mm1/include/asm-x86/processor.h	2008-04-18 12:18:34.000000000 +0100
+++ linux/include/asm-x86/processor.h	2008-04-26 22:17:06.000000000 +0100
@@ -726,6 +726,7 @@ static inline void __mwait(unsigned long
 
 static inline void __sti_mwait(unsigned long eax, unsigned long ecx)
 {
+	trace_hardirqs_on();
 	/* "mwait %eax, %ecx;" */
 	asm volatile("sti; .byte 0x0f, 0x01, 0xc9;"
 		     :: "a" (eax), "c" (ecx));