Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 12 Sep 2010 14:12:16 +0000 (UTC)
From:      Rui Paulo <rpaulo@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r212494 - in head/sys/cddl/contrib/opensolaris/uts: common/dtrace intel/dtrace
Message-ID:  <201009121412.o8CECG9U051005@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: rpaulo
Date: Sun Sep 12 14:12:16 2010
New Revision: 212494
URL: http://svn.freebsd.org/changeset/base/212494

Log:
  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

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c
  head/sys/cddl/contrib/opensolaris/uts/intel/dtrace/fasttrap_isa.c

Modified: head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c	Sun Sep 12 14:04:54 2010	(r212493)
+++ head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c	Sun Sep 12 14:12:16 2010	(r212494)
@@ -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, fa
 	 * 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 @@ again:
 		 */
 #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, f
 		 */
 #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
 	 * 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
 			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
 	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_i
 		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_i
 			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;
 

Modified: head/sys/cddl/contrib/opensolaris/uts/intel/dtrace/fasttrap_isa.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/intel/dtrace/fasttrap_isa.c	Sun Sep 12 14:04:54 2010	(r212493)
+++ head/sys/cddl/contrib/opensolaris/uts/intel/dtrace/fasttrap_isa.c	Sun Sep 12 14:12:16 2010	(r212494)
@@ -74,15 +74,12 @@ proc_ops(int op, proc_t *p, void *kaddr,
 	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);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201009121412.o8CECG9U051005>