From owner-svn-src-stable-9@FreeBSD.ORG Fri Sep 20 23:50:15 2013 Return-Path: Delivered-To: svn-src-stable-9@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 3E8ABA26; Fri, 20 Sep 2013 23:50:15 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 29DB62A56; Fri, 20 Sep 2013 23:50:15 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.7/8.14.7) with ESMTP id r8KNoFCb079162; Fri, 20 Sep 2013 23:50:15 GMT (envelope-from markj@svn.freebsd.org) Received: (from markj@localhost) by svn.freebsd.org (8.14.7/8.14.5/Submit) id r8KNoFsd079161; Fri, 20 Sep 2013 23:50:15 GMT (envelope-from markj@svn.freebsd.org) Message-Id: <201309202350.r8KNoFsd079161@svn.freebsd.org> From: Mark Johnston Date: Fri, 20 Sep 2013 23:50:15 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r255749 - stable/9/sys/cddl/contrib/opensolaris/uts/common/dtrace X-SVN-Group: stable-9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-9@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for only the 9-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Sep 2013 23:50:15 -0000 Author: markj Date: Fri Sep 20 23:50:14 2013 New Revision: 255749 URL: http://svnweb.freebsd.org/changeset/base/255749 Log: MFC r250953: The fasttrap provider cleans up probes asynchronously when a process with USDT probes exits. This was previously done with a callout; however, it is possible to sleep while holding the DTrace mutexes, so a panic will occur on INVARIANTS kernels if the callout handler can't immediately acquire one of these mutexes. This panic will be frequently triggered on systems where a USDT-enabled program (perl, for instance) is often run. This revision changes the fasttrap cleanup mechanism so that a dedicated thread is used instead of a callout. The old behaviour is otherwise preserved. MFC r252493: Be sure to destory the fasttrap cleanup mutex when unloading the fasttrap module. This should be MFCed with r250953. Modified: stable/9/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c Directory Properties: stable/9/sys/ (props changed) stable/9/sys/cddl/contrib/opensolaris/ (props changed) Modified: stable/9/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c ============================================================================== --- stable/9/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c Fri Sep 20 23:22:00 2013 (r255748) +++ stable/9/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c Fri Sep 20 23:50:14 2013 (r255749) @@ -155,9 +155,9 @@ static struct cdevsw fasttrap_cdevsw = { static struct cdev *fasttrap_cdev; static dtrace_meta_provider_id_t fasttrap_meta_id; -static struct callout fasttrap_timeout; +static struct proc *fasttrap_cleanup_proc; static struct mtx fasttrap_cleanup_mtx; -static uint_t fasttrap_cleanup_work; +static uint_t fasttrap_cleanup_work, fasttrap_cleanup_drain, fasttrap_cleanup_cv; /* * Generation count on modifications to the global tracepoint lookup table. @@ -310,8 +310,11 @@ fasttrap_mod_barrier(uint64_t gen) } /* - * This is the timeout's callback for cleaning up the providers and their - * probes. + * This function performs asynchronous cleanup of fasttrap providers. The + * Solaris implementation of this mechanism use a timeout that's activated in + * fasttrap_pid_cleanup(), but this doesn't work in FreeBSD: one may sleep while + * holding the DTrace mutexes, but it is unsafe to sleep in a callout handler. + * Thus we use a dedicated process to perform the cleanup when requested. */ /*ARGSUSED*/ static void @@ -322,11 +325,8 @@ fasttrap_pid_cleanup_cb(void *data) dtrace_provider_id_t provid; int i, later = 0, rval; - static volatile int in = 0; - ASSERT(in == 0); - in = 1; - - while (fasttrap_cleanup_work) { + mtx_lock(&fasttrap_cleanup_mtx); + while (!fasttrap_cleanup_drain || later > 0) { fasttrap_cleanup_work = 0; mtx_unlock(&fasttrap_cleanup_mtx); @@ -397,39 +397,32 @@ fasttrap_pid_cleanup_cb(void *data) } mutex_exit(&bucket->ftb_mtx); } - mtx_lock(&fasttrap_cleanup_mtx); - } -#if 0 - ASSERT(fasttrap_timeout != 0); -#endif + /* + * If we were unable to retire a provider, try again after a + * second. This situation can occur in certain circumstances + * where providers cannot be unregistered even though they have + * no probes enabled because of an execution of dtrace -l or + * something similar. + */ + if (later > 0 || fasttrap_cleanup_work || + fasttrap_cleanup_drain) { + mtx_unlock(&fasttrap_cleanup_mtx); + pause("ftclean", hz); + mtx_lock(&fasttrap_cleanup_mtx); + } else + mtx_sleep(&fasttrap_cleanup_cv, &fasttrap_cleanup_mtx, + 0, "ftcl", 0); + } /* - * If we were unable to remove a retired provider, try again after - * a second. This situation can occur in certain circumstances where - * providers cannot be unregistered even though they have no probes - * enabled because of an execution of dtrace -l or something similar. - * If the timeout has been disabled (set to 1 because we're trying - * to detach), we set fasttrap_cleanup_work to ensure that we'll - * get a chance to do that work if and when the timeout is reenabled - * (if detach fails). - */ - if (later > 0) { - if (callout_active(&fasttrap_timeout)) { - callout_reset(&fasttrap_timeout, hz, - &fasttrap_pid_cleanup_cb, NULL); - } - - else if (later > 0) - fasttrap_cleanup_work = 1; - } else { -#if !defined(sun) - /* Nothing to be done for FreeBSD */ -#endif - } + * Wake up the thread in fasttrap_unload() now that we're done. + */ + wakeup(&fasttrap_cleanup_drain); + mtx_unlock(&fasttrap_cleanup_mtx); - in = 0; + kthread_exit(); } /* @@ -440,8 +433,10 @@ fasttrap_pid_cleanup(void) { mtx_lock(&fasttrap_cleanup_mtx); - fasttrap_cleanup_work = 1; - callout_reset(&fasttrap_timeout, 1, &fasttrap_pid_cleanup_cb, NULL); + if (!fasttrap_cleanup_work) { + fasttrap_cleanup_work = 1; + wakeup(&fasttrap_cleanup_cv); + } mtx_unlock(&fasttrap_cleanup_mtx); } @@ -991,7 +986,6 @@ fasttrap_pid_enable(void *arg, dtrace_id proc_t *p = NULL; int i, rc; - ASSERT(probe != NULL); ASSERT(!probe->ftp_enabled); ASSERT(id == probe->ftp_id); @@ -2272,17 +2266,23 @@ static int fasttrap_load(void) { ulong_t nent; - int i; + int i, ret; /* Create the /dev/dtrace/fasttrap entry. */ fasttrap_cdev = make_dev(&fasttrap_cdevsw, 0, UID_ROOT, GID_WHEEL, 0600, "dtrace/fasttrap"); mtx_init(&fasttrap_cleanup_mtx, "fasttrap clean", "dtrace", MTX_DEF); - callout_init_mtx(&fasttrap_timeout, &fasttrap_cleanup_mtx, 0); 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); @@ -2389,15 +2389,6 @@ fasttrap_unload(void) return (-1); /* - * Prevent any new timeouts from running by setting fasttrap_timeout - * to a non-zero value, and wait for the current timeout to complete. - */ - mtx_lock(&fasttrap_cleanup_mtx); - fasttrap_cleanup_work = 0; - callout_drain(&fasttrap_timeout); - mtx_unlock(&fasttrap_cleanup_mtx); - - /* * Iterate over all of our providers. If there's still a process * that corresponds to that pid, fail to detach. */ @@ -2431,26 +2422,21 @@ fasttrap_unload(void) } if (fail) { - uint_t work; - /* - * If we're failing to detach, we need to unblock timeouts - * and start a new timeout if any work has accumulated while - * we've been unsuccessfully trying to detach. - */ - mtx_lock(&fasttrap_cleanup_mtx); - work = fasttrap_cleanup_work; - callout_drain(&fasttrap_timeout); - mtx_unlock(&fasttrap_cleanup_mtx); - - if (work) - fasttrap_pid_cleanup(); - (void) dtrace_meta_register("fasttrap", &fasttrap_mops, NULL, &fasttrap_meta_id); return (-1); } + mtx_lock(&fasttrap_cleanup_mtx); + fasttrap_cleanup_drain = 1; + /* Wait for the cleanup thread to finish up and signal us. */ + wakeup(&fasttrap_cleanup_cv); + mtx_sleep(&fasttrap_cleanup_drain, &fasttrap_cleanup_mtx, 0, "ftcld", + 0); + fasttrap_cleanup_proc = NULL; + mtx_destroy(&fasttrap_cleanup_mtx); + #ifdef DEBUG mutex_enter(&fasttrap_count_mtx); ASSERT(fasttrap_pid_count == 0);