* [KJ] (re) audit return code handling for kernel_thread [0/3]
@ 2006-07-29 20:11 Neil Horman
2006-07-29 20:15 ` [KJ] (re) audit return code handling for kernel_thread [1/3] Neil Horman
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Neil Horman @ 2006-07-29 20:11 UTC (permalink / raw)
To: kernel-janitors, linux-kernel, nhorman, torvalds, akpm, marcel,
fpavlic, paulus, bcollins, tony.luck
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [KJ] (re) audit return code handling for kernel_thread [1/3] 2006-07-29 20:11 [KJ] (re) audit return code handling for kernel_thread [0/3] Neil Horman @ 2006-07-29 20:15 ` Neil Horman 2006-07-30 0:03 ` Andrew Morton 2006-07-29 20:18 ` [KJ] (re) audit return code handling for kernel_thread [2/3] Neil Horman 2006-07-29 20:20 ` [KJ] (re) audit return code handling for kernel_thread [3/3] Neil Horman 2 siblings, 1 reply; 8+ messages in thread From: Neil Horman @ 2006-07-29 20:15 UTC (permalink / raw) To: kernel-janitors, linux-kernel, torvalds, akpm, marcel, fpavlic, paulus, bcollins, tony.luck 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [KJ] (re) audit return code handling for kernel_thread [1/3] 2006-07-29 20:15 ` [KJ] (re) audit return code handling for kernel_thread [1/3] Neil Horman @ 2006-07-30 0:03 ` Andrew Morton 2006-07-30 0:48 ` Neil Horman 0 siblings, 1 reply; 8+ messages in thread From: Andrew Morton @ 2006-07-30 0:03 UTC (permalink / raw) To: Neil Horman Cc: kernel-janitors, linux-kernel, torvalds, marcel, fpavlic, paulus, bcollins, tony.luck 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [KJ] (re) audit return code handling for kernel_thread [1/3] 2006-07-30 0:03 ` Andrew Morton @ 2006-07-30 0:48 ` Neil Horman 2006-07-30 21:39 ` Heiko Carstens 0 siblings, 1 reply; 8+ messages in thread From: Neil Horman @ 2006-07-30 0:48 UTC (permalink / raw) To: Andrew Morton Cc: kernel-janitors, linux-kernel, torvalds, marcel, fpavlic, paulus, bcollins, tony.luck 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [KJ] (re) audit return code handling for kernel_thread [1/3] 2006-07-30 0:48 ` Neil Horman @ 2006-07-30 21:39 ` Heiko Carstens 2006-07-31 9:17 ` Heiko Carstens 0 siblings, 1 reply; 8+ messages in thread From: Heiko Carstens @ 2006-07-30 21:39 UTC (permalink / raw) To: Neil Horman Cc: Andrew Morton, kernel-janitors, linux-kernel, torvalds, marcel, fpavlic, paulus, bcollins, tony.luck > > > --- 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [KJ] (re) audit return code handling for kernel_thread [1/3] 2006-07-30 21:39 ` Heiko Carstens @ 2006-07-31 9:17 ` Heiko Carstens 0 siblings, 0 replies; 8+ messages in thread From: Heiko Carstens @ 2006-07-31 9:17 UTC (permalink / raw) To: Neil Horman Cc: Andrew Morton, kernel-janitors, linux-kernel, torvalds, marcel, fpavlic, paulus, bcollins, tony.luck, Martin Schwidefsky 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [KJ] (re) audit return code handling for kernel_thread [2/3] 2006-07-29 20:11 [KJ] (re) audit return code handling for kernel_thread [0/3] Neil Horman 2006-07-29 20:15 ` [KJ] (re) audit return code handling for kernel_thread [1/3] Neil Horman @ 2006-07-29 20:18 ` Neil Horman 2006-07-29 20:20 ` [KJ] (re) audit return code handling for kernel_thread [3/3] Neil Horman 2 siblings, 0 replies; 8+ messages in thread From: Neil Horman @ 2006-07-29 20:18 UTC (permalink / raw) To: kernel-janitors, linux-kernel, torvalds, akpm, marcel, fpavlic, paulus, bcollins, tony.luck 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [KJ] (re) audit return code handling for kernel_thread [3/3] 2006-07-29 20:11 [KJ] (re) audit return code handling for kernel_thread [0/3] Neil Horman 2006-07-29 20:15 ` [KJ] (re) audit return code handling for kernel_thread [1/3] Neil Horman 2006-07-29 20:18 ` [KJ] (re) audit return code handling for kernel_thread [2/3] Neil Horman @ 2006-07-29 20:20 ` Neil Horman 2 siblings, 0 replies; 8+ messages in thread From: Neil Horman @ 2006-07-29 20:20 UTC (permalink / raw) To: kernel-janitors, linux-kernel, torvalds, akpm, marcel, fpavlic, paulus, bcollins, tony.luck 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2006-07-31 9:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-07-29 20:11 [KJ] (re) audit return code handling for kernel_thread [0/3] Neil Horman 2006-07-29 20:15 ` [KJ] (re) audit return code handling for kernel_thread [1/3] Neil Horman 2006-07-30 0:03 ` Andrew Morton 2006-07-30 0:48 ` Neil Horman 2006-07-30 21:39 ` Heiko Carstens 2006-07-31 9:17 ` Heiko Carstens 2006-07-29 20:18 ` [KJ] (re) audit return code handling for kernel_thread [2/3] Neil Horman 2006-07-29 20:20 ` [KJ] (re) audit return code handling for kernel_thread [3/3] Neil Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).