From owner-p4-projects@FreeBSD.ORG Mon Feb 20 22:41:24 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 17D3816A423; Mon, 20 Feb 2006 22:41:24 +0000 (GMT) X-Original-To: perforce@freebsd.org 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 C9A0F16A420 for ; Mon, 20 Feb 2006 22:41:23 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8B7C343D48 for ; Mon, 20 Feb 2006 22:41:23 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.1/8.13.1) with ESMTP id k1KMfNi7063031 for ; Mon, 20 Feb 2006 22:41:23 GMT (envelope-from jhb@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.1/8.13.1/Submit) id k1KMfNo8063028 for perforce@freebsd.org; Mon, 20 Feb 2006 22:41:23 GMT (envelope-from jhb@freebsd.org) Date: Mon, 20 Feb 2006 22:41:23 GMT Message-Id: <200602202241.k1KMfNo8063028@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jhb@freebsd.org using -f From: John Baldwin To: Perforce Change Reviews Cc: Subject: PERFORCE change 92099 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Feb 2006 22:41:24 -0000 http://perforce.freebsd.org/chv.cgi?CH=92099 Change 92099 by jhb@jhb_slimer on 2006/02/20 22:40:44 - Do a single PHOLD around the actions of kern_ptrace(). - Fix the default error case to be in the default: case so we can just use break instead of goto out. Simplify a lot of cleanup and unlocking code as a result. Many ptrace actions are now back to just ptrace_foo(); break; - Revert PHOLD/PRELE in FIX_SSTEP as all callers already do it now. Affected files ... .. //depot/projects/smpng/sys/alpha/include/ptrace.h#10 edit .. //depot/projects/smpng/sys/kern/sys_process.c#51 edit Differences ... ==== //depot/projects/smpng/sys/alpha/include/ptrace.h#10 (text+ko) ==== @@ -34,11 +34,7 @@ #define _MACHINE_PTRACE_H_ #ifdef _KERNEL -#define FIX_SSTEP(p) do { \ - _PHOLD((p)); \ - ptrace_clear_single_step((p)); \ - _PRELE((p)); \ -} while (0) +#define FIX_SSTEP(p) ptrace_clear_single_step(p) #endif #endif ==== //depot/projects/smpng/sys/kern/sys_process.c#51 (text+ko) ==== @@ -658,11 +658,14 @@ break; } + /* Keep this process around until we finish this request. */ + _PHOLD(p); + #ifdef FIX_SSTEP /* * Single step fixup ala procfs */ - FIX_SSTEP(td2); /* XXXKSE */ + FIX_SSTEP(td2); #endif /* @@ -676,9 +679,7 @@ /* set my trace flag and "owner" so it can read/write me */ p->p_flag |= P_TRACED; p->p_oppid = p->p_pptr->p_pid; - PROC_UNLOCK(p); - sx_xunlock(&proctree_lock); - return (0); + break; case PT_ATTACH: /* security check done above */ @@ -694,36 +695,24 @@ goto sendsig; /* in PT_CONTINUE below */ case PT_CLEARSTEP: - _PHOLD(p); error = ptrace_clear_single_step(td2); - _PRELE(p); - if (error) - goto fail; - PROC_UNLOCK(p); - return (0); + break; case PT_SETSTEP: - _PHOLD(p); error = ptrace_single_step(td2); - _PRELE(p); - if (error) - goto fail; - PROC_UNLOCK(p); - return (0); + break; case PT_SUSPEND: mtx_lock_spin(&sched_lock); td2->td_flags |= TDF_DBSUSPEND; mtx_unlock_spin(&sched_lock); - PROC_UNLOCK(p); - return (0); + break; case PT_RESUME: mtx_lock_spin(&sched_lock); td2->td_flags &= ~TDF_DBSUSPEND; mtx_unlock_spin(&sched_lock); - PROC_UNLOCK(p); - return (0); + break; case PT_STEP: case PT_CONTINUE: @@ -734,18 +723,14 @@ /* Zero means do not send any signal */ if (data < 0 || data > _SIG_MAXSIG) { error = EINVAL; - goto fail; + break; } - _PHOLD(p); - switch (req) { case PT_STEP: error = ptrace_single_step(td2); - if (error) { - _PRELE(p); - goto fail; - } + if (error) + goto out; break; case PT_TO_SCE: p->p_stops |= S_PT_SCE; @@ -760,12 +745,9 @@ if (addr != (void *)1) { error = ptrace_set_pc(td2, (u_long)(uintfptr_t)addr); - if (error) { - _PRELE(p); - goto fail; - } + if (error) + break; } - _PRELE(p); if (req == PT_DETACH) { /* reset process parent */ @@ -795,8 +777,10 @@ } sendsig: - if (proctree_locked) + if (proctree_locked) { sx_xunlock(&proctree_lock); + proctree_locked = 0; + } /* deliver or queue signal */ mtx_lock_spin(&sched_lock); td2->td_flags &= ~TDF_XSIG; @@ -827,8 +811,7 @@ if (data) psignal(p, data); - PROC_UNLOCK(p); - return (0); + break; case PT_WRITE_I: case PT_WRITE_D: @@ -836,7 +819,6 @@ /* FALLTHROUGH */ case PT_READ_I: case PT_READ_D: - _PHOLD(p); PROC_UNLOCK(p); tmp = 0; /* write = 0 set above */ @@ -850,7 +832,6 @@ uio.uio_rw = write ? UIO_WRITE : UIO_READ; uio.uio_td = td; error = proc_rwmem(p, &uio); - PRELE(p); if (uio.uio_resid != 0) { /* * XXX proc_rwmem() doesn't currently return ENOSPC, @@ -866,11 +847,10 @@ } if (!write) td->td_retval[0] = tmp; - return (error); + PROC_LOCK(p); + break; case PT_IO: - _PHOLD(p); - PROC_UNLOCK(p); #ifdef COMPAT_IA32 if (wrap32) { piod32 = addr; @@ -906,69 +886,52 @@ uio.uio_rw = UIO_WRITE; break; default: - PRELE(p); - return (EINVAL); + error = EINVAL; + goto out; } + PROC_UNLOCK(p); error = proc_rwmem(p, &uio); - PRELE(p); #ifdef COMPAT_IA32 if (wrap32) piod32->piod_len -= uio.uio_resid; else #endif piod->piod_len -= uio.uio_resid; - return (error); + PROC_LOCK(p); + break; case PT_KILL: data = SIGKILL; goto sendsig; /* in PT_CONTINUE above */ case PT_SETREGS: - _PHOLD(p); error = PROC_WRITE(regs, td2, addr); - _PRELE(p); - PROC_UNLOCK(p); - return (error); + break; case PT_GETREGS: - _PHOLD(p); error = PROC_READ(regs, td2, addr); - _PRELE(p); - PROC_UNLOCK(p); - return (error); + break; case PT_SETFPREGS: - _PHOLD(p); error = PROC_WRITE(fpregs, td2, addr); - _PRELE(p); - PROC_UNLOCK(p); - return (error); + break; case PT_GETFPREGS: - _PHOLD(p); error = PROC_READ(fpregs, td2, addr); - _PRELE(p); - PROC_UNLOCK(p); - return (error); + break; case PT_SETDBREGS: - _PHOLD(p); error = PROC_WRITE(dbregs, td2, addr); - _PRELE(p); - PROC_UNLOCK(p); - return (error); + break; case PT_GETDBREGS: - _PHOLD(p); error = PROC_READ(dbregs, td2, addr); - _PRELE(p); - PROC_UNLOCK(p); - return (error); + break; case PT_LWPINFO: if (data == 0 || data > sizeof(*pl)) { error = EINVAL; - goto fail; + break; } pl = addr; pl->pl_lwpid = td2->td_tid; @@ -983,21 +946,18 @@ } else { pl->pl_flags = 0; } - PROC_UNLOCK(p); - return (0); + break; case PT_GETNUMLWPS: td->td_retval[0] = p->p_numthreads; - PROC_UNLOCK(p); - return (0); + break; case PT_GETLWPLIST: if (data <= 0) { error = EINVAL; - goto fail; + break; } num = imin(p->p_numthreads, data); - _PHOLD(p); PROC_UNLOCK(p); buf = malloc(num * sizeof(lwpid_t), M_TEMP, M_WAITOK); tmp = 0; @@ -1009,30 +969,30 @@ buf[tmp++] = td2->td_tid; } mtx_unlock_spin(&sched_lock); - _PRELE(p); PROC_UNLOCK(p); error = copyout(buf, addr, tmp * sizeof(lwpid_t)); free(buf, M_TEMP); if (!error) td->td_retval[0] = num; - return (error); + PROC_LOCK(p); + break; default: #ifdef __HAVE_PTRACE_MACHDEP if (req >= PT_FIRSTMACH) { - _PHOLD(p); PROC_UNLOCK(p); error = cpu_ptrace(td2, req, addr, data); - PRELE(p); - return (error); - } + PROC_LOCK(p); + } else #endif + /* Unknown request. */ + error = EINVAL; break; } - /* Unknown request. */ - error = EINVAL; - +out: + /* Drop our hold on this process now that the request has completed. */ + _PRELE(p); fail: PROC_UNLOCK(p); if (proctree_locked)