PERFORCE change 48321 for review

Peter Wemm peter at FreeBSD.org
Sat Mar 6 20:12:38 PST 2004


http://perforce.freebsd.org/chv.cgi?CH=48321

Change 48321 by peter at peter_melody on 2004/03/06 20:12:12

	Do some more Giant pushdown. thread_single*() doesn't need
	it anymore, invert the assertion and update callers.
	
	Move the vref in fork to after the proc lock section, since
	we are currently executing and the fact that we are executing
	means there is a reference on the textvp and it can't go away.
	
	I wish I knew more about which parts of vm are safe, the vm_forkproc()
	and vm_waitproc() stuff is a major spoiler.  Also, cpu_thread_clean
	on i386 wraps a kmem_free() with giant calls.  Is this needed?

Affected files ...

.. //depot/projects/hammer/sys/kern/kern_exec.c#20 edit
.. //depot/projects/hammer/sys/kern/kern_exit.c#20 edit
.. //depot/projects/hammer/sys/kern/kern_fork.c#29 edit
.. //depot/projects/hammer/sys/kern/kern_thread.c#43 edit
.. //depot/projects/hammer/sys/kern/subr_trap.c#19 edit

Differences ...

==== //depot/projects/hammer/sys/kern/kern_exec.c#20 (text+ko) ====

@@ -253,7 +253,6 @@
 	 * that might allow a local user to illicitly obtain elevated
 	 * privileges.
 	 */
-	mtx_lock(&Giant);
 	PROC_LOCK(p);
 	KASSERT((p->p_flag & P_INEXEC) == 0,
 	    ("%s(): process already has P_INEXEC flag", __func__));
@@ -271,7 +270,6 @@
 		td->td_mailbox = NULL;
 		thread_single_end();
 	}
-	mtx_unlock(&Giant);
 	p->p_flag |= P_INEXEC;
 	PROC_UNLOCK(p);
 

==== //depot/projects/hammer/sys/kern/kern_exit.c#20 (text+ko) ====

@@ -137,7 +137,6 @@
 	/*
 	 * MUST abort all other threads before proceeding past here.
 	 */
-	mtx_lock(&Giant);
 	PROC_LOCK(p);
 	if (p->p_flag & P_SA || p->p_numthreads > 1) {
 		/*
@@ -160,9 +159,8 @@
 		 * from userret().  thread_exit() will unsuspend us
 		 * when the last other thread exits.
 		 */
-		if (thread_single(SINGLE_EXIT)) {
+		if (thread_single(SINGLE_EXIT))
 			panic ("Exit: Single threading fouled up");
-		}
 		/*
 		 * All other activity in this process is now stopped.
 		 * Remove excess KSEs and KSEGRPS. XXXKSE (when we have them)
@@ -172,7 +170,6 @@
 		p->p_flag &= ~P_SA;
 		thread_single_end();	/* Don't need this any more. */
 	}
-	mtx_unlock(&Giant);
 	/*
 	 * With this state set:
 	 * Any thread entering the kernel from userspace will thread_exit()
@@ -716,7 +713,6 @@
 			/*
 			 * do any thread-system specific cleanups
 			 */
-			mtx_lock(&Giant);
 			thread_wait(p);
 
 			/*
@@ -724,6 +720,7 @@
 			 * to free anything that cpu_exit couldn't
 			 * release while still running in process context.
 			 */
+			mtx_lock(&Giant);
 			vm_waitproc(p);
 			mtx_unlock(&Giant);
 #ifdef MAC

==== //depot/projects/hammer/sys/kern/kern_fork.c#29 (text+ko) ====

@@ -270,16 +270,13 @@
 		 * where they will try restart in the parent and will
 		 * be aborted in the child.
 		 */
-		mtx_lock(&Giant);
 		PROC_LOCK(p1);
 		if (thread_single(SINGLE_NO_EXIT)) {
 			/* Abort. Someone else is single threading before us. */
 			PROC_UNLOCK(p1);
-			mtx_unlock(&Giant);
 			return (ERESTART);
 		}
 		PROC_UNLOCK(p1);
-		mtx_unlock(&Giant);
 		/*
 		 * All other activity in this process
 		 * is now suspended at the user boundary,
@@ -474,7 +471,6 @@
 	if (pages != 0)
 		vm_thread_new_altkstack(td2, pages);
 
-	mtx_lock(&Giant);	/* XXX: for VREF() */
 	PROC_LOCK(p2);
 	PROC_LOCK(p1);
 
