From: Barret Rhoden Date: Sun, 18 Sep 2011 01:29:35 +0000 (-0700) Subject: pthread_join() no longer spins X-Git-Tag: current~4196 X-Git-Url: http://akaros.cs.berkeley.edu/gitweb/?p=akaros.git;a=commitdiff_plain;h=5b57866b57370e3dad3e840de781cfd84097eecf;hp=e9e0888e4fbe52aa12f2f71aa758709f65efa367 pthread_join() no longer spins It'll now wait on the join_target, which will wake it up when it exits (depending on how the atomic_swap race went). --- diff --git a/user/pthread/pthread.c b/user/pthread/pthread.c index cbfa68f..4387785 100644 --- a/user/pthread/pthread.c +++ b/user/pthread/pthread.c @@ -127,30 +127,53 @@ void pth_thread_runnable(struct uthread *uthread) void pth_thread_yield(struct uthread *uthread) { struct pthread_tcb *pthread = (struct pthread_tcb*)uthread; + struct pthread_tcb *temp_pth = 0; /* used for exiting AND joining */ struct mcs_lock_qnode local_qn = {0}; /* Remove from the active list, whether exiting or yielding. We're holding * the lock throughout both list modifications (if applicable). */ mcs_lock_notifsafe(&queue_lock, &local_qn); threads_active--; TAILQ_REMOVE(&active_queue, pthread, next); + mcs_unlock_notifsafe(&queue_lock, &local_qn); if (pthread->flags & PTHREAD_EXITING) { - mcs_unlock_notifsafe(&queue_lock, &local_qn); /* Destroy the pthread */ uthread_cleanup(uthread); /* Cleanup, mirroring pthread_create() */ __pthread_free_stack(pthread); /* TODO: race on detach state */ - if (pthread->detached) + if (pthread->detached) { free(pthread); - else - pthread->finished = 1; + } else { + /* See if someone is joining on us. If not, we're done (and the + * joiner will wake itself when it saw us there instead of 0). */ + temp_pth = atomic_swap_ptr((void**)&pthread->joiner, pthread); + if (temp_pth) { + /* they joined before we exited, we need to wake them */ + printd("[pth] %08p exiting, waking joiner %08p\n", + pthread, temp_pth); + pth_thread_runnable((struct uthread*)temp_pth); + } + } + } else if (pthread->flags & PTHREAD_JOINING) { + /* We're trying to join, yield til we get woken up */ + /* put ourselves in the join target's joiner slot. If we get anything + * back, we lost the race and need to wake ourselves. */ + temp_pth = atomic_swap_ptr((void**)&pthread->join_target->joiner, + pthread); + /* after that atomic swap, the pthread might be woken up (if it + * succeeded), so don't touch pthread again after that (this following + * if () is okay). */ + if (temp_pth) { + assert(temp_pth == pthread->join_target); /* Sanity */ + /* wake ourselves, not the exited one! */ + printd("[pth] %08p already exit, rewaking ourselves, joiner %08p\n", + temp_pth, pthread); + pth_thread_runnable((struct uthread*)pthread); + } } else { - /* Put it on the ready list (tail). Don't do this until we are done - * completely with the thread, since it can be restarted somewhere else. - * */ - threads_ready++; - TAILQ_INSERT_TAIL(&ready_queue, pthread, next); - mcs_unlock_notifsafe(&queue_lock, &local_qn); + /* Yielding for no apparent reason (being nice / help break deadlocks). + * Just wake it up and make it ready again. */ + pth_thread_runnable((struct uthread*)pthread); } } @@ -287,7 +310,8 @@ static int pthread_lib_init(void) t->stacktop = (void*)USTACKTOP; t->detached = TRUE; t->flags = 0; - t->finished = 0; + t->join_target = 0; + t->joiner = 0; assert(t->id == 0); /* Put the new pthread (thread0) on the active queue */ mcs_lock_notifsafe(&queue_lock, &local_qn); @@ -365,10 +389,11 @@ int pthread_create(pthread_t *thread, const pthread_attr_t *attr, pthread = (pthread_t)calloc(1, sizeof(struct pthread_tcb)); assert(pthread); pthread->stacksize = PTHREAD_STACK_SIZE; /* default */ - pthread->finished = 0; pthread->flags = 0; pthread->id = get_next_pid(); pthread->detached = FALSE; /* default */ + pthread->join_target = 0; + pthread->joiner = 0; /* Respect the attributes */ if (attr) { if (attr->stacksize) /* don't set a 0 stacksize */ @@ -395,6 +420,7 @@ int pthread_create(pthread_t *thread, const pthread_attr_t *attr, int pthread_join(pthread_t thread, void** retval) { + struct pthread_tcb *caller = (struct pthread_tcb*)current_uthread; /* Not sure if this is the right semantics. There is a race if we deref * thread and he is already freed (which would have happened if he was * detached. */ @@ -402,8 +428,17 @@ int pthread_join(pthread_t thread, void** retval) printf("[pthread] trying to join on a detached pthread"); return -1; } - while (!thread->finished) - pthread_yield(); + /* See if it is already done, to avoid the pain of a uthread_yield() (the + * early check is an optimization, pth_thread_yield() handles the race). */ + if (!thread->joiner) { + /* Time to join, set things up so pth_thread_yield() knows what to do */ + caller->flags |= PTHREAD_JOINING; + caller->join_target = thread; + uthread_yield(TRUE); + /* When we return/restart, the thread will be done */ + } else { + assert(thread->joiner == thread); /* sanity check */ + } if (retval) *retval = thread->retval; free(thread); diff --git a/user/pthread/pthread.h b/user/pthread/pthread.h index 4951956..f6fc594 100644 --- a/user/pthread/pthread.h +++ b/user/pthread/pthread.h @@ -12,15 +12,18 @@ /* Flags */ #define PTHREAD_EXITING 0x001 +#define PTHREAD_JOINING 0x002 /* Pthread struct. First has to be the uthread struct, which the vcore code * will access directly (as if pthread_tcb is a struct uthread). */ +struct pthread_tcb; struct pthread_tcb { struct uthread uthread; TAILQ_ENTRY(pthread_tcb) next; - int finished; /* TODO: merge this with flags */ int flags; bool detached; + struct pthread_tcb *join_target; /* only used to communicate with yield*/ + struct pthread_tcb *joiner; /* raced on by exit and join */ uint32_t id; uint32_t stacksize; void *stacktop;