All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: arjan@infradead.org, jeremy@goop.org, mschmidt@redhat.com,
	mingo@redhat.com, hpa@zytor.com, linux-kernel@vger.kernel.org,
	tj@kernel.org, tglx@linutronix.de,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-tip-commits@vger.kernel.org,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Rusty Russell <rusty@rustcorp.com.au>
Subject: [PATCH 1/1] kthreads: simplify !kthreadd_task logic, kill kthreadd_task_init_done
Date: Tue, 1 Sep 2009 18:53:13 +0200	[thread overview]
Message-ID: <20090901165312.GB9105@redhat.com> (raw)
In-Reply-To: <20090901165235.GA9105@redhat.com>

Revert "[PATCH] kthreads: Fix startup synchronization boot crash", it was
the minimal fix for stable.

Then:
	- change kthread_create() to check kthreadd_task != NULL before
	  wake_up_process(kthreadd_task)

	- remove kthreadd_task initialization from rest_init()

	- change kthreadd() to initialize kthreadd_task = current

	- change the main loop in kthreadd() to take kthread_create_lock
	  before the first schedule() (just shift schedule() down)

This way, if kthreadd_task needs the wakeup, kthread_create() must see
kthreadd_task != NULL after unlock(kthread_create_lock).

If kthread_create() sees kthreadd_task == NULL we can just sleep on .done,
kthreadd() must notice the new request before it calls schedule().

IOW both checks, !kthreadd_task and list_empty(), are done after
lock+unlock of the same lock. The 2nd task which takes the lock must
see the changes which were done by the 1st task which locked this lock.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 include/linux/kthread.h |    1 -
 init/main.c             |    7 +------
 kernel/kthread.c        |   35 ++++++++++++++++++-----------------
 3 files changed, 19 insertions(+), 24 deletions(-)

--- WAIT/include/linux/kthread.h~KT_KTHREADD_NULL_FIX	2009-09-01 18:25:50.000000000 +0200
+++ WAIT/include/linux/kthread.h	2009-09-01 18:31:39.000000000 +0200
@@ -33,6 +33,5 @@ int kthread_should_stop(void);
 
 int kthreadd(void *unused);
 extern struct task_struct *kthreadd_task;
-extern struct completion kthreadd_task_init_done;
 
 #endif /* _LINUX_KTHREAD_H */
