Messages in this thread Patch in this message | | | Subject | [PATCH] init: Fix race between init and kthreadd | From | Peter Zijlstra <> | Date | Mon, 28 Jun 2010 11:01:29 +0200 |
| |
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(); /*
| |