From c763679aaab857e313f4c436ad251f472fe3adb4 Mon Sep 17 00:00:00 2001 From: Bahadir Balban Date: Sat, 31 Oct 2009 20:47:20 +0200 Subject: [PATCH] Fixed a nasty spinlock issue with wake_up_all that didn't get caught. --- conts/test/main.c | 7 +++-- include/l4/lib/wait.h | 2 +- src/api/thread.c | 61 ++++++++++++++++++++++++++++++++++++---- src/arch/arm/exception.c | 2 +- src/generic/irq.c | 2 -- src/lib/wait.c | 8 ++++-- 6 files changed, 67 insertions(+), 15 deletions(-) diff --git a/conts/test/main.c b/conts/test/main.c index dbf53e0..48d13d2 100644 --- a/conts/test/main.c +++ b/conts/test/main.c @@ -36,6 +36,7 @@ int exit_test(void) // l4_thread_switch(0); +#if 0 /* Kill it */ printf("Killing Thread (%d).\n", ids.tid); if ((ret = l4_thread_control(THREAD_DESTROY, &ids)) < 0) @@ -43,7 +44,7 @@ int exit_test(void) else printf("Success: Killed Thread (%d)\n", ids.tid); - return 0; +#endif #if 0 /* Wait on it */ @@ -53,9 +54,9 @@ int exit_test(void) else printf("Error. Wait on (%d) failed. err = %d\n", ids.tid, ret); - return 0; #endif + return 0; out_err: BUG(); } @@ -70,7 +71,7 @@ int main(void) exit_test(); /* Now quit to demo self-paging quit */ - //l4_exit(0); + l4_exit(0); /* Now quit by null pointer */ // *((int *)0) = 5; diff --git a/include/l4/lib/wait.h b/include/l4/lib/wait.h index 455c9e2..98453a8 100644 --- a/include/l4/lib/wait.h +++ b/include/l4/lib/wait.h @@ -59,7 +59,7 @@ do { \ task_set_wqh(current, wqh, &wq); \ (wqh)->sleepers++; \ list_insert_tail(&wq.task_list, &(wqh)->task_list);\ - /* printk("(%d) waiting...\n", current->tid);*/ \ + /* printk("(%d) waiting...\n", current->tid); */ \ sched_prepare_sleep(); \ spin_unlock(&(wqh)->slock); \ schedule(); \ diff --git a/src/api/thread.c b/src/api/thread.c index 9390d46..bca194e 100644 --- a/src/api/thread.c +++ b/src/api/thread.c @@ -54,6 +54,54 @@ int thread_suspend(struct ktcb *task) return thread_signal(task, TASK_SUSPENDING, TASK_INACTIVE); } +static inline int TASK_IS_CHILD(struct ktcb *task) +{ + return (((task) != current) && + ((task)->pagerid == current->tid)); +} + +int thread_delete_children(void) +{ + struct ktcb *task, *n; + + spin_lock(&curcont->ktcb_list.list_lock); + list_foreach_removable_struct(task, n, + &curcont->ktcb_list.list, + task_list) { + if (TASK_IS_CHILD(task)) { + spin_unlock(&curcont->ktcb_list.list_lock); + tcb_remove(task); + wake_up_all(¤t->wqh_send, 0); + wake_up_all(¤t->wqh_recv, 0); + BUG_ON(task->wqh_pager.sleepers > 0); + BUG_ON(task->state != TASK_INACTIVE); + tcb_delete(task); + spin_lock(&curcont->ktcb_list.list_lock); + } + } + spin_unlock(&curcont->ktcb_list.list_lock); + return 0; +} + +int thread_suspend_children(void) +{ + struct ktcb *task, *n; + + spin_lock(&curcont->ktcb_list.list_lock); + list_foreach_removable_struct(task, n, + &curcont->ktcb_list.list, + task_list) { + if (TASK_IS_CHILD(task)) { + spin_unlock(&curcont->ktcb_list.list_lock); + thread_suspend(task); + spin_lock(&curcont->ktcb_list.list_lock); + } + } + spin_unlock(&curcont->ktcb_list.list_lock); + return 0; + +} + /* * Put them in TASK_DEAD so that a suspended exiting thread * does not run again if issued THREAD_RUN @@ -61,20 +109,23 @@ int thread_suspend(struct ktcb *task) int thread_destroy(struct ktcb *task) { if (task == current) { - if (current->tid == current->pagerid) + if (current->tid == current->pagerid) { + /* Suspend children */ + thread_suspend_children(); + thread_delete_children(); sched_exit_pager(); - else + } else { sched_suspend_sync(); + } return 0; } - thread_suspend(task); tcb_remove(task); /* Wake up waiters */ - wake_up_all(¤t->wqh_send, 0); - wake_up_all(¤t->wqh_recv, 0); + wake_up_all(&task->wqh_send, 0); + wake_up_all(&task->wqh_recv, 0); BUG_ON(task->wqh_pager.sleepers > 0); BUG_ON(task->state != TASK_INACTIVE); diff --git a/src/arch/arm/exception.c b/src/arch/arm/exception.c index 96c694b..fa88618 100644 --- a/src/arch/arm/exception.c +++ b/src/arch/arm/exception.c @@ -21,7 +21,7 @@ #include INC_SUBARCH(mm.h) /* Abort debugging conditions */ -// #define DEBUG_ABORTS +//#define DEBUG_ABORTS #if defined (DEBUG_ABORTS) #define dbg_abort(...) dprintk(__VA_ARGS__) #else diff --git a/src/generic/irq.c b/src/generic/irq.c index ac0c20e..be855f5 100644 --- a/src/generic/irq.c +++ b/src/generic/irq.c @@ -74,11 +74,9 @@ void do_irq(void) printk("Spurious or broken irq\n"); BUG(); } irq_enable(irq_index); - #if 0 /* Process any pending flags for currently runnable task */ if (!in_nested_irq_context()) { - /* This is buggy, races on prio_total */ if (in_user()) { if (current->flags & TASK_SUSPENDING) sched_suspend_async(); diff --git a/src/lib/wait.c b/src/lib/wait.c index c587525..97ddc48 100644 --- a/src/lib/wait.c +++ b/src/lib/wait.c @@ -107,11 +107,11 @@ int wait_on(struct waitqueue_head *wqh) return 0; } -/* Wake up all - FIXME: this is buggy with double spin_unlock */ +/* Wake up all in the queue */ void wake_up_all(struct waitqueue_head *wqh, unsigned int flags) { - BUG_ON(wqh->sleepers < 0); spin_lock(&wqh->slock); + BUG_ON(wqh->sleepers < 0); while (wqh->sleepers > 0) { struct waitqueue *wq = link_to_struct(wqh->task_list.next, struct waitqueue, @@ -130,6 +130,8 @@ void wake_up_all(struct waitqueue_head *wqh, unsigned int flags) sched_resume_sync(sleeper); else sched_resume_async(sleeper); + + spin_lock(&wqh->slock); } spin_unlock(&wqh->slock); } @@ -150,7 +152,7 @@ void wake_up(struct waitqueue_head *wqh, unsigned int flags) task_unset_wqh(sleeper); if (flags & WAKEUP_INTERRUPT) sleeper->flags |= TASK_INTERRUPTED; - // printk("(%d) Waking up (%d)\n", current->tid, sleeper->tid); + //printk("(%d) Waking up (%d)\n", current->tid, sleeper->tid); spin_unlock(&wqh->slock); if (flags & WAKEUP_SYNC)