* [PATCH] fix problem with reschenduling in rest_init (2.6.35-rc3)
@ 2010-06-23 20:11 Ilya Loginov
2010-06-24 13:11 ` Peter Zijlstra
0 siblings, 1 reply; 15+ messages in thread
From: Ilya Loginov @ 2010-06-23 20:11 UTC (permalink / raw)
To: torvalds; +Cc: linux-kernel
There is a bug in rest_init function. The problem is that kernel_init
thread starts before initialization of kthreadd_task when
CONFIG_PREEMPT_VOLUNTARY is enabled.
kernel_init thread do wake_up_process(kthreadd_task) and I have kernel Oops in
try_to_wake_up when I try to get p->state.
I found this problem on 2.6.34 on FPGA. It is very slow, and
find_task_by_pid is done after reschenduling. I have no this problem on
2.6.35-rc3 because kernel code is moved. But if I write simple loop like
volatile int tmp;
for(i = 0; i < preset_lpj; i++)
tmp++;
right after kernel_thread(kernel_init,.... , I have this problem on
2.6.35-rc3 too.
I understand that real problem is reschenduling in init code, but
I have no ability to fix it.
Signed-off-by: Ilya Loginov <isloginov@gmail.com>
---
diff --git a/init/main.c b/init/main.c
index 3bdb152..9febd69 100644
--- a/init/main.c
+++ b/init/main.c
@@ -428,12 +428,12 @@ static noinline void __init_refok rest_init(void)
int pid;
rcu_scheduler_starting();
- kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
numa_default_policy();
pid = kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
rcu_read_lock();
kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns);
rcu_read_unlock();
+ kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
unlock_kernel();
/*
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] fix problem with reschenduling in rest_init (2.6.35-rc3)
2010-06-23 20:11 [PATCH] fix problem with reschenduling in rest_init (2.6.35-rc3) Ilya Loginov
@ 2010-06-24 13:11 ` Peter Zijlstra
2010-06-24 13:23 ` Ilya Loginov
0 siblings, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2010-06-24 13:11 UTC (permalink / raw)
To: Ilya Loginov; +Cc: torvalds, linux-kernel, Ingo Molnar
On Thu, 2010-06-24 at 00:11 +0400, Ilya Loginov wrote:
> There is a bug in rest_init function. The problem is that kernel_init
> thread starts before initialization of kthreadd_task when
> CONFIG_PREEMPT_VOLUNTARY is enabled.
>
> kernel_init thread do wake_up_process(kthreadd_task) and I have kernel Oops in
> try_to_wake_up when I try to get p->state.
>
> I found this problem on 2.6.34 on FPGA. It is very slow, and
> find_task_by_pid is done after reschenduling. I have no this problem on
> 2.6.35-rc3 because kernel code is moved. But if I write simple loop like
> volatile int tmp;
> for(i = 0; i < preset_lpj; i++)
> tmp++;
> right after kernel_thread(kernel_init,.... , I have this problem on
> 2.6.35-rc3 too.
>
> I understand that real problem is reschenduling in init code, but
> I have no ability to fix it.
Ooh, interesting problem, so kernel_init() will indeed spawn all kinds
of kernel threads, and doing so before we create the kthreadd will
result in the oops you observed.
However I suspect the ordering is like it is because we want init to
have pid 1, if we were to re-order like you suggest kthreadd will end up
with pid 1 and init with pid 2.
Humm, how to fix this...
> Signed-off-by: Ilya Loginov <isloginov@gmail.com>
> ---
> diff --git a/init/main.c b/init/main.c
> index 3bdb152..9febd69 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -428,12 +428,12 @@ static noinline void __init_refok rest_init(void)
> int pid;
>
> rcu_scheduler_starting();
> - kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
> numa_default_policy();
> pid = kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
> rcu_read_lock();
> kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns);
> rcu_read_unlock();
> + kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
> unlock_kernel();
>
> /*
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] fix problem with reschenduling in rest_init (2.6.35-rc3)
2010-06-24 13:11 ` Peter Zijlstra
@ 2010-06-24 13:23 ` Ilya Loginov
2010-06-24 14:08 ` Илья Логинов
2010-06-28 9:01 ` [PATCH] init: Fix race between init and kthreadd Peter Zijlstra
0 siblings, 2 replies; 15+ messages in thread
From: Ilya Loginov @ 2010-06-24 13:23 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: torvalds, linux-kernel, Ingo Molnar
On Thu, 24 Jun 2010 15:11:36 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
> However I suspect the ordering is like it is because we want init to
> have pid 1, if we were to re-order like you suggest kthreadd will end up
> with pid 1 and init with pid 2.
Strange, but init does not die after I did this. Fix me if I wrong, but it wants
to have pid 1, and die in other case.
Interesting...
--
Ilya Loginov <isloginov@gmail.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] fix problem with reschenduling in rest_init (2.6.35-rc3)
2010-06-24 13:23 ` Ilya Loginov
@ 2010-06-24 14:08 ` Илья Логинов
2010-06-28 9:01 ` [PATCH] init: Fix race between init and kthreadd Peter Zijlstra
1 sibling, 0 replies; 15+ messages in thread
From: Илья Логинов @ 2010-06-24 14:08 UTC (permalink / raw)
To: Ilya Loginov; +Cc: Peter Zijlstra, torvalds, linux-kernel, Ingo Molnar
On Thu, 24 Jun 2010 17:23:34 +0400
Ilya Loginov <isloginov@gmail.com> wrote:
> Strange, but init does not die after I did this. Fix me if I wrong, but it wants
> to have pid 1, and die in other case.
>
> Interesting...
Yeah, of course, I am wrong. To make it clear I watched sources of sysvinit and
founded out that if pid not equal to 1 init does
exit(telinit(p, argc, argv));
So, it does not die. Anyway, I would have kernel panic.
--
Илья Логинов <zerone2@gmail.com>
ЪТХ╨{.nг+┴╥÷╝┴╜├+%┼кЪ╠Ищ╤\x17╔┼wЪ╨{.nг+┴╥╔┼{╠ЧG╚²ИЪ┼{ay╨\x1dй┤з≥К,j\a╜╒fё╒╥h ▐О│ЙЪ▒ЙГz_Х╝\x03(╜И ▌┼щ╒j"²З\x1a╤^[m╖ЪЪ╬\a╚ЧG╚²ИЪ╒╦?≥╗Х╜з&ёЬ╖~▐А╤iO∙Ф╛z╥ vь^\x14\x04\x1a╤^[m╖ЪЪц\fЪ╤ЛЪ╒╦?√I╔
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] init: Fix race between init and kthreadd
2010-06-24 13:23 ` Ilya Loginov
2010-06-24 14:08 ` Илья Логинов
@ 2010-06-28 9:01 ` Peter Zijlstra
2010-06-28 11:53 ` [PATCH] init: Fix race between init and kthreadd -v2 Peter Zijlstra
1 sibling, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2010-06-28 9:01 UTC (permalink / raw)
To: Ilya Loginov; +Cc: torvalds, linux-kernel, Ingo Molnar
On Thu, 2010-06-24 at 17:23 +0400, Ilya Loginov wrote:
> On Thu, 24 Jun 2010 15:11:36 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
>
> > However I suspect the ordering is like it is because we want init to
> > have pid 1, if we were to re-order like you suggest kthreadd will end up
> > with pid 1 and init with pid 2.
>
> Strange, but init does not die after I did this. Fix me if I wrong, but it wants
> to have pid 1, and die in other case.
Does something like this work for you?
---
Subject: init: Fix race between init and kthreadd
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Mon Jun 28 10:49:09 CEST 2010
Ilya reported that on a very slow machine he could reliably reproduce a
race between forking init and kthreadd. We first fork init so that it
obtains pid-1, however since the scheduler is already fully running at
this point it can preempt and run the init thread before we spawn and
set kthreadd_task.
The init thread can then attempt spawning kthreads without kthreadd being
present which results in an OOPS.
Cure this in a crude way by having the init task spin-wait on
kthreadd_task. Nicer solutions are more complex and have more overhead
which doesn't appear worth it.
Reported-by: Ilya Loginov <isloginov@gmail.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
init/main.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
Index: linux-2.6/init/main.c
===================================================================
--- linux-2.6.orig/init/main.c
+++ linux-2.6/init/main.c
@@ -426,6 +426,17 @@ static noinline void __init_refok rest_i
int pid;
rcu_scheduler_starting();
+ /*
+ * Here we first fork the init thread and then the kthreadd so that
+ * init ends up with pid-1.
+ *
+ * Since the scheduler is already fully active we can end up
+ * running the init thread for long enough to start spawning kthreads
+ * before this thread continues and spawns/sets kthreadd, which
+ * would result in an OOPS.
+ *
+ * See the serialization against kthreadd_task in kernel_init().
+ */
kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
numa_default_policy();
pid = kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
@@ -847,6 +858,14 @@ static noinline int init_post(void)
static int __init kernel_init(void * unused)
{
+ /*
+ * Synchronize against setting kthreadd_task in rest_init().
+ * Using a mutex would have been a lot nicer, but since its a very
+ * rare race don't bother wasting the space overhead.
+ */
+ while (!kthreadd_task)
+ yield();
+
lock_kernel();
/*
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] init: Fix race between init and kthreadd -v2
2010-06-28 9:01 ` [PATCH] init: Fix race between init and kthreadd Peter Zijlstra
@ 2010-06-28 11:53 ` Peter Zijlstra
2010-06-28 14:19 ` Ingo Molnar
2010-06-28 20:06 ` [PATCH] init: Fix race between init and kthreadd -v2 Ilya Loginov
0 siblings, 2 replies; 15+ messages in thread
From: Peter Zijlstra @ 2010-06-28 11:53 UTC (permalink / raw)
To: Ilya Loginov; +Cc: torvalds, linux-kernel, Ingo Molnar
On Mon, 2010-06-28 at 11:01 +0200, Peter Zijlstra wrote:
> static int __init kernel_init(void * unused)
> {
> + /*
> + * Synchronize against setting kthreadd_task in rest_init().
> + * Using a mutex would have been a lot nicer, but since its a very
> + * rare race don't bother wasting the space overhead.
> + */
> + while (!kthreadd_task)
> + yield();
> +
> lock_kernel();
I just realized its all __init code so its all 'free' anyway, how about
the nicer version:
---
Subject: init: Fix race between init and kthreadd -v2
Ilya reported that on a very slow machine he could reliably reproduce a
race between forking init and kthreadd. We first fork init so that it
obtains pid-1, however since the scheduler is already fully running at
this point it can preempt and run the init thread before we spawn and
set kthreadd_task.
The init thread can then attempt spawning kthreads without kthreadd
being present which results in an OOPS.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
init/main.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/init/main.c b/init/main.c
index e2a2bf3..8f2acf5 100644
--- a/init/main.c
+++ b/init/main.c
@@ -420,18 +420,27 @@ static void __init setup_command_line(char *command_line)
* gcc-3.4 accidentally inlines this function, so use noinline.
*/
+static __initdata DEFINE_MUTEX(kthreadd_lock);
+
static noinline void __init_refok rest_init(void)
__releases(kernel_lock)
{
int pid;
rcu_scheduler_starting();
+ /*
+ * We need to spawn init first so that it obtains pid-1, however
+ * the init task will end up wanting to create kthreads, which
+ * if we schedule it before we create kthreadd, will OOPS.
+ */
+ mutex_lock(&kthreadd_lock);
kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
numa_default_policy();
pid = kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
rcu_read_lock();
kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns);
rcu_read_unlock();
+ mutex_unlock(&kthreadd_lock);
unlock_kernel();
/*
@@ -847,6 +856,13 @@ static noinline int init_post(void)
static int __init kernel_init(void * unused)
{
+ /*
+ * We spawned this thread while holding this lock, ensure the
+ * locked section in rest_init() is complete before proceeding.
+ */
+ mutex_lock(&kthreadd_lock);
+ mutex_unlock(&kthreadd_lock);
+
lock_kernel();
/*
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] init: Fix race between init and kthreadd -v2
2010-06-28 11:53 ` [PATCH] init: Fix race between init and kthreadd -v2 Peter Zijlstra
@ 2010-06-28 14:19 ` Ingo Molnar
2010-06-28 14:51 ` [PATCH] init: Fix race between init and kthreadd -v3 Peter Zijlstra
2010-06-28 20:06 ` [PATCH] init: Fix race between init and kthreadd -v2 Ilya Loginov
1 sibling, 1 reply; 15+ messages in thread
From: Ingo Molnar @ 2010-06-28 14:19 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Ilya Loginov, torvalds, linux-kernel
* Peter Zijlstra <peterz@infradead.org> wrote:
> +static __initdata DEFINE_MUTEX(kthreadd_lock);
> + /*
> + * We need to spawn init first so that it obtains pid-1, however
> + * the init task will end up wanting to create kthreads, which
> + * if we schedule it before we create kthreadd, will OOPS.
> + */
> + mutex_lock(&kthreadd_lock);
> kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
> numa_default_policy();
> pid = kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
> rcu_read_lock();
> kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns);
> rcu_read_unlock();
> + mutex_unlock(&kthreadd_lock);
> unlock_kernel();
>
> /*
> @@ -847,6 +856,13 @@ static noinline int init_post(void)
>
> static int __init kernel_init(void * unused)
> {
> + /*
> + * We spawned this thread while holding this lock, ensure the
> + * locked section in rest_init() is complete before proceeding.
> + */
> + mutex_lock(&kthreadd_lock);
> + mutex_unlock(&kthreadd_lock);
I think you may be using a mutex as a completion in essence. Why not use
completions instead?
Ingo
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] init: Fix race between init and kthreadd -v3
2010-06-28 14:19 ` Ingo Molnar
@ 2010-06-28 14:51 ` Peter Zijlstra
2010-06-28 15:03 ` Linus Torvalds
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Peter Zijlstra @ 2010-06-28 14:51 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Ilya Loginov, torvalds, linux-kernel
On Mon, 2010-06-28 at 16:19 +0200, Ingo Molnar wrote:
> I think you may be using a mutex as a completion in essence. Why not use
> completions instead?
Totally forgot about those things.. Yes they fit perfectly.
---
Subject: init: Fix race between init and kthreadd -v2
Ilya reported that on a very slow machine he could reliably reproduce a
race between forking init and kthreadd. We first fork init so that it
obtains pid-1, however since the scheduler is already fully running at
this point it can preempt and run the init thread before we spawn and
set kthreadd_task.
The init thread can then attempt spawning kthreads without kthreadd
being present which results in an OOPS.
Reported-by: Ilya Loginov <isloginov@gmail.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
init/main.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/init/main.c b/init/main.c
index e2a2bf3..2280f63 100644
--- a/init/main.c
+++ b/init/main.c
@@ -420,18 +420,26 @@ static void __init setup_command_line(char *command_line)
* gcc-3.4 accidentally inlines this function, so use noinline.
*/
+static __initdata DECLARE_COMPLETION(kthreadd_done);
+
static noinline void __init_refok rest_init(void)
__releases(kernel_lock)
{
int pid;
rcu_scheduler_starting();
+ /*
+ * We need to spawn init first so that it obtains pid-1, however
+ * the init task will end up wanting to create kthreads, which, if
+ * we schedule it before we create kthreadd, will OOPS.
+ */
kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
numa_default_policy();
pid = kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
rcu_read_lock();
kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns);
rcu_read_unlock();
+ complete(&kthreadd_done);
unlock_kernel();
/*
@@ -847,6 +855,10 @@ static noinline int init_post(void)
static int __init kernel_init(void * unused)
{
+ /*
+ * Wait until kthreadd is all set-up.
+ */
+ wait_for_completion(&kthreadd_done);
lock_kernel();
/*
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] init: Fix race between init and kthreadd -v3
2010-06-28 14:51 ` [PATCH] init: Fix race between init and kthreadd -v3 Peter Zijlstra
@ 2010-06-28 15:03 ` Linus Torvalds
2010-06-28 16:23 ` Randy Dunlap
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Linus Torvalds @ 2010-06-28 15:03 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Ingo Molnar, Ilya Loginov, linux-kernel
On Mon, Jun 28, 2010 at 7:51 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> static int __init kernel_init(void * unused)
> {
> + /*
> + * Wait until kthreadd is all set-up.
> + */
> + wait_for_completion(&kthreadd_done);
> lock_kernel();
Ack. Looks sane.
Linus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] init: Fix race between init and kthreadd -v3
2010-06-28 14:51 ` [PATCH] init: Fix race between init and kthreadd -v3 Peter Zijlstra
2010-06-28 15:03 ` Linus Torvalds
@ 2010-06-28 16:23 ` Randy Dunlap
2010-06-28 20:24 ` [tip:sched/urgent] init, sched: Fix race between init and kthreadd tip-bot for Peter Zijlstra
2010-06-30 8:32 ` [PATCH] init: Fix race between init and kthreadd -v3 Ilya Loginov
3 siblings, 0 replies; 15+ messages in thread
From: Randy Dunlap @ 2010-06-28 16:23 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Ingo Molnar, Ilya Loginov, torvalds, linux-kernel
On Mon, 28 Jun 2010 16:51:01 +0200 Peter Zijlstra wrote:
> On Mon, 2010-06-28 at 16:19 +0200, Ingo Molnar wrote:
>
> > I think you may be using a mutex as a completion in essence. Why not use
> > completions instead?
>
> Totally forgot about those things.. Yes they fit perfectly.
>
> ---
> Subject: init: Fix race between init and kthreadd -v2
>
> Ilya reported that on a very slow machine he could reliably reproduce a
> race between forking init and kthreadd. We first fork init so that it
> obtains pid-1, however since the scheduler is already fully running at
> this point it can preempt and run the init thread before we spawn and
> set kthreadd_task.
>
> The init thread can then attempt spawning kthreads without kthreadd
> being present which results in an OOPS.
>
> Reported-by: Ilya Loginov <isloginov@gmail.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
> init/main.c | 12 ++++++++++++
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index e2a2bf3..2280f63 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -420,18 +420,26 @@ static void __init setup_command_line(char *command_line)
> * gcc-3.4 accidentally inlines this function, so use noinline.
> */
>
> +static __initdata DECLARE_COMPLETION(kthreadd_done);
> +
> static noinline void __init_refok rest_init(void)
> __releases(kernel_lock)
> {
> int pid;
>
> rcu_scheduler_starting();
> + /*
> + * We need to spawn init first so that it obtains pid-1, however
Would be better to say "pid 1" or "pid=1".
"pid-1" seems odd to me.
> + * the init task will end up wanting to create kthreads, which, if
> + * we schedule it before we create kthreadd, will OOPS.
> + */
> kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
> numa_default_policy();
> pid = kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
> rcu_read_lock();
> kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns);
> rcu_read_unlock();
> + complete(&kthreadd_done);
> unlock_kernel();
>
> /*
> @@ -847,6 +855,10 @@ static noinline int init_post(void)
>
> static int __init kernel_init(void * unused)
> {
> + /*
> + * Wait until kthreadd is all set-up.
> + */
> + wait_for_completion(&kthreadd_done);
> lock_kernel();
>
> /*
>
> --
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] init: Fix race between init and kthreadd -v2
2010-06-28 11:53 ` [PATCH] init: Fix race between init and kthreadd -v2 Peter Zijlstra
2010-06-28 14:19 ` Ingo Molnar
@ 2010-06-28 20:06 ` Ilya Loginov
1 sibling, 0 replies; 15+ messages in thread
From: Ilya Loginov @ 2010-06-28 20:06 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: torvalds, linux-kernel, Ingo Molnar
On Mon, 28 Jun 2010 13:53:17 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
> I just realized its all __init code so its all 'free' anyway, how about
> the nicer version:
I will try both versions and write about results. But I think, that there is
some logical inconsistent in these patches (for me). I like v2 more than v1.
But, I would move mutex_lock/unlock. Anyway, I will try your patches first.
Many thanks.
--
Ilya Loginov <isloginov@gmail.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [tip:sched/urgent] init, sched: Fix race between init and kthreadd
2010-06-28 14:51 ` [PATCH] init: Fix race between init and kthreadd -v3 Peter Zijlstra
2010-06-28 15:03 ` Linus Torvalds
2010-06-28 16:23 ` Randy Dunlap
@ 2010-06-28 20:24 ` tip-bot for Peter Zijlstra
2010-06-30 8:32 ` [PATCH] init: Fix race between init and kthreadd -v3 Ilya Loginov
3 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Peter Zijlstra @ 2010-06-28 20:24 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, torvalds, a.p.zijlstra, peterz,
isloginov, tglx, mingo
Commit-ID: b433c3d4549ae74935b585115f076c6fb7bc48fe
Gitweb: http://git.kernel.org/tip/b433c3d4549ae74935b585115f076c6fb7bc48fe
Author: Peter Zijlstra <peterz@infradead.org>
AuthorDate: Mon, 28 Jun 2010 16:51:01 +0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 28 Jun 2010 18:21:30 +0200
init, sched: Fix race between init and kthreadd
Ilya reported that on a very slow machine he could reliably
reproduce a race between forking init and kthreadd. We first
fork init so that it obtains pid-1, however since the scheduler
is already fully running at this point it can preempt and run
the init thread before we spawn and set kthreadd_task.
The init thread can then attempt spawning kthreads without
kthreadd being present which results in an OOPS.
Reported-by: Ilya Loginov <isloginov@gmail.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
LKML-Reference: <1277736661.3561.110.camel@laptop>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
init/main.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/init/main.c b/init/main.c
index 3bdb152..633442f 100644
--- a/init/main.c
+++ b/init/main.c
@@ -422,18 +422,26 @@ static void __init setup_command_line(char *command_line)
* gcc-3.4 accidentally inlines this function, so use noinline.
*/
+static __initdata DECLARE_COMPLETION(kthreadd_done);
+
static noinline void __init_refok rest_init(void)
__releases(kernel_lock)
{
int pid;
rcu_scheduler_starting();
+ /*
+ * We need to spawn init first so that it obtains pid-1, however
+ * the init task will end up wanting to create kthreads, which, if
+ * we schedule it before we create kthreadd, will OOPS.
+ */
kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
numa_default_policy();
pid = kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
rcu_read_lock();
kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns);
rcu_read_unlock();
+ complete(&kthreadd_done);
unlock_kernel();
/*
@@ -855,6 +863,10 @@ static noinline int init_post(void)
static int __init kernel_init(void * unused)
{
+ /*
+ * Wait until kthreadd is all set-up.
+ */
+ wait_for_completion(&kthreadd_done);
lock_kernel();
/*
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] init: Fix race between init and kthreadd -v3
2010-06-28 14:51 ` [PATCH] init: Fix race between init and kthreadd -v3 Peter Zijlstra
` (2 preceding siblings ...)
2010-06-28 20:24 ` [tip:sched/urgent] init, sched: Fix race between init and kthreadd tip-bot for Peter Zijlstra
@ 2010-06-30 8:32 ` Ilya Loginov
2010-06-30 8:37 ` [PATCH] init: Fix comment Peter Zijlstra
3 siblings, 1 reply; 15+ messages in thread
From: Ilya Loginov @ 2010-06-30 8:32 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Ingo Molnar, torvalds, randy.dunlap, linux-kernel
I have tested patch v3. All works fine for me.
I agree with Randy. The commentary should be fixed.
--
Ilya Loginov <isloginov@gmail.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] init: Fix comment
2010-06-30 8:32 ` [PATCH] init: Fix race between init and kthreadd -v3 Ilya Loginov
@ 2010-06-30 8:37 ` Peter Zijlstra
2010-06-30 8:45 ` [tip:sched/urgent] " tip-bot for Peter Zijlstra
0 siblings, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2010-06-30 8:37 UTC (permalink / raw)
To: Ilya Loginov; +Cc: Ingo Molnar, torvalds, randy.dunlap, linux-kernel
On Wed, 2010-06-30 at 12:32 +0400, Ilya Loginov wrote:
> I have tested patch v3. All works fine for me.
>
> I agree with Randy. The commentary should be fixed.
>
By popular request..
---
Subject: init: Fix comment
Apparently "pid-1" confuses people...
Requested-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
init/main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/init/main.c b/init/main.c
index 9eed0f3..4ab5124 100644
--- a/init/main.c
+++ b/init/main.c
@@ -431,7 +431,7 @@ static noinline void __init_refok rest_init(void)
rcu_scheduler_starting();
/*
- * We need to spawn init first so that it obtains pid-1, however
+ * We need to spawn init first so that it obtains pid 1, however
* the init task will end up wanting to create kthreads, which, if
* we schedule it before we create kthreadd, will OOPS.
*/
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [tip:sched/urgent] init: Fix comment
2010-06-30 8:37 ` [PATCH] init: Fix comment Peter Zijlstra
@ 2010-06-30 8:45 ` tip-bot for Peter Zijlstra
0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Peter Zijlstra @ 2010-06-30 8:45 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, a.p.zijlstra, peterz, isloginov,
randy.dunlap, tglx, mingo
Commit-ID: 9715856922bf8475f5428c29b6f4a9eebc97d391
Gitweb: http://git.kernel.org/tip/9715856922bf8475f5428c29b6f4a9eebc97d391
Author: Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 30 Jun 2010 10:37:11 +0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 30 Jun 2010 10:42:44 +0200
init: Fix comment
Apparently "pid-1" confuses people...
Requested-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: torvalds@linux-foundation.org
Cc: randy.dunlap@oracle.com
Cc: Ilya Loginov <isloginov@gmail.com>
LKML-Reference: <1277887031.1868.82.camel@laptop>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
init/main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/init/main.c b/init/main.c
index 633442f..1692df3 100644
--- a/init/main.c
+++ b/init/main.c
@@ -431,7 +431,7 @@ static noinline void __init_refok rest_init(void)
rcu_scheduler_starting();
/*
- * We need to spawn init first so that it obtains pid-1, however
+ * We need to spawn init first so that it obtains pid 1, however
* the init task will end up wanting to create kthreads, which, if
* we schedule it before we create kthreadd, will OOPS.
*/
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2010-06-30 8:46 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-23 20:11 [PATCH] fix problem with reschenduling in rest_init (2.6.35-rc3) Ilya Loginov
2010-06-24 13:11 ` Peter Zijlstra
2010-06-24 13:23 ` Ilya Loginov
2010-06-24 14:08 ` Илья Логинов
2010-06-28 9:01 ` [PATCH] init: Fix race between init and kthreadd Peter Zijlstra
2010-06-28 11:53 ` [PATCH] init: Fix race between init and kthreadd -v2 Peter Zijlstra
2010-06-28 14:19 ` Ingo Molnar
2010-06-28 14:51 ` [PATCH] init: Fix race between init and kthreadd -v3 Peter Zijlstra
2010-06-28 15:03 ` Linus Torvalds
2010-06-28 16:23 ` Randy Dunlap
2010-06-28 20:24 ` [tip:sched/urgent] init, sched: Fix race between init and kthreadd tip-bot for Peter Zijlstra
2010-06-30 8:32 ` [PATCH] init: Fix race between init and kthreadd -v3 Ilya Loginov
2010-06-30 8:37 ` [PATCH] init: Fix comment Peter Zijlstra
2010-06-30 8:45 ` [tip:sched/urgent] " tip-bot for Peter Zijlstra
2010-06-28 20:06 ` [PATCH] init: Fix race between init and kthreadd -v2 Ilya Loginov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.