[KJ] (re) audit return code handling for kernel_thread [0/3]

[KJ] (re) audit return code handling for kernel_thread [0/3]

[Neil Horman]

Hello all-
	I recently posted 11 patches to audit the handling of return codes for
calls made to kernel_thread.  I'd like to rescind those patches, in favor of the
following three, which take into account the feedback I've recieved on them.
Specifically these patches are redistributed to only be three patches rather
than 11, so that each patch fixes a class of problem that I found.  Also the
changes for delayed checks of return codes that don't consider negative returns
now properly check for >= 0 codes.  Finally added printks, all have the same
string format, to save space in the string table.

Thanks & Regards
Neil

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] (re) audit return code handling for kernel_thread [1/3]

[Neil Horman]

Patch to audit return code checking of kernel_thread.  These fixes correct those
callers who fail to check the return code of kernel_thread at all

Thanks & Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 arch/s390/mm/cmm.c           |    6 +++++-
 drivers/macintosh/mediabay.c |    4 +++-
 drivers/s390/net/lcs.c       |   10 +++++++---
 drivers/s390/net/qeth_main.c |   12 +++++++++---
 init/main.c                  |    6 +++++-
 net/bluetooth/rfcomm/core.c  |    6 +++++-
 6 files changed, 34 insertions(+), 10 deletions(-)


--- a/arch/s390/mm/cmm.c
+++ b/arch/s390/mm/cmm.c
@@ -161,7 +161,11 @@ cmm_thread(void *dummy)
 static void
 cmm_start_thread(void)
 {
-	kernel_thread(cmm_thread, NULL, 0);
+	if (kernel_thread(cmm_thread, NULL, 0) < 0) {
+		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+			__FUNCTION__,__LINE__);
+		clear_bit(0,&cmm_thread_active);
+	}
 }
 
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -699,7 +699,9 @@ static int __devinit media_bay_attach(st
 
 	/* Startup kernel thread */
 	if (i = 0)
-		kernel_thread(media_bay_task, NULL, CLONE_KERNEL);
+		if (kernel_thread(media_bay_task, NULL, CLONE_KERNEL) < 0)
+			printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+				__FUNCTION__,__LINE__);
 
 	return 0;
 
--- a/drivers/s390/net/lcs.c
+++ b/drivers/s390/net/lcs.c
@@ -1729,11 +1729,15 @@ lcs_start_kernel_thread(struct lcs_card 
 {
 	LCS_DBF_TEXT(5, trace, "krnthrd");
 	if (lcs_do_start_thread(card, LCS_RECOVERY_THREAD))
-		kernel_thread(lcs_recovery, (void *) card, SIGCHLD);
+		if (kernel_thread(lcs_recovery, (void *) card, SIGCHLD) < 0)
+			printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+				__FUNCTION__, __LINE__);
 #ifdef CONFIG_IP_MULTICAST
 	if (lcs_do_start_thread(card, LCS_SET_MC_THREAD))
-		kernel_thread(lcs_register_mc_addresses,
-				(void *) card, SIGCHLD);
+		if (kernel_thread(lcs_register_mc_addresses,
+				(void *) card, SIGCHLD) < 0)
+			printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+				__FUNCTION__, __LINE__);
 #endif
 }
 
--- a/drivers/s390/net/qeth_main.c
+++ b/drivers/s390/net/qeth_main.c
@@ -1048,11 +1048,17 @@ qeth_start_kernel_thread(struct qeth_car
 		return;
 
 	if (qeth_do_start_thread(card, QETH_SET_IP_THREAD))
-		kernel_thread(qeth_register_ip_addresses, (void *)card,SIGCHLD);
+		if (kernel_thread(qeth_register_ip_addresses, (void *)card,SIGCHLD) < 0)
+			printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+				__FUNCTION__, __LINE__);
 	if (qeth_do_start_thread(card, QETH_SET_PROMISC_MODE_THREAD))
-		kernel_thread(qeth_set_promisc_mode, (void *)card, SIGCHLD);
+		if (kernel_thread(qeth_set_promisc_mode, (void *)card, SIGCHLD) < 0)
+			printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+				__FUNCTION__, __LINE__);
 	if (qeth_do_start_thread(card, QETH_RECOVER_THREAD))
-		kernel_thread(qeth_recover, (void *) card, SIGCHLD);
+		if (kernel_thread(qeth_recover, (void *) card, SIGCHLD) < 0)
+			printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+				__FUNCTION__, __LINE__);
 }
 
 
--- a/init/main.c
+++ b/init/main.c
@@ -389,7 +389,11 @@ #endif
 static void noinline rest_init(void)
 	__releases(kernel_lock)
 {
-	kernel_thread(init, NULL, CLONE_FS | CLONE_SIGHAND);
+	if (kernel_thread(init, NULL, CLONE_FS | CLONE_SIGHAND) < 0) {
+		printk(KERN_CRIT "Unable to start kernel thread at %s:%d\n",
+			__FUNCTION__, __LINE__);
+		BUG();
+	}
 	numa_default_policy();
 	unlock_kernel();
 
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2052,11 +2052,15 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
 /* ---- Initialization ---- */
 static int __init rfcomm_init(void)
 {
+	int ret;
 	l2cap_load();
 
 	hci_register_cb(&rfcomm_cb);
 
-	kernel_thread(rfcomm_run, NULL, CLONE_KERNEL);
+	ret = kernel_thread(rfcomm_run, NULL, CLONE_KERNEL);
+	
+	if (ret < 0)
+		return ret;
 
 	class_create_file(bt_class, &class_attr_rfcomm_dlc);
 

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] (re) audit return code handling for kernel_thread [2/3]

[Neil Horman]

Patch to audit return codes from kernel_thread.  This patch handles cases in
which the return code was stored for later interrogations, but the later checks
assumed that negative return codes were successful.

Thanks 
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 arch/s390/mm/cmm.c             |    6 +++++-
 drivers/macintosh/adb.c        |    7 +++++--
 drivers/macintosh/therm_pm72.c |    5 ++++-
 3 files changed, 14 insertions(+), 4 deletions(-)


--- a/arch/s390/mm/cmm.c
+++ b/arch/s390/mm/cmm.c
@@ -161,7 +161,11 @@ cmm_thread(void *dummy)
 static void
 cmm_start_thread(void)
 {
-	kernel_thread(cmm_thread, NULL, 0);
+	if (kernel_thread(cmm_thread, NULL, 0) < 0) {
+		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+			__FUNCTION__,__LINE__);
+		clear_bit(0,&cmm_thread_active);
+	}
 }
 
 static void
