[PATCH] init: Fix race between init and kthreadd -v2

Peter Zijlstra
2010-06-28T11:53:34Z

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();

/*