[PATCH] kthread: convert stop_machine into a kthread

[PATCH] kthread: convert stop_machine into a kthread

[Serge E. Hallyn]

Update stop_machine.c to spawn stop_machine as kthreads rather
than the deprecated kernel_threads.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>

---

 kernel/stop_machine.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

ce04ccc88ac3e2e6c3942fe2c8c4b2d5492d8397
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index dcfb5d7..2dd5a48 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -4,6 +4,7 @@ #include <linux/sched.h>
 #include <linux/cpu.h>
 #include <linux/err.h>
 #include <linux/syscalls.h>
+#include <linux/kthread.h>
 #include <asm/atomic.h>
 #include <asm/semaphore.h>
 #include <asm/uaccess.h>
@@ -96,11 +97,14 @@ static int stop_machine(void)
 	stopmachine_state = STOPMACHINE_WAIT;
 
 	for_each_online_cpu(i) {
+		struct task_struct *tsk;
 		if (i == raw_smp_processor_id())
 			continue;
-		ret = kernel_thread(stopmachine, (void *)(long)i,CLONE_KERNEL);
-		if (ret < 0)
+		tsk = kthread_run(stopmachine, (void *)(long)i, "stopmachine");
+		if (IS_ERR(tsk)) {
+			ret = PTR_ERR(tsk);
 			break;
+		}
 		stopmachine_num_threads++;
 	}
 
-- 
1.3.3

Re: [PATCH] kthread: convert stop_machine into a kthread

[Andrew Morton]

On Thu, 15 Jun 2006 09:43:31 -0500
"Serge E. Hallyn" <serue@us.ibm.com> wrote:

> Update stop_machine.c to spawn stop_machine as kthreads rather
> than the deprecated kernel_threads.
> 
> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> 
> ---
> 
>  kernel/stop_machine.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> ce04ccc88ac3e2e6c3942fe2c8c4b2d5492d8397
> diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
> index dcfb5d7..2dd5a48 100644
> --- a/kernel/stop_machine.c
> +++ b/kernel/stop_machine.c
> @@ -4,6 +4,7 @@ #include <linux/sched.h>
>  #include <linux/cpu.h>
>  #include <linux/err.h>
>  #include <linux/syscalls.h>
> +#include <linux/kthread.h>
>  #include <asm/atomic.h>
>  #include <asm/semaphore.h>
>  #include <asm/uaccess.h>
> @@ -96,11 +97,14 @@ static int stop_machine(void)
>  	stopmachine_state = STOPMACHINE_WAIT;
>  
>  	for_each_online_cpu(i) {
> +		struct task_struct *tsk;
>  		if (i == raw_smp_processor_id())
>  			continue;
> -		ret = kernel_thread(stopmachine, (void *)(long)i,CLONE_KERNEL);
> -		if (ret < 0)
> +		tsk = kthread_run(stopmachine, (void *)(long)i, "stopmachine");
> +		if (IS_ERR(tsk)) {
> +			ret = PTR_ERR(tsk);
>  			break;
> +		}
>  		stopmachine_num_threads++;
>  	}
>  

OK.  But if we're going to convert to the kthread API then stopmachine()
really whould be switched to the more efficient kthread_bind().

RE: [PATCH] kthread: convert stop_machine into a kthread

[Erik Ohrnberger]

unsubscribe linux-kernel

Re: [PATCH] kthread: convert stop_machine into a kthread

[Serge E. Hallyn]

Quoting Andrew Morton (akpm@osdl.org):
> OK.  But if we're going to convert to the kthread API then stopmachine()
> really whould be switched to the more efficient kthread_bind().

Ah, like so?

Rusty, do you feel this makes the conversion less of a step backward?
If not, Andrew, as Rusty pointed out, stop_machine.c does not fall into
the set of kernel_thread users which need to be updated either for the
deprecation or to deal with pid namespaces, and perhaps my previous
patch should not be applied after all.

thanks,
-serge

