init: Fix race between init and kthreadd -v3
in [Kernel]
|
Prev: USB: EHCI: fix NULL pointer dererence in HCDs that use HCD_LOCAL_MEM
Next: vhost-net: add dhclient work-around from userspace
From: Linus Torvalds on 28 Jun 2010 11:10 On Mon, Jun 28, 2010 at 7:51 AM, Peter Zijlstra <peterz(a)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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Randy Dunlap on 28 Jun 2010 12:30 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(a)gmail.com> > Signed-off-by: Peter Zijlstra <a.p.zijlstra(a)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 *** -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Ilya Loginov on 30 Jun 2010 04:40
I have tested patch v3. All works fine for me. I agree with Randy. The commentary should be fixed. -- Ilya Loginov <isloginov(a)gmail.com> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |