Date: Mon, 20 Feb 2006 22:41:23 GMT From: John Baldwin <jhb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 92099 for review Message-ID: <200602202241.k1KMfNo8063028@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200602202241.k1KMfNo8063028>