Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 29 Aug 2002 11:23:33 -0700 (PDT)
From:      Julian Elischer <julian@elischer.org>
To:        John Baldwin <jhb@FreeBSD.org>
Cc:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   Re: PERFORCE change 16771 for review
Message-ID:  <Pine.BSF.4.21.0208291118420.98653-100000@InterJet.elischer.org>
In-Reply-To: <200208291814.g7TIEfdL023514@freefall.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
I'd be happy to see this merged into -current now..
Any objections?

BTW the extra tests were at one stage necessary when thread/proc teardown
was done differently. Thread_exit did at one time take a td argument and
it was possible to call it on a thread other than curthread
in the case where thread-setup failed and we were backing out of creating
a new thread. This was all discovered to be "A Bad Thing" (TM)
but the tests remained.

Also, peter and I discussed over the weekend some changes that may affect
this a little.


On Thu, 29 Aug 2002, John Baldwin wrote:

> http://people.freebsd.org/~peter/p4db/chv.cgi?CH=16771
> 
> Change 16771 by jhb@jhb_zion on 2002/08/29 11:13:42
> 
> 	Fix crack-smoking code that was panicing on the quad xeon:
> 	- If either of proc or kse are NULL during thread_exit(), then
> 	  the kernel is going to fault because parts of the function
> 	  assume they aren't NULL.  Instead, just assert they aren't NULL
> 	  (as well as the kse group) and assume they are in all of the
> 	  code.  It doesn't make sense for them to be NULL here anyways.
> 	- Move the PROC_UNLOCK(p) up above clearing td_proc, etc. since
> 	  otherwise we will panic if the proc's lock is contested.
> 
> Affected files ...
> 
> .. //depot/projects/smpng/sys/kern/kern_thread.c#4 edit
> 
> Differences ...
> 
> ==== //depot/projects/smpng/sys/kern/kern_thread.c#4 (text+ko) ====
> 
> @@ -320,6 +320,9 @@
>  	ke = td->td_kse;
>  
>  	mtx_assert(&sched_lock, MA_OWNED);
> +	KASSERT(p != NULL, ("thread exiting without a process"));
> +	KASSERT(ke != NULL, ("thread exiting without a kse"));
> +	KASSERT(kg != NULL, ("thread exiting without a kse group"));
>  	PROC_LOCK_ASSERT(p, MA_OWNED);
>  	CTR1(KTR_PROC, "thread_exit: thread %p", td);
>  	KASSERT(!mtx_owned(&Giant), ("dying thread owns giant"));
> @@ -331,41 +334,35 @@
>  	cpu_thread_exit(td);	/* XXXSMP */
>  
>  	/* Reassign this thread's KSE. */
> -	if (ke != NULL) {
> -		ke->ke_thread = NULL;
> -		td->td_kse = NULL;
> -		ke->ke_state = KES_UNQUEUED;
> -		kse_reassign(ke);
> -	}
> +	ke->ke_thread = NULL;
> +	td->td_kse = NULL;
> +	ke->ke_state = KES_UNQUEUED;
> +	kse_reassign(ke);
>  
>  	/* Unlink this thread from its proc. and the kseg */
> -	if (p != NULL) {
> -		TAILQ_REMOVE(&p->p_threads, td, td_plist);
> -		p->p_numthreads--;
> -		if (kg != NULL) {
> -			TAILQ_REMOVE(&kg->kg_threads, td, td_kglist);
> -			kg->kg_numthreads--;
> -		}
> -		/*
> -		 * The test below is NOT true if we are the
> -		 * sole exiting thread. P_STOPPED_SNGL is unset
> -		 * in exit1() after it is the only survivor.
> -		 */
> -		if (P_SHOULDSTOP(p) == P_STOPPED_SNGL) {
> -			if (p->p_numthreads == p->p_suspcount) {
> -				TAILQ_REMOVE(&p->p_suspended,
> -				    p->p_singlethread, td_runq);
> -				setrunqueue(p->p_singlethread);
> -				p->p_suspcount--;
> -			}
> +	TAILQ_REMOVE(&p->p_threads, td, td_plist);
> +	p->p_numthreads--;
> +	TAILQ_REMOVE(&kg->kg_threads, td, td_kglist);
> +	kg->kg_numthreads--;
> +	/*
> +	 * The test below is NOT true if we are the
> +	 * sole exiting thread. P_STOPPED_SNGL is unset
> +	 * in exit1() after it is the only survivor.
> +	 */
> +	if (P_SHOULDSTOP(p) == P_STOPPED_SNGL) {
> +		if (p->p_numthreads == p->p_suspcount) {
> +			TAILQ_REMOVE(&p->p_suspended,
> +			    p->p_singlethread, td_runq);
> +			setrunqueue(p->p_singlethread);
> +			p->p_suspcount--;
>  		}
>  	}
> +	PROC_UNLOCK(p);
>  	td->td_state	= TDS_SURPLUS;
>  	td->td_proc	= NULL;
>  	td->td_ksegrp	= NULL;
>  	td->td_last_kse	= NULL;
>  	ke->ke_tdspare = td;
> -	PROC_UNLOCK(p);
>  	cpu_throw();
>  	/* NOTREACHED */
>  }
> 


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe p4-projects" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0208291118420.98653-100000>