flush_scheduled_tasks() question

flush_scheduled_tasks() question

[Manfred Spraul]

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]

Is is intentional that tummy_task is not initialized?
Ok, it won't crash because the current __run_task_queue() implementation
doesn't call tq->routine if it's NULL, but IMHO it's ugly.

Additionally I don't like the loop in flush_scheduled_tasks(), what
about replacing it with a locked semaphore (same idea as vfork)?

I've attched an untested patch.

--
	Manfred

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1888 bytes --]

--- context.c.orig	Sun Jan 28 17:02:35 2001
+++ context.c	Sun Jan 28 17:09:24 2001
@@ -22,7 +22,6 @@
 
 static DECLARE_TASK_QUEUE(tq_context);
 static DECLARE_WAIT_QUEUE_HEAD(context_task_wq);
-static DECLARE_WAIT_QUEUE_HEAD(context_task_done);
 static int keventd_running;
 static struct task_struct *keventd_task;
 
@@ -97,7 +96,6 @@
 		schedule();
 		remove_wait_queue(&context_task_wq, &wait);
 		run_task_queue(&tq_context);
-		wake_up(&context_task_done);
 		if (signal_pending(curtask)) {
 			while (waitpid(-1, (unsigned int *)0, __WALL|WNOHANG) > 0)
 				;
@@ -119,31 +117,24 @@
  * The caller should hold no spinlocks and should hold no semaphores which could
  * cause the scheduled tasks to block.
  */
-static struct tq_struct dummy_task;
 
-void flush_scheduled_tasks(void)
+static void up_semaphore(void* sem)
 {
-	int count;
-	DECLARE_WAITQUEUE(wait, current);
+	up((struct semaphore*)sem);
+}
 
-	/*
-	 * Do it twice. It's possible, albeit highly unlikely, that
-	 * the caller queued a task immediately before calling us,
-	 * and that the eventd thread was already past the run_task_queue()
-	 * but not yet into wake_up(), so it woke us up before completing
-	 * the caller's queued task or our new dummy task.
-	 */
-	add_wait_queue(&context_task_done, &wait);
-	for (count = 0; count < 2; count++) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
+void flush_scheduled_tasks(void)
+{
+	DECLARE_MUTEX_LOCKED(sem);
+	struct tq_struct wakeup_task;
 
-		/* Queue a dummy task to make sure we get kicked */
-		schedule_task(&dummy_task);
+	wakeup_task.routine = up_semaphore;
+	wakeup_task.data = &sem;
+	/* Queue a dummy task to make sure we get kicked */
+	schedule_task(&dummy_task);
 
-		/* Wait for it to complete */
-		schedule();
-	}
-	remove_wait_queue(&context_task_done, &wait);
+	/* Wait for it to complete */
+	down(&sem);
 }
 	
 int start_context_thread(void)

Re: flush_scheduled_tasks() question

[David Woodhouse]

manfred@colorfullife.com said:
> Is is intentional that tummy_task is not initialized? 

It _is_ initialised. To zero :)

> Ok, it won't crash because the current __run_task_queue()
> implementation doesn't call tq->routine if it's NULL, but IMHO it's
> ugly.

-static struct tq_struct dummy_task;
+static struct tq_struct dummy_task /* = all zero */;


manfred@colorfullife.com said:
>  Additionally I don't like the loop in flush_scheduled_tasks(), what
> about replacing it with a locked semaphore (same idea as vfork)?

The reason for doing it that way was because there was no guarantee that 
scheduled tasks will be called in order. So you can't just stick a new task 
in the queue and assume that when it's completed the queue is flushed. 

Linus then changed that and made the eventd thread call tasks in order, but 
I believe the intention is still that we don't make that guarantee, so it 
may change at any point in the future. 

--
dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: flush_scheduled_tasks() question

[Manfred Spraul]

David Woodhouse wrote:
> 
> -static struct tq_struct dummy_task;
> +static struct tq_struct dummy_task /* = all zero */;
>
That comment is superflous - that's just C.
The non-obvious part is

+static struct tq_struct dummy_task; /* remains zero, run_task_queue()
supports tqs.routine==NULL*/

BUT: The implementation isn't thread safe: what if multiple threads call
flush_scheduled_tasks()?

--
	Manfred
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/