Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 May 2004 18:04:35 -0700 (PDT)
From:      Julian Elischer <julian@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 53023 for review
Message-ID:  <200405190104.i4J14ZLZ083130@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=53023

Change 53023 by julian@julian_desk on 2004/05/18 18:04:05

	further cleanups in thread exiting.

Affected files ...

.. //depot/projects/nsched/sys/kern/kern_kse.c#6 edit
.. //depot/projects/nsched/sys/kern/kern_thread.c#14 edit
.. //depot/projects/nsched/sys/kern/sched_4bsd.c#9 edit
.. //depot/projects/nsched/sys/kern/sched_ule.c#4 edit
.. //depot/projects/nsched/sys/sys/proc.h#9 edit

Differences ...

==== //depot/projects/nsched/sys/kern/kern_kse.c#6 (text+ko) ====

@@ -244,10 +244,24 @@
 	int    error, count;
 
 	p = td->td_proc;
+	/* 
+	 * Ensure that this is only called from the UTS
+	 */
 	if ((ku = td->td_upcall) == NULL || TD_CAN_UNBIND(td))
 		return (EINVAL);
+
 	kg = td->td_ksegrp;
 	count = 0;
+
+	/*
+	 * Calculate the existing non-exiting upcalls in this ksegroup.
+	 * If we are the last upcall but there are still other threads,
+	 * then do not exit. We need the other threads to be able to 
+	 * complete whatever they are doing.
+	 * XXX This relies on the userland knowing what to do if we return.
+	 * It may be a better choice to convert ourselves into a kse_release
+	 * ( or similar) and wait in the kernel to be needed.
+	 */
 	PROC_LOCK(p);
 	mtx_lock_spin(&sched_lock);
 	FOREACH_UPCALL_IN_GROUP(kg, ku2) {
@@ -263,27 +277,51 @@
 	ku->ku_flags |= KUF_EXITING;
 	mtx_unlock_spin(&sched_lock);
 	PROC_UNLOCK(p);
+
+	/* 
+	 * Mark the UTS mailbox as having been finished with.
+	 * If that fails then just go for a segfault.
+	 * XXX need to check it that can be deliverred without a mailbox.
+	 */
 	error = suword(&ku->ku_mailbox->km_flags, ku->ku_mflags|KMF_DONE);
 	PROC_LOCK(p);
 	if (error)
 		psignal(p, SIGSEGV);
 	mtx_lock_spin(&sched_lock);
 	upcall_remove(td);
-	if (p->p_numthreads == 1) {
-		thread_purge(p, td);
-		p->p_flag &= ~P_SA;
-		mtx_unlock_spin(&sched_lock);
-		PROC_UNLOCK(p);
-	} else {
-		if (kg->kg_numthreads == 1) { /* Shutdown a group */
-			thread_purge_group(td);
-			sched_exit_ksegrp(p->p_pptr, td);
-		}
+
+	if (p->p_numthreads != 1) {
+		/* 
+		 * If we are not the last thread, but we are the last
+		 * thread in this ksegrp, then by definition this is not
+		 * the last group and we need to clean it up as well.
+		 * thread_exit will clean up the kseg as needed.
+		 */
 		thread_stopped(p);
 		thread_exit();
 		/* NOTREACHED */
 	}
+	/* 
+	 * This is the last thread. Just return to the user.
+	 * We know that there is only one ksegrp too, as any others
+	 * would have been discarded in previous calls to thread_exit().
+	 * Effectively we have left threading mode..
+	 * The only real thing left to do is ensure that the 
+	 * scheduler sets out concurrancy back to 1 as that may be a 
+	 * resource leak otherwise.
+	 * This is an A[PB]I issue.. what SHOULD we do?
+	 * One possibility is to return to the user. It may not cope well.
+	 * The other possibility would be to let the process exit.
+	 */
+	p->p_flag &= ~P_SA;
+	mtx_unlock_spin(&sched_lock);
+	sched_set_concurrancy(td->td_ksegrp, 1);
+	PROC_UNLOCK(p);
+#if 1
 	return (0);
+#else
+	exit1(td, 0);
+#endif
 }
 
 /*
@@ -327,7 +365,8 @@
 	PROC_LOCK(p);
 	if (ku->ku_mflags & KMF_WAITSIGEVENT) {
 		/* UTS wants to wait for signal event */
-		if (!(p->p_flag & P_SIGEVENT) && !(ku->ku_flags & KUF_DOUPCALL)) {
+		if (!(p->p_flag & P_SIGEVENT) &&
+		    !(ku->ku_flags & KUF_DOUPCALL)) {
 			td->td_kflags |= TDK_KSERELSIG;
 			error = msleep(&p->p_siglist, &p->p_mtx, PPAUSE|PCATCH,
 			    "ksesigwait", (uap->timeout ? tvtohz(&tv) : 0));

==== //depot/projects/nsched/sys/kern/kern_thread.c#14 (text+ko) ====

@@ -546,6 +546,12 @@
  * Of course in the end, they end up coming here through exit1
  * anyhow..  After fixing 'thr' to play by the rules we should be able 
  * to merge these two functions together.
+
+from kse_exit().. check if we need to do all this here now..
+
+                if (kg->kg_numthreads == 1) { /* Shutdown a group */
+                }
+
  */
 void
 thread_exit(void)
@@ -609,6 +615,7 @@
 			if (kg->kg_numthreads == 0) {
 				/* This kseg is kaput */
 				sched_set_concurrancy(kg, 0);
+                        	sched_exit_ksegrp(p, td); /* XXX fix */
 				ksegrp_unlink(kg);
 			}
 			
@@ -704,54 +711,6 @@
 }
 
 /*
- * Purge a ksegrp resource. When a ksegrp is preparing to
- * exit, it calls this function.
- */
-void
-thread_purge_group(struct thread *td)
-{
-	struct ksegrp *kg;
-
-	kg = td->td_ksegrp;
- 	KASSERT(kg->kg_numthreads == 1, ("%s: bad thread number", __func__));
-	sched_clean_ksegrp(kg, td);
-	KASSERT((kg->kg_numupcalls == 0),
-	        ("%s: ksegrp still has %d upcall datas",
-		__func__, kg->kg_numupcalls));
-}
-
-/*
- * Purge a process's KSE resource. When a process is preparing to
- * exit, it calls thread_purge to release any extra KSE resources in
- * the process.
- */
-void
-thread_purge(struct proc *p, struct thread *td)
-{
-	struct ksegrp *kg;
-
- 	KASSERT(p->p_numthreads == 1, ("bad thread number"));
-	while ((kg = TAILQ_FIRST(&p->p_ksegrps)) != NULL) {
-		TAILQ_REMOVE(&p->p_ksegrps, kg, kg_ksegrp);
-		p->p_numksegrps--;
-		/*
-		 * There is no ownership for KSE, after all threads
-		 * in the group exited, it is possible that some KSEs
-		 * were left in idle queue, gc them now.
-		 */
-		sched_clean_ksegrp(kg, td);
-		KASSERT((kg->kg_numupcalls == 0),
-		        ("%s: ksegrp still has %d upcall datas",
-			__func__, kg->kg_numupcalls));
-
-		if (kg != td->td_ksegrp)
-			ksegrp_stash(kg);
-	}
-	TAILQ_INSERT_HEAD(&p->p_ksegrps, td->td_ksegrp, kg_ksegrp);
-	p->p_numksegrps++;
-}
-
-/*
  * Enforce single-threading.
  *
  * Returns 1 if the caller must abort (another thread is waiting to
@@ -839,7 +798,6 @@
 	if (force_exit == SINGLE_EXIT) {
 		if (td->td_upcall)
 			upcall_remove(td);
-		thread_purge(p, td);
 	}
 	mtx_unlock_spin(&sched_lock);
 	return (0);

==== //depot/projects/nsched/sys/kern/sched_4bsd.c#9 (text+ko) ====

@@ -654,7 +654,10 @@
 	sched_exit_thread(parent, childtd);
 }
 
-/* This PROBABLY gives the estcpu to the wrong kseg */
+/*
+ * This PROBABLY gives the estcpu to the wrong kseg
+ * should have a proc estcpu that gathers for exited ksegrps.. (& children)
+ */
 void
 sched_exit_ksegrp(struct proc *parent, struct thread *child)
 {
@@ -1208,25 +1211,6 @@
 	kse_stash(ke);
 }
 
-/* need the thread to know if we are on that ksegrp or not. */
-void
-sched_clean_ksegrp(struct ksegrp *kg, struct thread *td)
-{
-	struct kse *ke;
-
-	while ((ke = TAILQ_FIRST(&kg->kg_iq)) != NULL) {
-		KASSERT(ke->ke_state == KES_IDLE,
-			("%s: wrong idle KSE state", __func__));
-		kse_unlink(ke);
-	}
-	KASSERT((kg->kg_kses == 1),
-		("%s: ksegrp still has %d KSEs", __func__, kg->kg_kses));
-	KASSERT(((kg->kg_kses == 0) && (kg != td->td_ksegrp)) ||
-	        ((kg->kg_kses == 1) && (kg == td->td_ksegrp)),
-	        ("ksegrp has wrong kg_kses: %d", kg->kg_kses));
-}
-
-
 #define RANGEOF(type, start, end) (offsetof(type, end) - offsetof(type, start))
 
 /* new version of sched-fork() */

==== //depot/projects/nsched/sys/kern/sched_ule.c#4 (text+ko) ====


==== //depot/projects/nsched/sys/sys/proc.h#9 (text+ko) ====

@@ -813,8 +813,6 @@
 void	thread_free(struct thread *td);
 void	thread_link(struct thread *td, struct ksegrp *kg);
 int	thread_new_tid(void);
-void	thread_purge(struct proc *p, struct thread *td);
-void	thread_purge_group(struct thread *td);
 void	thread_reap(void);
 struct thread *thread_schedule_upcall(struct thread *td, struct kse_upcall *ku);
 int	thread_single(int how);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200405190104.i4J14ZLZ083130>