From: Serge E. Hallyn <serue@us.ibm.com>
Date: Tue, 20 Jun 2006 03:17:44 -0500
Subject: [PATCH] kthread: convert stop_machine to use kthread_bind

Convert stop_machine to use the more efficient kthread_bind()
in place of set_cpus_allowed().

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>

---

 kernel/stop_machine.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

831c955bcc8572f0aea75ab608ed5da37680df4e
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index 2dd5a48..a462deb 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -31,7 +31,7 @@ static int stopmachine(void *cpu)
 	int irqs_disabled = 0;
 	int prepared = 0;
 
-	set_cpus_allowed(current, cpumask_of_cpu((int)(long)cpu));
+	kthread_bind(current, (unsigned int)(long)cpu);
 
 	/* Ack: we are alive */
 	smp_mb(); /* Theoretically the ack = 0 might not be on this CPU yet. */
-- 
1.3.3

Re: [PATCH] kthread: convert stop_machine into a kthread

[Andrew Morton]

On Tue, 20 Jun 2006 03:27:45 -0500
"Serge E. Hallyn" <serue@us.ibm.com> wrote:

> Ah, like so?

Nope.  kthread_bind() is supposed to be called by the thread creator,
before the thread is started.

The documentation for kthread_bind() is irritatingly hidden in the header
file.

Re: [PATCH] kthread: convert stop_machine into a kthread

[Serge E. Hallyn]

Quoting Andrew Morton (akpm@osdl.org):
> On Tue, 20 Jun 2006 03:27:45 -0500
> "Serge E. Hallyn" <serue@us.ibm.com> wrote:
> 
> > Ah, like so?
> 
> Nope.  kthread_bind() is supposed to be called by the thread creator,
> before the thread is started.
> 
> The documentation for kthread_bind() is irritatingly hidden in the header
> file.

Oh, I see - then it makes more sense that it gets away with being so
much simpler than set_cpus_allowed().

So here's another attempt.  However I'm not sure now whether
the first round of synchronization around stopmachine_thread_ack
is necessary anymore.  If any threads fail, we'll find out from the
kthread_create() return value, right?

Still I'm not sure about that, so first things first:

thanks,
-serge

From: Serge E. Hallyn <serue@us.ibm.com>
Date: Tue, 20 Jun 2006 11:01:08 -0500
Subject: [PATCH] kthread: update stop_machine to use kthread_bind

Update stop_machine to use the more efficient kthread_bind()
before running task in place of set_cpus_allowed() after.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>

---

 kernel/stop_machine.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