--- a/drivers/macintosh/adb.c
+++ b/drivers/macintosh/adb.c
@@ -137,7 +137,7 @@ #endif
 
 static __inline__ void adb_wait_ms(unsigned int ms)
 {
-	if (current->pid && adb_probe_task_pid &&
+	if (current->pid && (adb_probe_task_pid >= 0) &&
 	  adb_probe_task_pid = current->pid)
 		msleep(ms);
 	else
@@ -270,6 +270,9 @@ static void
 __adb_probe_task(void *data)
 {
 	adb_probe_task_pid = kernel_thread(adb_probe_task, NULL, SIGCHLD | CLONE_KERNEL);
+	if (adb_probe_task_pid < 0)
+		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+			__FUNCTION__,__LINE__);
 }
 
 static DECLARE_WORK(adb_reset_work, __adb_probe_task, NULL);
@@ -494,7 +497,7 @@ adb_request(struct adb_request *req, voi
 	 * block. Beware that the "done" callback will be overriden !
 	 */
 	if ((flags & ADBREQ_SYNC) &&
-	    (current->pid && adb_probe_task_pid &&
+	    (current->pid && (adb_probe_task_pid >= 0) &&
 	    adb_probe_task_pid = current->pid)) {
 		req->done = adb_probe_wakeup;
 		rc = adb_controller->send_request(req, 0);
--- a/drivers/macintosh/therm_pm72.c
+++ b/drivers/macintosh/therm_pm72.c
@@ -1769,6 +1769,9 @@ static void start_control_loops(void)
 	init_completion(&ctrl_complete);
 
 	ctrl_task = kernel_thread(main_control_loop, NULL, SIGCHLD | CLONE_KERNEL);
+	if (ctrl_task < 0)
+		printk(KERN_CRIT "Could not start kernel thread at %s:%d\n",
+			__FUNCTION__, __LINE__);
 }
 
 /*
@@ -1776,7 +1779,7 @@ static void start_control_loops(void)
  */
 static void stop_control_loops(void)
 {
-	if (ctrl_task != 0)
+	if (ctrl_task >= 0)
 		wait_for_completion(&ctrl_complete);
 }
 

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] (re) audit return code handling for kernel_thread [3/3]

[Neil Horman]

Patch to audit return codes for kernel_thread.  This patch corrects callers who
have differing assumptions about the meaning of a zero return code.  I've
normalized all the callers to consider a zero return code a successful return.

Thanks & Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 arch/ia64/sn/kernel/xpc_main.c |    2 +-
 init/do_mounts_initrd.c        |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


--- a/arch/ia64/sn/kernel/xpc_main.c
+++ b/arch/ia64/sn/kernel/xpc_main.c
@@ -583,7 +583,7 @@ xpc_activate_partition(struct xpc_partit
 
 	pid = kernel_thread(xpc_activating, (void *) ((u64) partid), 0);
 
-	if (unlikely(pid <= 0)) {
+	if (unlikely(pid < 0)) {
 		spin_lock_irqsave(&part->act_lock, irq_flags);
 		part->act_state = XPC_P_INACTIVE;
 		XPC_SET_REASON(part, xpcCloneKThreadFailed, __LINE__);
--- a/init/do_mounts_initrd.c
+++ b/init/do_mounts_initrd.c
@@ -57,7 +57,7 @@ static void __init handle_initrd(void)
 
 	current->flags |= PF_NOFREEZE;
 	pid = kernel_thread(do_linuxrc, "/linuxrc", SIGCHLD);
-	if (pid > 0) {
+	if (pid >= 0) {
 		while (pid != sys_wait4(-1, NULL, 0, NULL))
 			yield();
 	}
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] (re) audit return code handling for kernel_thread [1/3]

[Andrew Morton]

On Sat, 29 Jul 2006 16:15:55 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:

> Patch to audit return code checking of kernel_thread.  These fixes correct those
> callers who fail to check the return code of kernel_thread at all
> 
> 

Various people are running around converting open-coded kernel_thread
callers over to the kthread API.  Generally that's a good thing, and error
checking should be incorporated at that time.

So there's probably not a lot of point in making these changes now - it'd
be better to work with the various subsystem owners on doing the kthread
conversion.

> --- a/arch/s390/mm/cmm.c
> +++ b/arch/s390/mm/cmm.c
> @@ -161,7 +161,11 @@ cmm_thread(void *dummy)
>  static void
>  cmm_start_thread(void)
>  {
> -	kernel_thread(cmm_thread, NULL, 0);
> +	if (kernel_thread(cmm_thread, NULL, 0) < 0) {
> +		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
> +			__FUNCTION__,__LINE__);
> +		clear_bit(0,&cmm_thread_active);
> +	}
>  }

This is OK as far as it goes.  But really we should propagate any failure
back up to cmm_init() and fail the whole thing, rather than leaving the
driver hanging around in a loaded-but-useless state.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] (re) audit return code handling for kernel_thread [1/3]

[Neil Horman]

On Sat, Jul 29, 2006 at 05:03:33PM -0700, Andrew Morton wrote:
> On Sat, 29 Jul 2006 16:15:55 -0400
> Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > Patch to audit return code checking of kernel_thread.  These fixes correct those
> > callers who fail to check the return code of kernel_thread at all
> > 
> > 
> 
> Various people are running around converting open-coded kernel_thread
> callers over to the kthread API.  Generally that's a good thing, and error
> checking should be incorporated at that time.
> 
> So there's probably not a lot of point in making these changes now - it'd
> be better to work with the various subsystem owners on doing the kthread
> conversion.
> 
> > --- a/arch/s390/mm/cmm.c
> > +++ b/arch/s390/mm/cmm.c
> > @@ -161,7 +161,11 @@ cmm_thread(void *dummy)
> >  static void
> >  cmm_start_thread(void)
> >  {
> > -	kernel_thread(cmm_thread, NULL, 0);
> > +	if (kernel_thread(cmm_thread, NULL, 0) < 0) {
> > +		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
> > +			__FUNCTION__,__LINE__);
> > +		clear_bit(0,&cmm_thread_active);
> > +	}
> >  }
> 
> This is OK as far as it goes.  But really we should propagate any failure
> back up to cmm_init() and fail the whole thing, rather than leaving the
> driver hanging around in a loaded-but-useless state.


Understood, new patch attached, that removes most of the additional failure to
check return code cases.  I've left the cmm_start_thread case and the
rfcomm_init cases as is, because the cmm_start_thread case is called
asynchronously from a work queue, fired from a timer, meaning we cannot
propogate the error to prevent the module from loading, and the rfcomm_init case
does precisely what you ask, in that it detects a failure to start the kernel
thread, and fails the module load if the thread creation fails.

Thanks & Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 arch/s390/mm/cmm.c          |    6 +++++-
 net/bluetooth/rfcomm/core.c |    6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)


--- a/arch/s390/mm/cmm.c
+++ b/arch/s390/mm/cmm.c
@@ -161,7 +161,11 @@
 static void
 cmm_start_thread(void)
 {
-	kernel_thread(cmm_thread, NULL, 0);
+	if (kernel_thread(cmm_thread, NULL, 0) < 0) {
+		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+			__FUNCTION__,__LINE__);
+		clear_bit(0,&cmm_thread_active);
+	}
 }
 
 
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2052,11 +2052,15 @@
 /* ---- Initialization ---- */
 static int __init rfcomm_init(void)
 {
+	int ret;
 	l2cap_load();
 
 	hci_register_cb(&rfcomm_cb);
 
-	kernel_thread(rfcomm_run, NULL, CLONE_KERNEL);
+	ret = kernel_thread(rfcomm_run, NULL, CLONE_KERNEL);
+	
+	if (ret < 0)
+		return ret;
 
 	class_create_file(bt_class, &class_attr_rfcomm_dlc);
 
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] (re) audit return code handling for kernel_thread [1/3]