--- WAIT/init/main.c~KT_KTHREADD_NULL_FIX	2009-09-01 18:25:50.000000000 +0200
+++ WAIT/init/main.c	2009-09-01 18:32:05.000000000 +0200
@@ -453,14 +453,9 @@ static void __init setup_command_line(ch
 static noinline void __init_refok rest_init(void)
 	__releases(kernel_lock)
 {
-	int pid;
-
 	kernel_thread(kernel_init, NULL, CLONE_FS | CLONE_SIGHAND);
 	numa_default_policy();
-	pid = kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
-	kthreadd_task = find_task_by_pid_ns(pid, &init_pid_ns);
-	complete(&kthreadd_task_init_done);
-
+	kernel_thread(kthreadd, NULL, CLONE_FS | CLONE_FILES);
 	unlock_kernel();
 
 	/*
--- WAIT/kernel/kthread.c~KT_KTHREADD_NULL_FIX	2009-09-01 18:25:50.000000000 +0200
+++ WAIT/kernel/kthread.c	2009-09-01 18:32:05.000000000 +0200
@@ -20,9 +20,7 @@
 
 static DEFINE_SPINLOCK(kthread_create_lock);
 static LIST_HEAD(kthread_create_list);
-
 struct task_struct *kthreadd_task;
-DECLARE_COMPLETION(kthreadd_task_init_done);
 
 struct kthread_create_info
 {
@@ -130,11 +128,14 @@ struct task_struct *kthread_create(int (
 	spin_lock(&kthread_create_lock);
 	list_add_tail(&create.list, &kthread_create_list);
 	spin_unlock(&kthread_create_lock);
-
-	if (unlikely(!kthreadd_task))
-		wait_for_completion(&kthreadd_task_init_done);
-
-	wake_up_process(kthreadd_task);
+	/*
+	 * If kthreadd was not created yet, kthreadd() must see the result
+	 * of list_add_tail() later, it takes kthread_create_lock before the
+	 * first schedule(). If kthreadd() locked kthread_create_lock at
+	 * least once, we must see kthreadd_task != NULL.
+	 */
+	if (likely(kthreadd_task))
+		wake_up_process(kthreadd_task);
 	wait_for_completion(&create.done);
 
 	if (!IS_ERR(create.result)) {
@@ -219,23 +220,18 @@ EXPORT_SYMBOL(kthread_stop);
 
 int kthreadd(void *unused)
 {
-	struct task_struct *tsk = current;
+	kthreadd_task = current;
 
 	/* Setup a clean context for our children to inherit. */
-	set_task_comm(tsk, "kthreadd");
-	ignore_signals(tsk);
-	set_user_nice(tsk, KTHREAD_NICE_LEVEL);
-	set_cpus_allowed_ptr(tsk, cpu_all_mask);
+	set_task_comm(kthreadd_task, "kthreadd");
+	ignore_signals(kthreadd_task);
+	set_user_nice(kthreadd_task, KTHREAD_NICE_LEVEL);
+	set_cpus_allowed_ptr(kthreadd_task, cpu_all_mask);
 	set_mems_allowed(node_possible_map);
 
 	current->flags |= PF_NOFREEZE | PF_FREEZER_NOSIG;
 
 	for (;;) {
-		set_current_state(TASK_INTERRUPTIBLE);
-		if (list_empty(&kthread_create_list))
-			schedule();
-		__set_current_state(TASK_RUNNING);
-
 		spin_lock(&kthread_create_lock);
 		while (!list_empty(&kthread_create_list)) {
 			struct kthread_create_info *create;
@@ -250,6 +246,11 @@ int kthreadd(void *unused)
 			spin_lock(&kthread_create_lock);
 		}
 		spin_unlock(&kthread_create_lock);
+
+		set_current_state(TASK_INTERRUPTIBLE);
+		if (list_empty(&kthread_create_list))
+			schedule();
+		__set_current_state(TASK_RUNNING);
 	}
 
 	return 0;


  reply	other threads:[~2009-09-01 16:58 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-29 16:27 [PATCH] x86: detect stack protector for i386 builds on x86_64 Michal Schmidt
2009-08-30 13:39 ` Américo Wang
2009-08-30 18:43 ` [tip:x86/asm] x86: Detect " tip-bot for Michal Schmidt
2009-09-01 10:03   ` Ingo Molnar
2009-09-01 11:39     ` [PATCH] kthreads: Fix startup synchronization boot crash Ingo Molnar
2009-09-01 13:04       ` Oleg Nesterov
2009-09-01 13:14         ` Ingo Molnar
2009-09-01 13:37           ` Oleg Nesterov
2009-09-01 13:59             ` Ingo Molnar
2009-09-01 14:55               ` Oleg Nesterov
2009-09-01 15:54                 ` Ingo Molnar
2009-09-01 16:00                   ` Oleg Nesterov
2009-09-02 13:06                     ` Ingo Molnar
2009-09-01 16:52                 ` [PATCH 0/1] kthreads: simplify !kthreadd_task logic, kill kthreadd_task_init_done Oleg Nesterov
2009-09-01 16:53                   ` Oleg Nesterov [this message]
2009-09-01 23:22                   ` Eric W. Biederman
2009-09-02  9:13                     ` Oleg Nesterov
2009-09-04  7:37                       ` Ingo Molnar
2009-09-18 16:32                         ` Wu Fei
2009-09-18 18:54                           ` Oleg Nesterov
2009-09-18 19:17                             ` Linus Torvalds
2009-09-18 21:12                               ` Oleg Nesterov
2009-09-18 21:15                                 ` Oleg Nesterov
2009-09-18 22:06                                 ` Linus Torvalds
2009-09-18 23:11                                   ` Eric W. Biederman
2009-09-18 23:22                                   ` Oleg Nesterov
2009-09-18 23:38                                     ` Linus Torvalds
2009-09-01 15:08             ` [PATCH] kthreads: Fix startup synchronization boot crash Américo Wang
2009-09-01 15:19               ` Oleg Nesterov
2009-08-31  6:21 ` [PATCH] x86: detect stack protector for i386 builds on x86_64 Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090901165312.GB9105@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@infradead.org \
    --cc=ebiederm@xmission.com \
    --cc=hpa@zytor.com \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mingo@redhat.com \
    --cc=mschmidt@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.