lkml.org 
[lkml]   [2006]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [KJ] (re) audit return code handling for kernel_thread [1/3]
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);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2006-07-30 02:57    [W:0.040 / U:0.916 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site