@@ -537,11 +533,7 @@
 	else
 	        p2->p_sigparent = SIGCHLD;
 
-	/* Bump references to the text vnode (for procfs) */
 	p2->p_textvp = p1->p_textvp;
-	if (p2->p_textvp)
-		VREF(p2->p_textvp);
-	mtx_unlock(&Giant); /* XXX: for VREF() */
 	p2->p_fd = fd;
 	p2->p_fdtol = fdtol;
 
@@ -552,6 +544,10 @@
 	PROC_UNLOCK(p1);
 	PROC_UNLOCK(p2);
 
+	/* Bump references to the text vnode (for procfs) */
+	if (p2->p_textvp)
+		vref(p2->p_textvp);
+
 	/*
 	 * Set up linkage for kernel based threading.
 	 */

==== //depot/projects/hammer/sys/kern/kern_thread.c#43 (text+ko) ====

@@ -1331,13 +1331,14 @@
 
 /*
  * Do any thread specific cleanups that may be needed in wait()
- * called with Giant held, proc and schedlock not held.
+ * called with Giant, proc and schedlock not held.
  */
 void
 thread_wait(struct proc *p)
 {
 	struct thread *td;
 
+	mtx_assert(&Giant, MA_NOTOWNED);
 	KASSERT((p->p_numthreads == 1), ("Multiple threads in wait1()"));
 	KASSERT((p->p_numksegrps == 1), ("Multiple ksegrps in wait1()"));
 	FOREACH_THREAD_IN_PROC(p, td) {
@@ -1468,6 +1469,7 @@
 void
 thread_alloc_spare(struct thread *td, struct thread *spare)
 {
+
 	if (td->td_standin)
 		return;
 	if (spare == NULL)
@@ -1876,7 +1878,7 @@
 
 	td = curthread;
 	p = td->td_proc;
-	mtx_assert(&Giant, MA_OWNED);
+	mtx_assert(&Giant, MA_NOTOWNED);
 	PROC_LOCK_ASSERT(p, MA_OWNED);
 	KASSERT((td != NULL), ("curthread is NULL"));
 
@@ -1933,11 +1935,9 @@
 		 * In the mean time we suspend as well.
 		 */
 		thread_suspend_one(td);
-		DROP_GIANT();
 		PROC_UNLOCK(p);
 		mi_switch(SW_VOL);
 		mtx_unlock_spin(&sched_lock);
-		PICKUP_GIANT();
 		PROC_LOCK(p);
 		mtx_lock_spin(&sched_lock);
 	}
@@ -1991,6 +1991,7 @@
 
 	td = curthread;
 	p = td->td_proc;
+	mtx_assert(&Giant, MA_NOTOWNED);
 	PROC_LOCK_ASSERT(p, MA_OWNED);
 	while (P_SHOULDSTOP(p)) {
 		if (P_SHOULDSTOP(p) == P_STOPPED_SINGLE) {
@@ -2016,8 +2017,6 @@
 		 * Assumes that P_SINGLE_EXIT implies P_STOPPED_SINGLE.
 		 */
 		if ((p->p_flag & P_SINGLE_EXIT) && (p->p_singlethread != td)) {
-			while (mtx_owned(&Giant))
-				mtx_unlock(&Giant);
 			if (p->p_flag & P_SA)
 				thread_exit();
 			else
@@ -2035,11 +2034,9 @@
 				thread_unsuspend_one(p->p_singlethread);
 			}
 		}
-		DROP_GIANT();
 		PROC_UNLOCK(p);
 		mi_switch(SW_INVOL);
 		mtx_unlock_spin(&sched_lock);
-		PICKUP_GIANT();
 		PROC_LOCK(p);
 	}
 	return (0);

==== //depot/projects/hammer/sys/kern/subr_trap.c#19 (text+ko) ====

@@ -114,9 +114,8 @@
 	/*
 	 * Do special thread processing, e.g. upcall tweaking and such.
 	 */
-	if (p->p_flag & P_SA) {
+	if (p->p_flag & P_SA)
 		thread_userret(td, frame);
-	}
 
 	/*
 	 * Charge system time if profiling.


More information about the p4-projects mailing list