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>