sched domains bringup race?

sched domains bringup race?

[Dave Hansen]

I keep getting oopses for the non-boot CPU in find_busiest_group(). 
This occurs the first time that the CPU goes idle.  Those groups are set
up in sched_init_smp(), which is called after smp_init():

static int init(void * unused)
{
	...
        fixup_cpu_present_map();
        smp_init();
        sched_init_smp();

But, the idle threads for the secondary CPUs are initialized in
smp_init().  So, what happens when a CPU tries to schedule (using sched
domains) before sched_init_smp() completes?  I think it goes boom! :)

Anyway, I was thinking that we should just hold the runqueue lock on the
non-boot CPUs until the sched domain init code is done.  Does that sound
feasible?

-- Dave

Re: sched domains bringup race?

[Nick Piggin]

Dave Hansen wrote:
> I keep getting oopses for the non-boot CPU in find_busiest_group(). 
> This occurs the first time that the CPU goes idle.  Those groups are set
> up in sched_init_smp(), which is called after smp_init():
> 
> static int init(void * unused)
> {
> 	...
>         fixup_cpu_present_map();
>         smp_init();
>         sched_init_smp();
> 
> But, the idle threads for the secondary CPUs are initialized in
> smp_init().  So, what happens when a CPU tries to schedule (using sched
> domains) before sched_init_smp() completes?  I think it goes boom! :)
> 

It shouldn't because sched_init sets up dummy domains for
all runqueues.

Obviously something is going wrong somewhere though.

Re: sched domains bringup race?

[Dave Hansen]

On Thu, 2004-07-15 at 20:03, Nick Piggin wrote:
> It shouldn't because sched_init sets up dummy domains for
> all runqueues.
> 
> Obviously something is going wrong somewhere though.

Hmmm, but there still might be some concurrency problems, right?  There
isn't any locking while the setup is being done, so are all of the
intermediate initialization states valid?  Or, could one of the CPUs be
catching the init code in the middle of an operation?

-- Dave

Re: sched domains bringup race?

[Nick Piggin]

Dave Hansen wrote:
> On Thu, 2004-07-15 at 20:03, Nick Piggin wrote:
> 
>>It shouldn't because sched_init sets up dummy domains for
>>all runqueues.
>>
>>Obviously something is going wrong somewhere though.
> 
> 
> Hmmm, but there still might be some concurrency problems, right?  There
> isn't any locking while the setup is being done, so are all of the
> intermediate initialization states valid?  Or, could one of the CPUs be
> catching the init code in the middle of an operation?
> 

cpu_attach_domain is supposed to be able to do the switchover
without any races.

Re: sched domains bringup race?

[Nick Piggin]

Nick Piggin wrote:
> Dave Hansen wrote:
> 
>> On Thu, 2004-07-15 at 20:03, Nick Piggin wrote:
>>
>>> It shouldn't because sched_init sets up dummy domains for
>>> all runqueues.
>>>
>>> Obviously something is going wrong somewhere though.
>>
>>
>>
>> Hmmm, but there still might be some concurrency problems, right?  There
>> isn't any locking while the setup is being done, so are all of the
>> intermediate initialization states valid?  Or, could one of the CPUs be
>> catching the init code in the middle of an operation?
>>
> 
> cpu_attach_domain is supposed to be able to do the switchover
> without any races.

Although the sched_domain_debug is definitely racy. It should really
be locking each runqueue before traversing its domains. Just undef
SCHED_DOMAIN_DEBUG to be sure...

Re: sched domains bringup race?

[Keshavamurthy Anil S]

On Thu, Jul 15, 2004 at 07:13:46PM -0700, Dave Hansen wrote:
> I keep getting oopses for the non-boot CPU in find_busiest_group(). 
> This occurs the first time that the CPU goes idle.  Those groups are set
> up in sched_init_smp(), which is called after smp_init():
> 
> static int init(void * unused)
> {
> 	...
>         fixup_cpu_present_map();
>         smp_init();
>         sched_init_smp();
> 
> But, the idle threads for the secondary CPUs are initialized in
> smp_init().  So, what happens when a CPU tries to schedule (using sched
> domains) before sched_init_smp() completes?  I think it goes boom! :)
> 
> Anyway, I was thinking that we should just hold the runqueue lock on the
> non-boot CPUs until the sched domain init code is done.  Does that sound
> feasible?

