Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 18 Nov 2013 16:51:56 +0000 (UTC)
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r258311 - head/sys/cddl/contrib/opensolaris/uts/common/dtrace
Message-ID:  <201311181651.rAIGpuu4088968@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: asomers
Date: Mon Nov 18 16:51:56 2013
New Revision: 258311
URL: http://svnweb.freebsd.org/changeset/base/258311

Log:
  opensolaris/uts/common/dtrace/fasttrap.c
  	Fix several problems that can cause panics on kldload and kldunload.
  
  	* kproc_create(fasttrap_pid_cleanup_cb, ...) gets called before
  	  fasttrap_provs.fth_table gets allocated.  This can lead to a panic
  	  on module load, because fasttrap_pid_cleanup_cb references
  	  fasttrap_provs.fth_table.  Move kproc_create down after the point
  	  that fasttrap_provs.fth_table gets allocated, and modify the error
  	  handling accordingly.
  
  	* dtrace_fasttrap_{fork,exec,exit} weren't getting NULLed until
  	  after fasttrap_provs.fth_table got freed.  That caused panics on
  	  module unload because fasttrap_exec_exit calls
  	  fasttrap_provider_retire, which references
  	  fasttrap_provs.fth_table.  NULL those function pointers earlier.
  
  	* There wasn't any code to destroy the
  	  fasttrap_{tpoints,provs,procs}.fth_table mutexes on module unload,
  	  leading to a resource leak when WITNESS is enabled.  Destroy those
  	  mutexes during fasttrap_unload().
  
  Reviewed by:	markj
  Approved by:	ken (mentor)
  Sponsored by:	Spectra Logic
  MFC after:	4 weeks

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

Modified: head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c	Mon Nov 18 16:25:56 2013	(r258310)
+++ head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c	Mon Nov 18 16:51:56 2013	(r258311)
@@ -2271,13 +2271,6 @@ fasttrap_load(void)
 	mutex_init(&fasttrap_count_mtx, "fasttrap count mtx", MUTEX_DEFAULT,
 	    NULL);
 
-	ret = kproc_create(fasttrap_pid_cleanup_cb, NULL,
-	    &fasttrap_cleanup_proc, 0, 0, "ftcleanup");
-	if (ret != 0) {
-		destroy_dev(fasttrap_cdev);
-		return (ret);
-	}
-
 #if defined(sun)
 	fasttrap_max = ddi_getprop(DDI_DEV_T_ANY, devi, DDI_PROP_DONTPASS,
 	    "fasttrap-max-probes", FASTTRAP_MAX_DEFAULT);
@@ -2331,6 +2324,24 @@ fasttrap_load(void)
 		    "providers bucket mtx", MUTEX_DEFAULT, NULL);
 #endif
 
+	ret = kproc_create(fasttrap_pid_cleanup_cb, NULL,
+	    &fasttrap_cleanup_proc, 0, 0, "ftcleanup");
+	if (ret != 0) {
+		destroy_dev(fasttrap_cdev);
+#if !defined(sun)
+		for (i = 0; i < fasttrap_provs.fth_nent; i++)
+			mutex_destroy(&fasttrap_provs.fth_table[i].ftb_mtx);
+		for (i = 0; i < fasttrap_tpoints.fth_nent; i++)
+			mutex_destroy(&fasttrap_tpoints.fth_table[i].ftb_mtx);
+#endif
+		kmem_free(fasttrap_provs.fth_table, fasttrap_provs.fth_nent *
+		    sizeof (fasttrap_bucket_t));
+		mtx_destroy(&fasttrap_cleanup_mtx);
+		mutex_destroy(&fasttrap_count_mtx);
+		return (ret);
+	}
+
+
 	/*
 	 * ... and the procs hash table.
 	 */
@@ -2423,6 +2434,20 @@ fasttrap_unload(void)
 		return (-1);
 	}
 
+	/*
+	 * Stop new processes from entering these hooks now, before the
+	 * fasttrap_cleanup thread runs.  That way all processes will hopefully
+	 * be out of these hooks before we free fasttrap_provs.fth_table
+	 */
+	ASSERT(dtrace_fasttrap_fork == &fasttrap_fork);
+	dtrace_fasttrap_fork = NULL;
+
+	ASSERT(dtrace_fasttrap_exec == &fasttrap_exec_exit);
+	dtrace_fasttrap_exec = NULL;
+
+	ASSERT(dtrace_fasttrap_exit == &fasttrap_exec_exit);
+	dtrace_fasttrap_exit = NULL;
+
 	mtx_lock(&fasttrap_cleanup_mtx);
 	fasttrap_cleanup_drain = 1;
 	/* Wait for the cleanup thread to finish up and signal us. */
@@ -2438,6 +2463,14 @@ fasttrap_unload(void)
 	mutex_exit(&fasttrap_count_mtx);
 #endif
 
+#if !defined(sun)
+	for (i = 0; i < fasttrap_tpoints.fth_nent; i++)
+		mutex_destroy(&fasttrap_tpoints.fth_table[i].ftb_mtx);
+	for (i = 0; i < fasttrap_provs.fth_nent; i++)
+		mutex_destroy(&fasttrap_provs.fth_table[i].ftb_mtx);
+	for (i = 0; i < fasttrap_procs.fth_nent; i++)
+		mutex_destroy(&fasttrap_procs.fth_table[i].ftb_mtx);
+#endif
 	kmem_free(fasttrap_tpoints.fth_table,
 	    fasttrap_tpoints.fth_nent * sizeof (fasttrap_bucket_t));
 	fasttrap_tpoints.fth_nent = 0;
@@ -2450,22 +2483,6 @@ fasttrap_unload(void)
 	    fasttrap_procs.fth_nent * sizeof (fasttrap_bucket_t));
 	fasttrap_procs.fth_nent = 0;
 
-	/*
-	 * We know there are no tracepoints in any process anywhere in
-	 * the system so there is no process which has its p_dtrace_count
-	 * greater than zero, therefore we know that no thread can actively
-	 * be executing code in fasttrap_fork(). Similarly for p_dtrace_probes
-	 * and fasttrap_exec() and fasttrap_exit().
-	 */
-	ASSERT(dtrace_fasttrap_fork == &fasttrap_fork);
-	dtrace_fasttrap_fork = NULL;
-
-	ASSERT(dtrace_fasttrap_exec == &fasttrap_exec_exit);
-	dtrace_fasttrap_exec = NULL;
-
-	ASSERT(dtrace_fasttrap_exit == &fasttrap_exec_exit);
-	dtrace_fasttrap_exit = NULL;
-
 #if !defined(sun)
 	destroy_dev(fasttrap_cdev);
 	mutex_destroy(&fasttrap_count_mtx);



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