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

KJ Neil Horman 3 participants 8 messages Jul 29, 2006 – Jul 31, 2006

Subsystems (8)

Network drivers VFS Kernel janitors s390 +4 more subsystems

lore

Share & tools

lore

Neil Horman

author

2006-07-29 20:11 UTC #0 lore raw

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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Neil Horman

author

2006-07-29 20:15 UTC #1 lore raw

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

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 <[email protected]>

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

arch/s390/mm/cmm.c +9 -3

@@ -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;

drivers/s390/net/lcs.c +7 -3

@@ -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
 }

drivers/s390/net/qeth_main.c +9 -3

@@ -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__);
 }

init/main.c +5 -1

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

net/bluetooth/rfcomm/core.c +5 -1

@@ -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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Andrew Morton

2006-07-30 00:03 UTC #2 lore raw

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

Quoted 4 lines · On Sat, 29 Jul 2006 16:15:55 -0400 Neil Horman <[email protected]> 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.

arch/s390/mm/cmm.c cmm_thread(void *dummy) · line 169

Quoted 13 lines

arch/s390/mm/cmm.c +5 -1

@@ -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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Neil Horman

author

2006-07-30 00:48 UTC #3 lore raw

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

Quoted 25 lines · Andrew Morton

Quoted 4 lines · On Sat, 29 Jul 2006 16:15:55 -0400 Neil Horman <[email protected]> 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.
Quoted 9 lines
arch/s390/mm/cmm.c +2 -1
@@ -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",

Quoted 8 lines

arch/s390/mm/cmm.c cmm_thread(void *dummy) · line 169
Quoted 4 lines
arch/s390/mm/cmm.c +3 -0
+			__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 <[email protected]>

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

arch/s390/mm/cmm.c +5 -1

@@ -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);
+	}
 }

net/bluetooth/rfcomm/core.c +5 -1

@@ -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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Heiko Carstens

2006-07-30 21:39 UTC #4 lore raw

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

Quoted 19 lines

Quoted 9 lines
Quoted 9 lines
arch/s390/mm/cmm.c +2 -1
@@ -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",
Quoted 8 lines
arch/s390/mm/cmm.c cmm_thread(void *dummy) · line 169
Quoted 4 lines
arch/s390/mm/cmm.c +3 -0
+			__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.

Quoted 7 lines

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 <[email protected]>

[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 <[email protected]>

Signed-off-by Heiko Carstens <[email protected]>

---

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

arch/s390/mm/cmm.c +23 ~~-22~~

index ceea51c..5428689 100644
@@ -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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Heiko Carstens

2006-07-31 09:17 UTC #5 lore raw

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

Quoted 9 lines · Heiko Carstens

Quoted 9 lines

arch/s390/mm/cmm.c +2 -1

@@ -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",

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
arch/s390/mm/cmm.c cmm_thread(void *dummy) · line 169
Quoted 4 lines
arch/s390/mm/cmm.c +3 -0
+			__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 <[email protected]>

[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 <[email protected]>

Signed-off-by Heiko Carstens <[email protected]>

---

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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Neil Horman

author

2006-07-29 20:18 UTC #6 lore raw

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

In reply to 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 <[email protected]>

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

arch/s390/mm/cmm.c +5 -1

@@ -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

drivers/macintosh/adb.c +5 -2

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

drivers/macintosh/therm_pm72.c +4 -1

@@ -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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Neil Horman

author

2006-07-29 20:20 UTC #7 lore raw

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

In reply to 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 <[email protected]>

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

arch/ia64/sn/kernel/xpc_main.c +1 -1

@@ -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__);

init/do_mounts_initrd.c +1 -1

@@ -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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors