libpthread problem + possible solution

[Julian Elischer]

Peter Edwards wrote:
> Hi,
> 
> I've a problem when a process uses:
>        libpthread
>        detached threads
>        mixed bound/unbound threads
>        suspended threads  (a la pthread_resume_np())
> 
> whereby some newly created suspended threads don't get scheduled.
> I think I've tracked it down, so if someone could review the
> reasoning, I'd be grateful.
> 
> Newly launched threads have a "struct pthread" that may be allocated
> from a freelist of GCed threads. Apparently, when detached threads
> enter the GCed list, they can still have the "active" flag set on
> them. Later, this causes problems when this thread is recycled and
> resumed, because _thr_setrunnable_unlocked() doesn't add it to a
> run queue.
> 
> thr_cleanup can be called either from the bound-threads scheduler,
> or the unbound scheduler. One callsite clears "active", "needswitchout",
> and "lock_switch" to zero before the call. The other callsite just
> clears "check_pending". I think these flags are all either bound-thread
> or unbound-thread specific, and that there was an unintended
> assumption that the thread would remain with the same "boundedness"
> after being recycled, which isn't neccessarily the case. (Or another
> way - the idea was that there was no need to clear the "active"
> flag on a bound thread, as its only used for unbound threads, but
> a GCed bound thread might be recycled into an unbound thread)
> 
> Given that, it seems correct to clean up the thread the same way
> for both cases, and just move that code into thr_cleanup. So, does
> the attached patch make sense? I can commit it if someone gives me
> the nod. (It definitely fixes my specific problem with threads not
> getting scheduled.)


your logic sounds sound.. I'll wait for DAN to make a pronouncement however.

> 
> 
> ------------------------------------------------------------------------
> 
> Index: lib/libpthread/thread/thr_kern.c
> ===================================================================
> RCS file: /net/dyson/export/home/petere/FreeBSD-CVS/src/lib/libpthread/thread/thr_kern.c,v
> retrieving revision 1.116.2.1
> diff -u -r1.116.2.1 thr_kern.c
> --- lib/libpthread/thread/thr_kern.c	16 Mar 2006 23:29:07 -0000	1.116.2.1
> +++ lib/libpthread/thread/thr_kern.c	15 Dec 2006 17:48:20 -0000
> @@ -764,7 +764,6 @@
>  		break;
>  
>  	case PS_DEAD:
> -		curthread->check_pending = 0;
>  		/* Unlock the scheduling queue and exit the KSE and thread. */
>  		thr_cleanup(curkse, curthread);
>  		KSE_SCHED_UNLOCK(curkse, curkse->k_kseg);
> @@ -1150,6 +1149,11 @@
>  	struct kse_mailbox *kmbx = NULL;
>  	int sys_scope;
>  
> +	thread->active = 0;
> +	thread->need_switchout = 0;
> +	thread->lock_switch = 0;
> +	thread->check_pending = 0;
> +
>  	if ((joiner = thread->joiner) != NULL) {
>  		/* Joinee scheduler lock held; joiner won't leave. */
>  		if (joiner->kseg == curkse->k_kseg) {
> @@ -1717,9 +1721,6 @@
>  			 * stack.  It is safe to do garbage collecting
>  			 * here.
>  			 */
> -			thread->active = 0;
> -			thread->need_switchout = 0;
> -			thread->lock_switch = 0;
>  			thr_cleanup(kse, thread);
>  			return;
>  			break;
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> freebsd-threads at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-threads
> To unsubscribe, send any mail to "freebsd-threads-unsubscribe at freebsd.org"