Skip to contentSkip to conversation

flush_scheduled_tasks() question

PATCH Manfred Spraul 2 participants 3 messages Jan 28, 2001 – Jan 29, 2001

lore
Share & tools
lore
#0 lore raw

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

context.c renamed from context.c.orig +13 -22
@@ -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)
#1 lore raw
Quoted 1 line · [email protected] said:

Is is intentional that tummy_task is not initialized?

It _is_ initialised. To zero :)

Quoted 3 lines

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 */;

Quoted 2 lines · [email protected] 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 [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#2 lore raw
In reply to Manfred Spraul

Review

Quoted 4 lines · David Woodhouse
(quoted diff) +1 -1
-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 [email protected]
Please read the FAQ at http://www.tux.org/lkml/