[Heiko Carstens]

> > > --- a/arch/s390/mm/cmm.c
> > > +++ b/arch/s390/mm/cmm.c
> > > @@ -161,7 +161,11 @@ cmm_thread(void *dummy)
> > >  static void
> > >  cmm_start_thread(void)
> > >  {
> > > -	kernel_thread(cmm_thread, NULL, 0);
> > > +	if (kernel_thread(cmm_thread, NULL, 0) < 0) {
> > > +		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
> > > +			__FUNCTION__,__LINE__);
> > > +		clear_bit(0,&cmm_thread_active);
> > > +	}
> > >  }
> > 
> > This is OK as far as it goes.  But really we should propagate any failure
> > back up to cmm_init() and fail the whole thing, rather than leaving the
> > driver hanging around in a loaded-but-useless state.
> 
> 
> Understood, new patch attached, that removes most of the additional failure to
> check return code cases.  I've left the cmm_start_thread case and the
> rfcomm_init cases as is, because the cmm_start_thread case is called
> asynchronously from a work queue, fired from a timer, meaning we cannot
> propogate the error to prevent the module from loading, and the rfcomm_init case
> does precisely what you ask, in that it detects a failure to start the kernel
> thread, and fails the module load if the thread creation fails.

The cmm stuff is a bit more broken than that it just doesn't take the return
value of kernel_thread into account. I think this patch would be better:

From: Heiko Carstens <heiko.carstens@de.ibm.com>

[patch] s390: fix cmm kernel thread handling.

Convert cmm's usage of kernel_thread to kthread_create. Also create the
cmmthread at module load time, so it is possible to check if creation of
the thread fails.
In addition the cmmthread now gets terminated when the module gets unloaded
instead of leaving a stale kernel thread that would execute random data
if it ever would run again.
Also check the return values of other registration functions at module load
and handle their return values appropriately.

Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---

 arch/s390/mm/cmm.c |   45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/arch/s390/mm/cmm.c b/arch/s390/mm/cmm.c
index ceea51c..5428689 100644
--- a/arch/s390/mm/cmm.c
+++ b/arch/s390/mm/cmm.c
@@ -15,6 +15,7 @@
 #include <linux/sched.h>
 #include <linux/sysctl.h>
 #include <linux/ctype.h>
+#include <linux/kthread.h>
 
 #include <asm/pgalloc.h>
 #include <asm/uaccess.h>
@@ -44,8 +45,7 @@ static long cmm_timeout_seconds = 0;
 static struct cmm_page_array *cmm_page_list = NULL;
 static struct cmm_page_array *cmm_timed_page_list = NULL;
 
-static unsigned long cmm_thread_active = 0;
-static struct work_struct cmm_thread_starter;
+static struct task_struct *cmm_thread_ptr;
 static wait_queue_head_t cmm_thread_wait;
 static struct timer_list cmm_timer;
 