Even on my system which is Intel 865 chipset (P4 with HT enabled system) 
I see a bug check somewhere in the schedular_tick during boot.
However if I move the sched_init_smp() after do_basic_setup() the
kernel boots without any problem. Any clue here?

 static int init(void * unused)
 {
 	...
         fixup_cpu_present_map();
         smp_init();
	 populate_rootfs();
	 do_basic_setup();
 
         sched_init_smp();
-Anil

Re: sched domains bringup race?

[Nick Piggin]

Keshavamurthy Anil S wrote:
> On Thu, Jul 15, 2004 at 07:13:46PM -0700, Dave Hansen wrote:
> 
>>I keep getting oopses for the non-boot CPU in find_busiest_group(). 
>>This occurs the first time that the CPU goes idle.  Those groups are set
>>up in sched_init_smp(), which is called after smp_init():
>>
>>static int init(void * unused)
>>{
>>	...
>>        fixup_cpu_present_map();
>>        smp_init();
>>        sched_init_smp();
>>
>>But, the idle threads for the secondary CPUs are initialized in
>>smp_init().  So, what happens when a CPU tries to schedule (using sched
>>domains) before sched_init_smp() completes?  I think it goes boom! :)
>>
>>Anyway, I was thinking that we should just hold the runqueue lock on the
>>non-boot CPUs until the sched domain init code is done.  Does that sound
>>feasible?
> 
> 
> Even on my system which is Intel 865 chipset (P4 with HT enabled system) 
> I see a bug check somewhere in the schedular_tick during boot.
> However if I move the sched_init_smp() after do_basic_setup() the
> kernel boots without any problem. Any clue here?
> 
>  static int init(void * unused)
>  {
>  	...
>          fixup_cpu_present_map();
>          smp_init();
> 	 populate_rootfs();
> 	 do_basic_setup();
>  
>          sched_init_smp();

There shouldn't be any problem doing that if we have to, obviously we
need to know why. Is it possible that cpu_sibling_map, or one of the
CPU masks isn't set up correctly at the time of the call?

Re: sched domains bringup race?

[Nathan Lynch]

On Mon, 2004-07-19 at 02:31, Nick Piggin wrote:
> Keshavamurthy Anil S wrote:
> > Even on my system which is Intel 865 chipset (P4 with HT enabled system) 
> > I see a bug check somewhere in the schedular_tick during boot.
> > However if I move the sched_init_smp() after do_basic_setup() the
> > kernel boots without any problem. Any clue here?
> 
> There shouldn't be any problem doing that if we have to, obviously we
> need to know why. Is it possible that cpu_sibling_map, or one of the
> CPU masks isn't set up correctly at the time of the call?

In 2.6.8-rc1-mm1 at least, backing this patch out fixed it for me on
ppc64:

http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.8-rc1/2.6.8-rc1-mm1/broken-out/detect-too-early-schedule-attempts.patch

Code with statements of the form:

if (system_state == SYSTEM_BOOTING)
	/* do something boot-specific */
else
	/* do something assuming system_state == SYSTEM_RUNNING */

is broken by this change.  Parts of the cpu bringup code in arch/ppc64
do this (and thus need to be fixed if the above change is kept). 
Chances are there is similar code in some x86 setups.

Nathan

Re: sched domains bringup race?

[Keshavamurthy Anil S]

On Thu, Jul 22, 2004 at 04:55:40PM -0500, Nathan Lynch wrote:
> On Mon, 2004-07-19 at 02:31, Nick Piggin wrote:
> > Keshavamurthy Anil S wrote:
> > > Even on my system which is Intel 865 chipset (P4 with HT enabled system) 
> > > I see a bug check somewhere in the schedular_tick during boot.
> > > However if I move the sched_init_smp() after do_basic_setup() the
> > > kernel boots without any problem. Any clue here?

  This was happening even without CONFIG_SCHED_SMT and later found to be
ACPI bug. Sorry for the confusion.

> > 
> > There shouldn't be any problem doing that if we have to, obviously we
> > need to know why. Is it possible that cpu_sibling_map, or one of the
> > CPU masks isn't set up correctly at the time of the call?
> 
> In 2.6.8-rc1-mm1 at least, backing this patch out fixed it for me on
> ppc64:
> 
> http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.8-rc1/2.6.8-rc1-mm1/broken-out/detect-too-early-schedule-attempts.patch
> 
> Code with statements of the form:
> 
> if (system_state == SYSTEM_BOOTING)
> 	/* do something boot-specific */
> else
> 	/* do something assuming system_state == SYSTEM_RUNNING */
> 
> is broken by this change.  Parts of the cpu bringup code in arch/ppc64
> do this (and thus need to be fixed if the above change is kept). 
> Chances are there is similar code in some x86 setups.
> 
> Nathan
> 

Re: sched domains bringup race?

[Nick Piggin]

Nathan Lynch wrote:

>On Mon, 2004-07-19 at 02:31, Nick Piggin wrote:
>
>>Keshavamurthy Anil S wrote:
>>
>>>Even on my system which is Intel 865 chipset (P4 with HT enabled system) 
>>>I see a bug check somewhere in the schedular_tick during boot.
>>>However if I move the sched_init_smp() after do_basic_setup() the
>>>kernel boots without any problem. Any clue here?
>>>
>>There shouldn't be any problem doing that if we have to, obviously we
>>need to know why. Is it possible that cpu_sibling_map, or one of the
>>CPU masks isn't set up correctly at the time of the call?
>>
>
>In 2.6.8-rc1-mm1 at least, backing this patch out fixed it for me on
>ppc64:
>
>http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.8-rc1/2.6.8-rc1-mm1/broken-out/detect-too-early-schedule-attempts.patch
>
>Code with statements of the form:
>
>if (system_state == SYSTEM_BOOTING)
>	/* do something boot-specific */
>else
>	/* do something assuming system_state == SYSTEM_RUNNING */
>
>is broken by this change.  Parts of the cpu bringup code in arch/ppc64
>do this (and thus need to be fixed if the above change is kept). 
>Chances are there is similar code in some x86 setups.
>
>

That patch can be dropped AFAIKS.

sched-clean-init-idle.patch introduces a better check.