failing kthread_create_on_node()

failing kthread_create_on_node()

[Konrad Eisele]

Hello,
I'm trying to get LEON run SMP in the latest kernel again.
Currently I'm ending up in the following state for a 2-CPU
system:
When smp_init() boost up the other CPU you end up in
->kernel/stop_machine.c:cpu_stop_cpu_callback() with hcpu=1
and a kthread is created for cpu 1 using
		p = kthread_create_on_node(cpu_stopper_thread,
					   stopper,
					   cpu_to_node(cpu),
					   "migration/%d", cpu);
It seems like that cpu_to_node(cpu) is ignored.
I end up in the following situation:
  <p> (the task created by kthread_create_on_node()) gets
inserted into runqueue[0], then still in cpu_stop_cpu_callback()
sched_set_stop_task(1,<p>) is called that
sets
   ...
   cpu_rq(1)->stop = stop;
   ...
which has the effect that kernel/sched/core.c:pick_next_task()'s
         ...
	for_each_class(class) {
		p = class->pick_next_task(rq);
		if (p)
			return p;
	}
         ...
will return rq->stop through stop-task scheduling class and
then CPU 1 will execute <p>. However <p> is actually bound to
rq[0] and i.e. task_thread_info(p)->cpu is 0 so for instance
smp_processor_id() will return 0 and the crash happens as a
side-effect of cpu0 and cpu1 at the end executing <init>-task
at the same time.

I'd like to try to fix this but before messing with arch/sparc/kernel_thread()
and arch/sparc/sparc_do_fork() I'd first want for how to fix this
the best way...

--  Greetings Konrad

Re: failing kthread_create_on_node()

[David Miller]

From: Konrad Eisele <konrad@gaisler.com>
Date: Mon, 16 Apr 2012 11:58:14 +0200

> will return rq->stop through stop-task scheduling class and
> then CPU 1 will execute <p>. However <p> is actually bound to
> rq[0] and i.e. task_thread_info(p)->cpu is 0 so for instance
> smp_processor_id() will return 0 and the crash happens as a
> side-effect of cpu0 and cpu1 at the end executing <init>-task
> at the same time.
> 
> I'd like to try to fix this but before messing with
> arch/sparc/kernel_thread()
> and arch/sparc/sparc_do_fork() I'd first want for how to fix this
> the best way...

I don't see why special handling should be necessary.

Sparc64 doesn't do anything special wrt. thread_info()->cpu in
it's fork handling, so I don't see why sparc32 would need to.

If the scheduler is executing a task from the runqueue of one
cpu, on another cpu, that doesn't sound like a sparc problem
unless some other piece of sparc specific state is not setup
properly.

Please describe the sequence of events in more detail so we
can analyze this better.

Thanks.

Re: failing kthread_create_on_node()

[Konrad Eisele]

>
> Please describe the sequence of events in more detail so we
> can analyze this better.

Here is the state of the kernel that causes the failure:
( <p> is the kthread created by
   kernel/stop_machine.c:cpu_stop_cpu_callback(),
   2 cpu system, cpu0 and cpu1, cpu0 has run-queue rq0,
   cpu1 has run-queue rq1, the run-queue structure has a
   field called "stop", "..." means "whatever"
)

rq0[ ... <p> ...] , rq0.stop=...
rq1[ ]            , rq1.stop=<p>

1. How to get there
...
CPU0 boots up and
->kernel/smp.c:smp_init()
  ->kernel/cpu.c:__cpu_up() calls __cpu_notify(CPU_UP_PREPARE..)
    ...->stop_machine.c:cpu_stop_cpu_callback()
         here you have the following code sequence:
         ...
310: 	case CPU_UP_PREPARE:
311:		BUG_ON(stopper->thread || stopper->enabled ||
312:		       !list_empty(&stopper->works));
313:		p = kthread_create_on_node(cpu_stopper_thread,
314:					   stopper,
315:					   cpu_to_node(cpu),
316:					   "migration/%d", cpu);
317:		if (IS_ERR(p))
318:			return notifier_from_errno(PTR_ERR(p));
319:		get_task_struct(p);
321:		kthread_bind(p, cpu);
322:		sched_set_stop_task(cpu, p);
323:		stopper->thread = p;
324:		break;
        ...
        I observe that lines 313-321 craete kthread <p> but it is
        in rq0 . I'm not shure why kthread_bind doesnt move <p> to
        rq1. Then come line 322 that calls:
        -> kernel/core/sched.c:sched_set_stop_task(1,<p>)
        and there you have the line:
        ...
980:   cpu_rq(cpu)->stop = stop;
        ...
        (where "stop" is <p> and cpu is 1).
        Now you end up with the above described state. You have <p> in
        rq0 and also in rq1.stop

2. What happens next

    CPU1 will boot up and execute schedule().
    ...
    -> kernel/core.c:__schedule()
         ...
3158:   cpu = smp_processor_id();
3159:	rq = cpu_rq(cpu);
         ...
3199: 	put_prev_task(rq, prev);
3200:	next = pick_next_task(rq);
3201:	clear_tsk_need_resched(prev);
         ...
     CPU1 is executing, so cpu = 1 and rq = rq1
     Line 3200 will call
    -> kernel/core.c:pick_next_task()

         ...
3137:	for_each_class(class) {
3138:		p = class->pick_next_task(rq);
3139:		if (p)
3140:			return p;
3141:	}
         ...
     (rq is still rq1)
     Line 3138 will end up in kernel/sched/stop_task.c:pick_next_task_stop()
         ...
28:	struct task_struct *stop = rq->stop;
29:
30:	if (stop && stop->on_rq)
31:		return stop;
32:
33:	return NULL;
         ...

With the state of the kernel being:

rq0[ ... <p> ...] , rq0.stop=...
rq1[ ]            , rq1.stop=<p>

You get as "next" in __schedule <p>, even though CPU is cpu1 and <p> in
in rq0 and has thread_info(<p>)->cpu set to 0.

3. Failure

After CPU1 has switched to <p> it will end up in __schedule()
again. However because of
#define raw_smp_processor_id()		(current_thread_info()->cpu)
now smp_processor_id() returns 0 even though you are cpu1 ( because <p> is on rq0)
=> lots of stange things happen and the kernel crashes
    eventually.


4. Question

Where should I look for the solution. Is it
  -  kthread_bind(p, cpu)
     That should move <p> to rq1.
  -  sched_set_stop_task(cpu, p)
     That should force move of <p> to rq1
  -  Should maybe smp_processor_id() be redefined to hard_smp_processor_id()
  -  rq1 is empty, maybe idle_balance(1, rq1) in schedule() should have migrated
     <p> to rq1, however it doesnt do it right now.

-- Greetings Konrad

Re: failing kthread_create_on_node()

[David Miller]

From: Konrad Eisele <konrad@gaisler.com>
Date: Tue, 17 Apr 2012 08:37:50 +0200

> 2. What happens next
> 
>    CPU1 will boot up and execute schedule().
>    ...
>    -> kernel/core.c:__schedule()

The stop thread doesn't actually run immediately when it is created,
so will need to be woken up to be scheduled and run.

At wakeup time, set_task_cpu() should fix things up and make the
task have a correct ->cpu in it's thread_info.

I wonder if one of the sparc32 global cpu masks isn't being
initialized properly.

Re: failing kthread_create_on_node()

[Konrad Eisele]

David Miller wrote:
> From: Konrad Eisele<konrad@gaisler.com>
> Date: Tue, 17 Apr 2012 08:37:50 +0200
>
>> 2. What happens next
>>
>>     CPU1 will boot up and execute schedule().
>>     ...
>>     ->  kernel/core.c:__schedule()
>
> The stop thread doesn't actually run immediately when it is created,
> so will need to be woken up to be scheduled and run.
>
> At wakeup time, set_task_cpu() should fix things up and make the
> task have a correct ->cpu in it's thread_info.
>
> I wonder if one of the sparc32 global cpu masks isn't being
> initialized properly.

Ok, thanks. I'll investigate in that direction.

> --
> To unsubscribe from this list: send the line "unsubscribe sparclinux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

Re: failing kthread_create_on_node()

[Konrad Eisele]

David Miller wrote:
> From: Konrad Eisele<konrad@gaisler.com>
> Date: Tue, 17 Apr 2012 08:37:50 +0200
>
>> 2. What happens next
>>
>>     CPU1 will boot up and execute schedule().
>>     ...
>>     ->  kernel/core.c:__schedule()
>
> The stop thread doesn't actually run immediately when it is created,
> so will need to be woken up to be scheduled and run.
>
> At wakeup time, set_task_cpu() should fix things up and make the
> task have a correct ->cpu in it's thread_info.
>
> I wonder if one of the sparc32 global cpu masks isn't being
> initialized properly.
> --
> To unsubscribe from this list: send the line "unsubscribe sparclinux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

I have posted
"[PATCH 1/1] sparc: task_cpu() for stopper thread on sparc32 returns 0"

I traced it further and you where right, the stop thread of CPU1 was
sleeping.
The reason that the stopper thread for cpu-1 gets inserted into runqueue-0
(which causes fatal failure later on) is traced below:

When CPU1 comes online it is woken up. Then I end up in
the following call path:

-> kernel/sched/core.c():try_to_wake_up():
         ...
1628:	cpu = select_task_rq(p, SD_BALANCE_WAKE, wake_flags);
1629:	if (task_cpu(p) != cpu) {
         ...
      -> kernel/sched/core.c:select_task_rq():
           ...
1332:	  int cpu = p->sched_class->select_task_rq(p, sd_flags, wake_flags);
           ...
         -> kernel/sched/stop_task.c:select_task_rq_stop():
             ...
16:         return task_cpu(p); /* stop tasks as never migrate */
             ...


however task_cpu(p) is 0.

The patch posted checks weather a different cpu should be returned in
kernel/sched/stop_task.c:select_task_rq_stop().

Hope that makes sense.

-- Konrad

Re: failing kthread_create_on_node()

[Konrad Eisele]

> When CPU1 comes online it is woken up. Then I end up in

Sorry, I mean:
When CPU1 comes online "its stopper thread" is woken up.

Re: failing kthread_create_on_node()

[David Miller]

This is still mysterious, because no other platform sees this problem.

You need to first determine why other platforms bootup properly on SMP
and only sparc32 does not.

This is the first question every scheduler hacker is going to ask
you when you propose that strange patch you just posted.