@@ -124,16 +124,11 @@ cmm_free_pages(long pages, long *counter
 static int
 cmm_thread(void *dummy)
 {
-	int rc;
-
-	daemonize("cmmthread");
 	while (1) {
-		rc = wait_event_interruptible(cmm_thread_wait,
-			(cmm_pages != cmm_pages_target ||
-			 cmm_timed_pages != cmm_timed_pages_target));
-		if (rc = -ERESTARTSYS) {
-			/* Got kill signal. End thread. */
-			clear_bit(0, &cmm_thread_active);
+		wait_event(cmm_thread_wait,
+			   (cmm_pages != cmm_pages_target ||
+			    cmm_timed_pages != cmm_timed_pages_target));
+		if (kthread_should_stop()) {
 			cmm_pages_target = cmm_pages;
 			cmm_timed_pages_target = cmm_timed_pages;
 			break;
@@ -159,16 +154,8 @@ cmm_thread(void *dummy)
 }
 
 static void
-cmm_start_thread(void)
-{
-	kernel_thread(cmm_thread, NULL, 0);
-}
-
-static void
 cmm_kick_thread(void)
 {
-	if (!test_and_set_bit(0, &cmm_thread_active))
-		schedule_work(&cmm_thread_starter);
 	wake_up(&cmm_thread_wait);
 }
 
@@ -412,21 +399,35 @@ struct ctl_table_header *cmm_sysctl_head
 static int
 cmm_init (void)
 {
+	int rc = 0;
+
+	cmm_thread_ptr = kthread_create(cmm_thread, NULL, "cmmthread");
+	if (IS_ERR(cmm_thread_ptr))
+		return PTR_ERR(cmm_thread_ptr);
 #ifdef CONFIG_CMM_PROC
 	cmm_sysctl_header = register_sysctl_table(cmm_dir_table, 1);
+	if (!cmm_sysctl_header) {
+		kthread_stop(cmm_thread_ptr);
+		return -ENOMEM;
+	}
 #endif
 #ifdef CONFIG_CMM_IUCV
-	smsg_register_callback(SMSG_PREFIX, cmm_smsg_target);
+	rc = smsg_register_callback(SMSG_PREFIX, cmm_smsg_target);
+	if (rc < 0) {
+		unregister_sysctl_table(cmm_sysctl_header);
+		kthread_stop(cmm_thread_ptr);
+		return rc;
+	}
 #endif
-	INIT_WORK(&cmm_thread_starter, (void *) cmm_start_thread, NULL);
 	init_waitqueue_head(&cmm_thread_wait);
 	init_timer(&cmm_timer);
-	return 0;
+	return rc;
 }
 
 static void
 cmm_exit(void)
 {
+	kthread_stop(cmm_thread_ptr);
 	cmm_free_pages(cmm_pages, &cmm_pages, &cmm_page_list);
 	cmm_free_pages(cmm_timed_pages, &cmm_timed_pages, &cmm_timed_page_list);
 #ifdef CONFIG_CMM_PROC
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] (re) audit return code handling for kernel_thread [1/3]

[Heiko Carstens]

On Sun, Jul 30, 2006 at 11:39:36PM +0200, Heiko Carstens wrote:
> > > > --- a/arch/s390/mm/cmm.c
> > > > +++ b/arch/s390/mm/cmm.c
> > > > @@ -161,7 +161,11 @@ cmm_thread(void *dummy)
> > > >  static void
> > > >  cmm_start_thread(void)
> > > >  {
> > > > -	kernel_thread(cmm_thread, NULL, 0);
> > > > +	if (kernel_thread(cmm_thread, NULL, 0) < 0) {
> > > > +		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
> > > > +			__FUNCTION__,__LINE__);
> > > > +		clear_bit(0,&cmm_thread_active);
> > > > +	}
> > > >  }
> > > 
> > > This is OK as far as it goes.  But really we should propagate any failure
> > > back up to cmm_init() and fail the whole thing, rather than leaving the
> > > driver hanging around in a loaded-but-useless state.

Updated patch. This time against -mm. And actually it also works unlike
my first patch...

From: Heiko Carstens <heiko.carstens@de.ibm.com>

[patch] s390: fix cmm kernel thread handling.

Convert cmm's usage of kernel_thread to kthread_run. Also create the
cmmthread at module load time, so it is possible to check if creation of
the thread fails.
In addition the cmmthread now gets terminated when the module gets unloaded
instead of leaving a stale kernel thread.
Also check the return values of other registration functions at module load
and handle their return values appropriately.

Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---

Index: linux-2.6.18-rc2-mm1/arch/s390/mm/cmm.c
=================================--- linux-2.6.18-rc2-mm1.orig/arch/s390/mm/cmm.c	2006-07-31 10:27:31.000000000 +0200
+++ linux-2.6.18-rc2-mm1/arch/s390/mm/cmm.c	2006-07-31 11:09:19.000000000 +0200
@@ -16,6 +16,7 @@
 #include <linux/sysctl.h>
 #include <linux/ctype.h>
 #include <linux/swap.h>
+#include <linux/kthread.h>
 
 #include <asm/pgalloc.h>
 #include <asm/uaccess.h>
@@ -46,8 +47,7 @@
 static struct cmm_page_array *cmm_timed_page_list;
 static DEFINE_SPINLOCK(cmm_lock);
 
-static unsigned long cmm_thread_active;
-static struct work_struct cmm_thread_starter;
+static struct task_struct *cmm_thread_ptr;
 static wait_queue_head_t cmm_thread_wait;
 static struct timer_list cmm_timer;
 
@@ -159,14 +159,12 @@
 {
 	int rc;
 
-	daemonize("cmmthread");
 	while (1) {
 		rc = wait_event_interruptible(cmm_thread_wait,
 			(cmm_pages != cmm_pages_target ||
-			 cmm_timed_pages != cmm_timed_pages_target));
-		if (rc = -ERESTARTSYS) {
-			/* Got kill signal. End thread. */
-			clear_bit(0, &cmm_thread_active);
+			 cmm_timed_pages != cmm_timed_pages_target ||
+			 kthread_should_stop()));
+		if (kthread_should_stop() || rc = -ERESTARTSYS) {
 			cmm_pages_target = cmm_pages;
 			cmm_timed_pages_target = cmm_timed_pages;
 			break;
@@ -192,16 +190,8 @@
 }
 
 static void
-cmm_start_thread(void)
-{
-	kernel_thread(cmm_thread, NULL, 0);
-}
-
-static void
 cmm_kick_thread(void)
 {
-	if (!test_and_set_bit(0, &cmm_thread_active))
-		schedule_work(&cmm_thread_starter);
 	wake_up(&cmm_thread_wait);
 }
 
@@ -445,22 +435,48 @@
 static int
 cmm_init (void)
 {
+	int rc = -ENOMEM;
+
 #ifdef CONFIG_CMM_PROC
 	cmm_sysctl_header = register_sysctl_table(cmm_dir_table, 1);
+	if (!cmm_sysctl_header)
+		goto out;
 #endif
 #ifdef CONFIG_CMM_IUCV
-	smsg_register_callback(SMSG_PREFIX, cmm_smsg_target);
+	rc = smsg_register_callback(SMSG_PREFIX, cmm_smsg_target);
+	if (rc < 0)
+		goto out_smsg;
 #endif
-	register_oom_notifier(&cmm_oom_nb);
-	INIT_WORK(&cmm_thread_starter, (void *) cmm_start_thread, NULL);
+	rc = register_oom_notifier(&cmm_oom_nb);
+	if (rc < 0)
+		goto out_oom_notify;
 	init_waitqueue_head(&cmm_thread_wait);
 	init_timer(&cmm_timer);
-	return 0;
+	cmm_thread_ptr = kthread_run(cmm_thread, NULL, "cmmthread");
+	rc = IS_ERR(cmm_thread_ptr) ? PTR_ERR(cmm_thread_ptr) : 0;
+	if (!rc)
+		goto out;
+	/*
+	 * kthread_create failed. undo all the stuff from above again.
+	 */
+	unregister_oom_notifier(&cmm_oom_nb);
+
+out_oom_notify:
+#ifdef CONFIG_CMM_IUCV
+	smsg_unregister_callback(SMSG_PREFIX, cmm_smsg_target);
+out_smsg:
+#endif
+#ifdef CONFIG_CMM_PROC
+	unregister_sysctl_table(cmm_sysctl_header);
+#endif
+out:
+	return rc;
 }
 
 static void
 cmm_exit(void)
 {
+	kthread_stop(cmm_thread_ptr);
 	unregister_oom_notifier(&cmm_oom_nb);
 	cmm_free_pages(cmm_pages, &cmm_pages, &cmm_page_list);
 	cmm_free_pages(cmm_timed_pages, &cmm_timed_pages, &cmm_timed_page_list);
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors