Re: [PATCH] idle thread preemption fix

[Ingo Molnar]

* Olaf Hering <olh@xxxxxxx> wrote:

> Whats the purpose of local_irq_disable() here? Locks up my toys in
> atkbd_init or IP hash foo functions.

fix already posted a couple of days ago, see:

--
* Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx> wrote:

> Hi Ingo !
> 
> Could you explain me precisely what is the race you are fixing by
> adding local_irq_disable() to rest_init() ?

it can be bad for the idle task to hold the BKL and to have preemption
enabled - in such a situation the scheduler will get confused if an
interrupt triggers a forced preemption in that small window. But it's
not necessary to keep IRQs disabled after the BKL has been dropped. In
fact i think IRQ-disabling doesnt have to be done at all, the patch
below ought to solve this scenario equally well, and should solve the
PPC side-effects too.

Tested ontop of 2.6.11-rc2 on x86 PREEMPT+SMP and PREEMPT+!SMP (which
IIRC were the config variants that triggered the original problem), on
an SMP and on a UP system.

	Ingo

Signed-off-by: Ingo Molnar <mingo@xxxxxxx>

--- linux/init/main.c.orig
+++ linux/init/main.c
@@ -373,14 +373,9 @@ static void noinline rest_init(void)
 {
 	kernel_thread(init, NULL, CLONE_FS | CLONE_SIGHAND);
 	numa_default_policy();
-	/*
-	 * Re-enable preemption but disable interrupts to make sure
-	 * we dont get preempted until we schedule() in cpu_idle().
-	 */
-	local_irq_disable();
-	preempt_enable_no_resched();
 	unlock_kernel();
- 	cpu_idle();
+	preempt_enable_no_resched();
+	cpu_idle();
 } 
 
 /* Check for early params. */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/