flush_scheduled_tasks() question
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 )
Quoted 1 line · [email protected] said:
Is is intentional that
tummy_taskis not initialized?
It _is_ initialised. To zero :)
Quoted 3 lines
Ok, it won't crash because the current
__run_task_queue()implementation doesn't calltq->routineif 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/
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/