Re: [PATCH] idle thread preemption fix

Re: [PATCH] idle thread preemption fix

[Olaf Hering]

 On Sat, Jan 08, Linux Kernel Mailing List wrote:

> ChangeSet 1.2316, 2005/01/08 13:53:41-08:00, mingo@elte.hu
> 
> 	[PATCH] idle thread preemption fix
> 	
> 	The early bootup stage is pretty fragile because the idle thread is not yet
> 	functioning as such and so we need preemption disabled.  Whether the bootup
> 	fails or not seems to depend on timing details so e.g.  the presence of
> 	SCHED_SMT makes it go away.
> 	
> 	Disabling preemption explicitly has another advantage: the atomicity check
> 	in schedule() will catch early-bootup schedule() calls from now on.
> 	
> 	The patch also fixes another preempt-bkl buglet: interrupt-driven
> 	forced-preemption didnt go through preempt_schedule() so it resulted in
> 	auto-dropping of the BKL.  Now we go through preempt_schedule() which
> 	properly deals with the BKL.
> 	
> 	Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 	Signed-off-by: Andrew Morton <akpm@osdl.org>
> 	Signed-off-by: Linus Torvalds <torvalds@osdl.org>

> diff -Nru a/init/main.c b/init/main.c
> --- a/init/main.c	2005-01-08 15:18:18 -08:00
> +++ b/init/main.c	2005-01-08 15:18:18 -08:00
> @@ -373,6 +373,12 @@
>  {
>  	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();
>  } 

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

Re: [PATCH] idle thread preemption fix

[Ingo Molnar]

* Olaf Hering <olh@suse.de> 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@kernel.crashing.org> 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@elte.hu>

--- 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. */