e25a88e3d60f3f139f10cc8cd894d87622033a16
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index 2dd5a48..593d8e4 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -86,7 +86,8 @@ static void stopmachine_set_state(enum s
 
 static int stop_machine(void)
 {
-	int i, ret = 0;
+	int ret = 0;
+	unsigned int i;
 	struct sched_param param = { .sched_priority = MAX_RT_PRIO-1 };
 
 	/* One high-prio thread per cpu.  We'll do this one. */
@@ -100,11 +101,13 @@ static int stop_machine(void)
 		struct task_struct *tsk;
 		if (i == raw_smp_processor_id())
 			continue;
-		tsk = kthread_run(stopmachine, (void *)(long)i, "stopmachine");
+		tsk = kthread_create(stopmachine, NULL, "stopmachine");
 		if (IS_ERR(tsk)) {
 			ret = PTR_ERR(tsk);
 			break;
 		}
+		kthread_bind(tsk, i);
+		wake_up_process(tsk);
 		stopmachine_num_threads++;
 	}
 
-- 
1.3.3

Re: [PATCH] kthread: convert stop_machine into a kthread

[Andrew Morton]

"Serge E. Hallyn" <serue@us.ibm.com> wrote:
>
> Quoting Andrew Morton (akpm@osdl.org):
> > On Tue, 20 Jun 2006 03:27:45 -0500
> > "Serge E. Hallyn" <serue@us.ibm.com> wrote:
> > 
> > > Ah, like so?
> > 
> > Nope.  kthread_bind() is supposed to be called by the thread creator,
> > before the thread is started.
> > 
> > The documentation for kthread_bind() is irritatingly hidden in the header
> > file.
> 
> Oh, I see - then it makes more sense that it gets away with being so
> much simpler than set_cpus_allowed().
> 
> So here's another attempt.  However I'm not sure now whether
> the first round of synchronization around stopmachine_thread_ack
> is necessary anymore.  If any threads fail, we'll find out from the
> kthread_create() return value, right?
> 
> Still I'm not sure about that, so first things first:
> 
> thanks,
> -serge
> 
> From: Serge E. Hallyn <serue@us.ibm.com>
> Date: Tue, 20 Jun 2006 11:01:08 -0500
> Subject: [PATCH] kthread: update stop_machine to use kthread_bind
> 
> Update stop_machine to use the more efficient kthread_bind()
> before running task in place of set_cpus_allowed() after.
> 
> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> 
> ---
> 
>  kernel/stop_machine.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> e25a88e3d60f3f139f10cc8cd894d87622033a16
> diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
> index 2dd5a48..593d8e4 100644
> --- a/kernel/stop_machine.c
> +++ b/kernel/stop_machine.c
> @@ -86,7 +86,8 @@ static void stopmachine_set_state(enum s
>  
>  static int stop_machine(void)
>  {
> -	int i, ret = 0;
> +	int ret = 0;
> +	unsigned int i;
>  	struct sched_param param = { .sched_priority = MAX_RT_PRIO-1 };
>  
>  	/* One high-prio thread per cpu.  We'll do this one. */
> @@ -100,11 +101,13 @@ static int stop_machine(void)
>  		struct task_struct *tsk;
>  		if (i == raw_smp_processor_id())
>  			continue;
> -		tsk = kthread_run(stopmachine, (void *)(long)i, "stopmachine");
> +		tsk = kthread_create(stopmachine, NULL, "stopmachine");
>  		if (IS_ERR(tsk)) {
>  			ret = PTR_ERR(tsk);
>  			break;
>  		}
> +		kthread_bind(tsk, i);
> +		wake_up_process(tsk);
>  		stopmachine_num_threads++;
>  	}
>  

You forgot this rather important bit:

--- 25/kernel/stop_machine.c~kthread-convert-stop_machine-into-a-kthread-update-fix	Tue Jun 20 15:39:30 2006
+++ 25-akpm/kernel/stop_machine.c	Tue Jun 20 15:39:30 2006
@@ -26,13 +26,11 @@ static unsigned int stopmachine_num_thre
 static atomic_t stopmachine_thread_ack;
 static DECLARE_MUTEX(stopmachine_mutex);
 
-static int stopmachine(void *cpu)
+static int stopmachine(void *unused)
 {
 	int irqs_disabled = 0;
 	int prepared = 0;
 
-	set_cpus_allowed(current, cpumask_of_cpu((int)(long)cpu));
-
 	/* Ack: we are alive */
 	smp_mb(); /* Theoretically the ack = 0 might not be on this CPU yet. */
 	atomic_inc(&stopmachine_thread_ack);
_


Without that change, every thread will run on cpu 0, and the whole
stopmachine thing is, presumably, busted.

I'll fold all three patches together and send 'em back to you for a bit of
runtime testing, OK?

Re: [PATCH] kthread: convert stop_machine into a kthread

[Serge E. Hallyn]

Quoting Andrew Morton (akpm@osdl.org):
> "Serge E. Hallyn" <serue@us.ibm.com> wrote:
> >
> > Quoting Andrew Morton (akpm@osdl.org):
> > > On Tue, 20 Jun 2006 03:27:45 -0500
> > > "Serge E. Hallyn" <serue@us.ibm.com> wrote:
> > > 
> > > > Ah, like so?
> > > 
> > > Nope.  kthread_bind() is supposed to be called by the thread creator,
> > > before the thread is started.
> > > 
> > > The documentation for kthread_bind() is irritatingly hidden in the header
> > > file.
> > 
> > Oh, I see - then it makes more sense that it gets away with being so
> > much simpler than set_cpus_allowed().
> > 
> > So here's another attempt.  However I'm not sure now whether
> > the first round of synchronization around stopmachine_thread_ack
> > is necessary anymore.  If any threads fail, we'll find out from the
> > kthread_create() return value, right?
> > 
> > Still I'm not sure about that, so first things first:
> > 
> > thanks,
> > -serge
> > 
> > From: Serge E. Hallyn <serue@us.ibm.com>
> > Date: Tue, 20 Jun 2006 11:01:08 -0500
> > Subject: [PATCH] kthread: update stop_machine to use kthread_bind
> > 
> > Update stop_machine to use the more efficient kthread_bind()
> > before running task in place of set_cpus_allowed() after.
> > 
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > 
> > ---
> > 
> >  kernel/stop_machine.c |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > e25a88e3d60f3f139f10cc8cd894d87622033a16
> > diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
> > index 2dd5a48..593d8e4 100644
> > --- a/kernel/stop_machine.c
> > +++ b/kernel/stop_machine.c
> > @@ -86,7 +86,8 @@ static void stopmachine_set_state(enum s
> >  
> >  static int stop_machine(void)
> >  {
> > -	int i, ret = 0;
> > +	int ret = 0;
> > +	unsigned int i;
> >  	struct sched_param param = { .sched_priority = MAX_RT_PRIO-1 };
> >  
> >  	/* One high-prio thread per cpu.  We'll do this one. */
> > @@ -100,11 +101,13 @@ static int stop_machine(void)
> >  		struct task_struct *tsk;
> >  		if (i == raw_smp_processor_id())
> >  			continue;
> > -		tsk = kthread_run(stopmachine, (void *)(long)i, "stopmachine");
> > +		tsk = kthread_create(stopmachine, NULL, "stopmachine");
> >  		if (IS_ERR(tsk)) {
> >  			ret = PTR_ERR(tsk);
> >  			break;
> >  		}
> > +		kthread_bind(tsk, i);
> > +		wake_up_process(tsk);
> >  		stopmachine_num_threads++;
> >  	}
> >  
> 
> You forgot this rather important bit:

Confounded, the only thing I can figure is I must not have done a
git-format-patch to regenerate the patch before I pasted it.

I see it's all sorted out in -mm, but just for the record here's
the patch I meant to send.

thanks,
-serge

>From nobody Mon Sep 17 00:00:00 2001
From: Serge E. Hallyn <hallyn@sergelap.(none)>
Date: Tue, 20 Jun 2006 11:01:08 -0500
Subject: [PATCH] kthread: update stop_machine to use kthread_bind

Update stop_machine to use the more efficient kthread_bind()
before running task in place of set_cpus_allowed() after.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>

---

 kernel/stop_machine.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

fe140b494141aefae45a4fa12a4b4e8fef5d8ee3
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index 2dd5a48..698b842 100644
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -26,15 +26,13 @@ static unsigned int stopmachine_num_thre
 static atomic_t stopmachine_thread_ack;
 static DECLARE_MUTEX(stopmachine_mutex);
 
-static int stopmachine(void *cpu)
+static int stopmachine(void)
 {
 	int irqs_disabled = 0;
 	int prepared = 0;
 
-	set_cpus_allowed(current, cpumask_of_cpu((int)(long)cpu));
-
 	/* Ack: we are alive */
-	smp_mb(); /* Theoretically the ack = 0 might not be on this CPU yet. */
+	smp_mb();
 	atomic_inc(&stopmachine_thread_ack);
 
 	/* Simple state machine */
@@ -86,7 +84,8 @@ static void stopmachine_set_state(enum s
 
 static int stop_machine(void)
 {
-	int i, ret = 0;
+	int ret = 0;
+	unsigned int i;
 	struct sched_param param = { .sched_priority = MAX_RT_PRIO-1 };
 
 	/* One high-prio thread per cpu.  We'll do this one. */
@@ -100,11 +99,13 @@ static int stop_machine(void)
 		struct task_struct *tsk;
 		if (i == raw_smp_processor_id())
 			continue;
-		tsk = kthread_run(stopmachine, (void *)(long)i, "stopmachine");
+		tsk = kthread_create(stopmachine, NULL, "stopmachine");
 		if (IS_ERR(tsk)) {
 			ret = PTR_ERR(tsk);
 			break;
 		}
+		kthread_bind(tsk, i);
+		wake_up_process(tsk);
 		stopmachine_num_threads++;
 	}
 
-- 
1.3.3

Re: [PATCH] kthread: convert stop_machine into a kthread

[Andrew Morton]

On Tue, 20 Jun 2006 19:52:06 -0500
"Serge E. Hallyn" <serue@us.ibm.com> wrote:

> here's
> the patch I meant to send.
> 
> ...
>
> -static int stopmachine(void *cpu)
> +static int stopmachine(void)
> ...
> -		tsk = kthread_run(stopmachine, (void *)(long)i, "stopmachine");
> +		tsk = kthread_create(stopmachine, NULL, "stopmachine");

This should have spat a compiler warning.

The confidence level on all of this ain't high.  Please, test the patch
which I merged?

Re: [PATCH] kthread: convert stop_machine into a kthread

[Serge E. Hallyn]

Quoting Andrew Morton (akpm@osdl.org):
> On Tue, 20 Jun 2006 19:52:06 -0500
> "Serge E. Hallyn" <serue@us.ibm.com> wrote:
> 
> > here's
> > the patch I meant to send.
> > 
> > ...
> >
> > -static int stopmachine(void *cpu)
> > +static int stopmachine(void)
> > ...
> > -		tsk = kthread_run(stopmachine, (void *)(long)i, "stopmachine");
> > +		tsk = kthread_create(stopmachine, NULL, "stopmachine");
> 
> This should have spat a compiler warning.
> 
> The confidence level on all of this ain't high.  Please, test the patch
> which I merged?

Compiles, boots, and shuts down on s390.

thanks,
-serge

[PATCH] kthread: move kernel-doc and put it into DocBook

[Randy.Dunlap]

From: Randy Dunlap <rdunlap@xenotime.net>

> The documentation for kthread_bind() is irritatingly hidden in the header
> file.

Move kthread API kernel-doc from kthread.h to kthread.c & fix it.
Add kthread API to kernel-api DocBook.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 Documentation/DocBook/kernel-api.tmpl |    2 +
 include/linux/kthread.h               |   65 +---------------------------------
 kernel/kthread.c                      |   61 +++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 63 deletions(-)

--- linux-2617-pv.orig/include/linux/kthread.h
+++ linux-2617-pv/include/linux/kthread.h
@@ -4,37 +4,19 @@
 #include <linux/err.h>
 #include <linux/sched.h>
 
-/**
- * kthread_create: create a kthread.
- * @threadfn: the function to run until signal_pending(current).
- * @data: data ptr for @threadfn.
- * @namefmt: printf-style name for the thread.
- *
- * Description: This helper function creates and names a kernel
- * thread.  The thread will be stopped: use wake_up_process() to start
- * it.  See also kthread_run(), kthread_create_on_cpu().
- *
- * When woken, the thread will run @threadfn() with @data as its
- * argument. @threadfn can either call do_exit() directly if it is a
- * standalone thread for which noone will call kthread_stop(), or
- * return when 'kthread_should_stop()' is true (which means
- * kthread_stop() has been called).  The return value should be zero
- * or a negative error number: it will be passed to kthread_stop().
- *
- * Returns a task_struct or ERR_PTR(-ENOMEM).
- */
 struct task_struct *kthread_create(int (*threadfn)(void *data),
 				   void *data,
 				   const char namefmt[], ...);
 
 /**
- * kthread_run: create and wake a thread.
+ * kthread_run - create and wake a thread.
  * @threadfn: the function to run until signal_pending(current).
  * @data: data ptr for @threadfn.
  * @namefmt: printf-style name for the thread.
  *
  * Description: Convenient wrapper for kthread_create() followed by
- * wake_up_process().  Returns the kthread, or ERR_PTR(-ENOMEM). */
+ * wake_up_process().  Returns the kthread or ERR_PTR(-ENOMEM).
+ */
 #define kthread_run(threadfn, data, namefmt, ...)			   \
 ({									   \
 	struct task_struct *__k						   \
@@ -44,50 +26,9 @@ struct task_struct *kthread_create(int (
 	__k;								   \
 })
 
-/**
- * kthread_bind: bind a just-created kthread to a cpu.
- * @k: thread created by kthread_create().
- * @cpu: cpu (might not be online, must be possible) for @k to run on.
- *
- * Description: This function is equivalent to set_cpus_allowed(),
- * except that @cpu doesn't need to be online, and the thread must be
- * stopped (ie. just returned from kthread_create().
- */
 void kthread_bind(struct task_struct *k, unsigned int cpu);
-
-/**
- * kthread_stop: stop a thread created by kthread_create().
- * @k: thread created by kthread_create().
- *
- * Sets kthread_should_stop() for @k to return true, wakes it, and
- * waits for it to exit.  Your threadfn() must not call do_exit()
- * itself if you use this function!  This can also be called after
- * kthread_create() instead of calling wake_up_process(): the thread
- * will exit without calling threadfn().
- *
- * Returns the result of threadfn(), or -EINTR if wake_up_process()
- * was never called. */
 int kthread_stop(struct task_struct *k);
-
-/**
- * kthread_stop_sem: stop a thread created by kthread_create().
- * @k: thread created by kthread_create().
- * @s: semaphore that @k waits on while idle.
- *
- * Does essentially the same thing as kthread_stop() above, but wakes
- * @k by calling up(@s).
- *
- * Returns the result of threadfn(), or -EINTR if wake_up_process()
- * was never called. */
 int kthread_stop_sem(struct task_struct *k, struct semaphore *s);
-
-/**
- * kthread_should_stop: should this kthread return now?
- *
- * When someone calls kthread_stop on your kthread, it will be woken
- * and this will return true.  You should then return, and your return
- * value will be passed through to kthread_stop().
- */
 int kthread_should_stop(void);
 
 #endif /* _LINUX_KTHREAD_H */
--- linux-2617-pv.orig/kernel/kthread.c
+++ linux-2617-pv/kernel/kthread.c
@@ -45,6 +45,13 @@ struct kthread_stop_info
 static DEFINE_MUTEX(kthread_stop_lock);
 static struct kthread_stop_info kthread_stop_info;
 
+/**
+ * kthread_should_stop - should this kthread return now?
+ *
+ * When someone calls kthread_stop on your kthread, it will be woken
+ * and this will return true.  You should then return, and your return
+ * value will be passed through to kthread_stop().
+ */
 int kthread_should_stop(void)
 {
 	return (kthread_stop_info.k == current);
@@ -122,6 +129,25 @@ static void keventd_create_kthread(void 
 	complete(&create->done);
 }
 
+/**
+ * kthread_create - create a kthread.
+ * @threadfn: the function to run until signal_pending(current).
+ * @data: data ptr for @threadfn.
+ * @namefmt: printf-style name for the thread.
+ *
+ * Description: This helper function creates and names a kernel
+ * thread.  The thread will be stopped: use wake_up_process() to start
+ * it.  See also kthread_run(), kthread_create_on_cpu().
+ *
+ * When woken, the thread will run @threadfn() with @data as its
+ * argument. @threadfn can either call do_exit() directly if it is a
+ * standalone thread for which noone will call kthread_stop(), or
+ * return when 'kthread_should_stop()' is true (which means
+ * kthread_stop() has been called).  The return value should be zero
+ * or a negative error number; it will be passed to kthread_stop().
+ *
+ * Returns a task_struct or ERR_PTR(-ENOMEM).
+ */
 struct task_struct *kthread_create(int (*threadfn)(void *data),
 				   void *data,
 				   const char namefmt[],
@@ -156,6 +182,15 @@ struct task_struct *kthread_create(int (
 }
 EXPORT_SYMBOL(kthread_create);
 
+/**
+ * kthread_bind - bind a just-created kthread to a cpu.
+ * @k: thread created by kthread_create().
+ * @cpu: cpu (might not be online, must be possible) for @k to run on.
+ *
+ * Description: This function is equivalent to set_cpus_allowed(),
+ * except that @cpu doesn't need to be online, and the thread must be
+ * stopped (i.e., just returned from kthread_create().
+ */
 void kthread_bind(struct task_struct *k, unsigned int cpu)
 {
 	BUG_ON(k->state != TASK_INTERRUPTIBLE);
@@ -166,12 +201,36 @@ void kthread_bind(struct task_struct *k,
 }
 EXPORT_SYMBOL(kthread_bind);
 
+/**
+ * kthread_stop - stop a thread created by kthread_create().
+ * @k: thread created by kthread_create().
+ *
+ * Sets kthread_should_stop() for @k to return true, wakes it, and
+ * waits for it to exit.  Your threadfn() must not call do_exit()
+ * itself if you use this function!  This can also be called after
+ * kthread_create() instead of calling wake_up_process(): the thread
+ * will exit without calling threadfn().
+ *
+ * Returns the result of threadfn(), or %-EINTR if wake_up_process()
+ * was never called.
+ */
 int kthread_stop(struct task_struct *k)
 {
 	return kthread_stop_sem(k, NULL);
 }
 EXPORT_SYMBOL(kthread_stop);
 
+/**
+ * kthread_stop_sem - stop a thread created by kthread_create().
+ * @k: thread created by kthread_create().
+ * @s: semaphore that @k waits on while idle.
+ *
+ * Does essentially the same thing as kthread_stop() above, but wakes
+ * @k by calling up(@s).
+ *
+ * Returns the result of threadfn(), or %-EINTR if wake_up_process()
+ * was never called.
+ */
 int kthread_stop_sem(struct task_struct *k, struct semaphore *s)
 {
 	int ret;
@@ -210,5 +269,5 @@ static __init int helper_init(void)
 
 	return 0;
 }
-core_initcall(helper_init);
 
+core_initcall(helper_init);
--- linux-2617-pv.orig/Documentation/DocBook/kernel-api.tmpl
+++ linux-2617-pv/Documentation/DocBook/kernel-api.tmpl
@@ -62,6 +62,8 @@
      <sect1><title>Internal Functions</title>
 !Ikernel/exit.c
 !Ikernel/signal.c
+!Iinclude/linux/kthread.h
+!Ekernel/kthread.c
      </sect1>
 
      <sect1><title>Kernel objects manipulation</title>

---

Re: [PATCH] kthread: convert stop_machine into a kthread

[Rusty Russell]

On Fri, 2006-06-16 at 08:20 +1000, Paul Mackerras wrote:
> Update stop_machine.c to spawn stop_machine as kthreads rather
> than the deprecated kernel_threads.
> 
> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>

kthread does a lot of stuff we don't need, and stop_machine is fairly
special, low-level interface.

Seems like change for change's sake, to me, *and* it added more code
than it removed.

Rusty.
-- 
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law

Re: [PATCH] kthread: convert stop_machine into a kthread

[Serge E. Hallyn]

Quoting Rusty Russell (rusty@rustcorp.com.au):
> On Fri, 2006-06-16 at 08:20 +1000, Paul Mackerras wrote:
> > Update stop_machine.c to spawn stop_machine as kthreads rather
> > than the deprecated kernel_threads.
> > 
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> 
> kthread does a lot of stuff we don't need, and stop_machine is fairly
> special, low-level interface.
> 
> Seems like change for change's sake, to me, *and* it added more code
> than it removed.

So if kernel_thread is really going to be removed, how else should this
be done?  Just clone?

-serge

Re: [PATCH] kthread: convert stop_machine into a kthread

[Paul Mackerras]

Serge E. Hallyn writes:

> So if kernel_thread is really going to be removed, how else should this
> be done?  Just clone?

Who said kernel_thread was going to be removed?  kthread *uses*
kernel_thread.  kthread is a helper to make life easier for drivers,
not a replacement for kernel_thread.

Paul.

Re: [PATCH] kthread: convert stop_machine into a kthread

[Rusty Russell]

On Thu, 2006-06-15 at 22:04 -0500, Serge E. Hallyn wrote:
> Quoting Rusty Russell (rusty@rustcorp.com.au):
> > Seems like change for change's sake, to me, *and* it added more code
> > than it removed.
> 
> So if kernel_thread is really going to be removed, how else should this
> be done?  Just clone?

Sorry, is kernel_thread going to be removed, or just not exported to
modules?  What's kthread going to use?

Confused,
Rusty.
-- 
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law

Re: [PATCH] kthread: convert stop_machine into a kthread

[Serge E. Hallyn]

Quoting Rusty Russell (rusty@rustcorp.com.au):
> On Thu, 2006-06-15 at 22:04 -0500, Serge E. Hallyn wrote:
> > Quoting Rusty Russell (rusty@rustcorp.com.au):
> > > Seems like change for change's sake, to me, *and* it added more code
> > > than it removed.
> > 
> > So if kernel_thread is really going to be removed, how else should this
> > be done?  Just clone?
> 
> Sorry, is kernel_thread going to be removed, or just not exported to
> modules?  What's kthread going to use?
> 
> Confused,
> Rusty.

Hah.

Yes, I see.  I misread.  So I should be focusing on modules  :)

Really, all *I* care about is cases where the resulting pid is cached
as a pointer to the thread, which it wasn't here anyway.  

thanks, sorry for the noise.

-serge

Re: [PATCH] kthread: convert stop_machine into a kthread

[Eric W. Biederman]

"Serge E. Hallyn" <serue@us.ibm.com> writes:

> Quoting Rusty Russell (rusty@rustcorp.com.au):
>> On Thu, 2006-06-15 at 22:04 -0500, Serge E. Hallyn wrote:
>> > Quoting Rusty Russell (rusty@rustcorp.com.au):
>> > > Seems like change for change's sake, to me, *and* it added more code
>> > > than it removed.
>> > 
>> > So if kernel_thread is really going to be removed, how else should this
>> > be done?  Just clone?
>> 
>> Sorry, is kernel_thread going to be removed, or just not exported to
>> modules?  What's kthread going to use?
>> 
>> Confused,
>> Rusty.
>
> Hah.
>
> Yes, I see.  I misread.  So I should be focusing on modules  :)
>
> Really, all *I* care about is cases where the resulting pid is cached
> as a pointer to the thread, which it wasn't here anyway.  

There is one other piece we care about as well.

We don't want to capture user space context like a non-default fs namespace
in a kernel thread as well.  Since the kthread api calls kernel_thread
from keventd non of the threads that it spawns can capture any user
space context, by accident.  There was a very nasty bug a while ago
when the fs namespace was captured by a kernel thread and then it was
impossible to unmount your filesystem because no one had access to
that filesystem mount tree.

In this instance the kstopmachine is stared using the kthread api so
it is safe, and then forking children is safe as well. 

Once everything that we can convert to the kthread api is converted we
need to audit all of the remaining kernel_thread instances to ensure
they don't capture user space context.

The basic rule is that is only safe to use kernel_thread directly if
it is coming from another kernel thread.

So while the conversion was overkill in this context and we certainly
want to concentrate on modules, where it is much less likely to be
correct.  We want to convert as many instances as we can away from
the raw kernel_thread api.

All that is ultimately going away is the export of kernel_thread to
modules, because there are so very few instances when using
kernel_thread directly can be justified.

Eric