From owner-p4-projects@FreeBSD.ORG Sat Mar 6 20:12:38 2004 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 607B216A4D0; Sat, 6 Mar 2004 20:12:38 -0800 (PST) Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3C08316A4CE for ; Sat, 6 Mar 2004 20:12:38 -0800 (PST) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2EF1C43D2D for ; Sat, 6 Mar 2004 20:12:38 -0800 (PST) (envelope-from peter@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.10/8.12.10) with ESMTP id i274CcGe044042 for ; Sat, 6 Mar 2004 20:12:38 -0800 (PST) (envelope-from peter@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.10/8.12.10/Submit) id i274CbdO044039 for perforce@freebsd.org; Sat, 6 Mar 2004 20:12:37 -0800 (PST) (envelope-from peter@freebsd.org) Date: Sat, 6 Mar 2004 20:12:37 -0800 (PST) Message-Id: <200403070412.i274CbdO044039@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to peter@freebsd.org using -f From: Peter Wemm To: Perforce Change Reviews Subject: PERFORCE change 48321 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Mar 2004 04:12:38 -0000 http://perforce.freebsd.org/chv.cgi?CH=48321 Change 48321 by peter@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.