Revamp locking a bit. This fixes three problems:

* processes now can't go away while we are inserting probes (fixes a panic)
* if a trap happens, we won't be holding the process lock (fixes a hang)
* fix a LOR between the process lock and the fasttrap bucket list lock

Thanks to kib for pointing some problems.
Sponsored by:	The FreeBSD Foundation
This commit is contained in:
Rui Paulo 2010-09-12 14:12:16 +00:00
parent f3f9858578
commit 47047e3418
2 changed files with 26 additions and 18 deletions

View File

@ -460,11 +460,13 @@ fasttrap_fork(proc_t *p, proc_t *cp)
/*
* dtrace_helpers_duplicate() allocates memory.
*/
_PHOLD(cp);
PROC_UNLOCK(p);
PROC_UNLOCK(cp);
dtrace_helpers_duplicate(p, cp);
PROC_LOCK(cp);
PROC_LOCK(p);
_PRELE(cp);
}
/*
* This check is purposely here instead of in kern_fork.c because,
@ -494,6 +496,8 @@ fasttrap_fork(proc_t *p, proc_t *cp)
mtx_lock_spin(&cp->p_slock);
sprlock_proc(cp);
mtx_unlock_spin(&cp->p_slock);
#else
_PHOLD(cp);
#endif
/*
@ -527,6 +531,8 @@ fasttrap_fork(proc_t *p, proc_t *cp)
#if defined(sun)
mutex_enter(&cp->p_lock);
sprunlock(cp);
#else
_PRELE(cp);
#endif
}
@ -542,6 +548,7 @@ fasttrap_exec_exit(proc_t *p)
ASSERT(p == curproc);
#endif
PROC_LOCK_ASSERT(p, MA_OWNED);
_PHOLD(p);
PROC_UNLOCK(p);
/*
@ -554,6 +561,7 @@ fasttrap_exec_exit(proc_t *p)
dtrace_helpers_destroy(p);
#endif
PROC_LOCK(p);
_PRELE(p);
}
@ -591,9 +599,7 @@ fasttrap_tracepoint_enable(proc_t *p, fasttrap_probe_t *probe, uint_t index)
* Before we make any modifications, make sure we've imposed a barrier
* on the generation in which this probe was last modified.
*/
PROC_UNLOCK(p);
fasttrap_mod_barrier(probe->ftp_gen);
PROC_LOCK(p);
bucket = &fasttrap_tpoints.fth_table[FASTTRAP_TPOINTS_INDEX(pid, pc)];
@ -695,8 +701,6 @@ fasttrap_tracepoint_enable(proc_t *p, fasttrap_probe_t *probe, uint_t index)
*/
#if defined(sun)
ASSERT(p->p_proc_flag & P_PR_LOCK);
#else
PROC_LOCK_ASSERT(p, MA_OWNED);
#endif
p->p_dtrace_count++;
@ -889,8 +893,6 @@ fasttrap_tracepoint_disable(proc_t *p, fasttrap_probe_t *probe, uint_t index)
*/
#if defined(sun)
ASSERT(p->p_proc_flag & P_PR_LOCK);
#else
PROC_LOCK_ASSERT(p, MA_OWNED);
#endif
p->p_dtrace_count--;
}
@ -1044,9 +1046,14 @@ fasttrap_pid_enable(void *arg, dtrace_id_t id, void *parg)
* the chance to execute the trap instruction we're about to place
* in their process's text.
*/
#ifdef __FreeBSD__
/*
* pfind() returns a locked process.
*/
_PHOLD(p);
PROC_UNLOCK(p);
#endif
fasttrap_enable_callbacks();
PROC_LOCK(p);
/*
* Enable all the tracepoints and add this probe's id to each
@ -1077,7 +1084,7 @@ fasttrap_pid_enable(void *arg, dtrace_id_t id, void *parg)
mutex_enter(&p->p_lock);
sprunlock(p);
#else
PROC_UNLOCK(p);
PRELE(p);
#endif
/*
@ -1092,7 +1099,7 @@ fasttrap_pid_enable(void *arg, dtrace_id_t id, void *parg)
mutex_enter(&p->p_lock);
sprunlock(p);
#else
PROC_UNLOCK(p);
PRELE(p);
#endif
probe->ftp_enabled = 1;
@ -1121,6 +1128,10 @@ fasttrap_pid_disable(void *arg, dtrace_id_t id, void *parg)
mutex_exit(&provider->ftp_mtx);
return;
}
#ifdef __FreeBSD__
_PHOLD(p);
PROC_UNLOCK(p);
#endif
/*
* Disable all the associated tracepoints (for fully enabled probes).
@ -1152,13 +1163,13 @@ fasttrap_pid_disable(void *arg, dtrace_id_t id, void *parg)
whack = provider->ftp_marked = 1;
mutex_exit(&provider->ftp_mtx);
}
#if !defined(sun)
PROC_UNLOCK(p);
#endif
if (whack)
fasttrap_pid_cleanup();
#ifdef __FreeBSD__
PRELE(p);
#endif
if (!probe->ftp_enabled)
return;

View File

@ -74,15 +74,12 @@ proc_ops(int op, proc_t *p, void *kaddr, off_t uaddr, size_t len)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_td = curthread;
uio.uio_rw = op;
_PHOLD(p);
PROC_UNLOCK(p);
PHOLD(p);
if (proc_rwmem(p, &uio) < 0) {
PROC_LOCK(p);
_PRELE(p);
PRELE(p);
return (-1);
}
PROC_LOCK(p);
_PRELE(p);
PRELE(p);
return (0);
}