Re: kernel_thread() & thread starting

From: Andrew Morton (andrewm@uow.edu.au)
Date: Mon Feb 19 2001 - 07:44:00 EST


David Woodhouse wrote:
>
> prumpf@mandrakesoft.com said:
> > Why bother ? It looks like a leftover debugging message which
> > doesn't make a lot of sense once the code is stable (what might make
> > sense is checking keventd is still around, but that's not what the
> > code is doing).

keventd *must* still be around.

And the code obviously isn't completely stable, and this debug
message has found something rather unpleasant.

I don't think we should run the init tasks when keventd may, or
may not be running. Sure, the current code does, by happenstance,
all work correctly when keventd hasn't yet started running, and
when it's starting up. But it's safer, saner and surer just
to crank the damn thing up before proceeding.

> > Proposed patch:
>
> > dwmw2 ?
>
> Don't look at me. I didn't like the current_is_keventd stuff very much
> in the first place. akpm?

Heh. Tell that to wakeup_bdflush().

The all-singing fully-async + fully-sync + async with callback
patch was dropped, and until we can demonstrate that it
fixes a bug, it can stay dropped. I think it _will_ fix
a bug, but the development of userspace hotplug infrastructure
hasn't reached the stage where the need for a kernel fix
has been proven.

I believe the right thing to do here is the RMK approach.

Here's a faintly tested patch.

--- linux-2.4.2-pre4/kernel/context.c Sat Jan 13 04:52:41 2001
+++ lk/kernel/context.c Mon Feb 19 23:33:38 2001
@@ -19,6 +19,7 @@
 #include <linux/init.h>
 #include <linux/unistd.h>
 #include <linux/signal.h>
+#include <asm/semaphore.h>
 
 static DECLARE_TASK_QUEUE(tq_context);
 static DECLARE_WAIT_QUEUE_HEAD(context_task_wq);
@@ -26,19 +27,9 @@
 static int keventd_running;
 static struct task_struct *keventd_task;
 
-static int need_keventd(const char *who)
-{
- if (keventd_running == 0)
- printk(KERN_ERR "%s(): keventd has not started\n", who);
- return keventd_running;
-}
-
 int current_is_keventd(void)
 {
- int ret = 0;
- if (need_keventd(__FUNCTION__))
- ret = (current == keventd_task);
- return ret;
+ return (current == keventd_task);
 }
 
 /**
@@ -57,13 +48,12 @@
 int schedule_task(struct tq_struct *task)
 {
         int ret;
- need_keventd(__FUNCTION__);
         ret = queue_task(task, &tq_context);
         wake_up(&context_task_wq);
         return ret;
 }
 
-static int context_thread(void *dummy)
+static int context_thread(void *sem)
 {
         struct task_struct *curtask = current;
         DECLARE_WAITQUEUE(wait, curtask);
@@ -85,6 +75,7 @@
         siginitset(&sa.sa.sa_mask, sigmask(SIGCHLD));
         do_sigaction(SIGCHLD, &sa, (struct k_sigaction *)0);
 
+ up((struct semaphore *)sem);
         /*
          * If one of the functions on a task queue re-adds itself
          * to the task queue we call schedule() in state TASK_RUNNING
@@ -146,9 +137,11 @@
         remove_wait_queue(&context_task_done, &wait);
 }
         
-int start_context_thread(void)
+int __init start_context_thread(void)
 {
- kernel_thread(context_thread, NULL, CLONE_FS | CLONE_FILES);
+ DECLARE_MUTEX_LOCKED(sem);
+ kernel_thread(context_thread, &sem, CLONE_FS | CLONE_FILES);
+ down(&sem);
         return 0;
 }
 
-
-
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/



This archive was generated by hypermail 2b29 : Fri Feb 23 2001 - 21:00:19 EST