* [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
* [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
* 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox