From f484800de70343e19872fa0f3fde2a00504a9cec Mon Sep 17 00:00:00 2001 From: Bryan Cantrill Date: Fri, 1 Jul 2011 22:35:53 -0700 Subject: [PATCH] 1368 enablings on defunct providers prevent providers from unregistering Reviewed by: Adam Leventhal Approved by: Garrett D'Amore --- usr/src/cmd/dtrace/test/cmd/jdtrace/exception.lst | 6 +- .../cmd/dtrace/test/tst/common/usdt/tst.noreap.ksh | 128 +++++++++++++++++++ .../dtrace/test/tst/common/usdt/tst.noreapring.ksh | 124 ++++++++++++++++++ .../cmd/dtrace/test/tst/common/usdt/tst.reap.ksh | 115 +++++++++++++++++ usr/src/pkg/manifests/system-dtrace-tests.mf | 3 + usr/src/uts/common/dtrace/dtrace.c | 140 ++++++++++++++++++++- usr/src/uts/common/dtrace/fasttrap.c | 23 +++- usr/src/uts/common/sys/dtrace_impl.h | 13 +- 8 files changed, 536 insertions(+), 16 deletions(-) create mode 100644 usr/src/cmd/dtrace/test/tst/common/usdt/tst.noreap.ksh create mode 100644 usr/src/cmd/dtrace/test/tst/common/usdt/tst.noreapring.ksh create mode 100644 usr/src/cmd/dtrace/test/tst/common/usdt/tst.reap.ksh diff --git a/usr/src/cmd/dtrace/test/cmd/jdtrace/exception.lst b/usr/src/cmd/dtrace/test/cmd/jdtrace/exception.lst index 261f8707c1..19fc3aca51 100644 --- a/usr/src/cmd/dtrace/test/cmd/jdtrace/exception.lst +++ b/usr/src/cmd/dtrace/test/cmd/jdtrace/exception.lst @@ -23,7 +23,6 @@ # Copyright 2008 Sun Microsystems, Inc. All rights reserved. # Use is subject to license terms. # -# ident "%Z%%M% %I% %E% SMI" # Exception list: names tests that are bypassed when running in Java # mode (relative to /opt/SUNWdtrt/tst) @@ -52,14 +51,17 @@ common/usdt/tst.enabled.ksh common/usdt/tst.enabled2.ksh common/usdt/tst.entryreturn.ksh common/usdt/tst.fork.ksh -common/usdt/tst.header.ksh common/usdt/tst.guess32.ksh common/usdt/tst.guess64.ksh +common/usdt/tst.header.ksh common/usdt/tst.linkpriv.ksh common/usdt/tst.linkunpriv.ksh common/usdt/tst.multiple.ksh common/usdt/tst.nodtrace.ksh +common/usdt/tst.noreap.ksh +common/usdt/tst.noreapring.ksh common/usdt/tst.onlyenabled.ksh +common/usdt/tst.reap.ksh common/usdt/tst.reeval.ksh common/usdt/tst.static.ksh common/usdt/tst.static2.ksh diff --git a/usr/src/cmd/dtrace/test/tst/common/usdt/tst.noreap.ksh b/usr/src/cmd/dtrace/test/tst/common/usdt/tst.noreap.ksh new file mode 100644 index 0000000000..0125d50b80 --- /dev/null +++ b/usr/src/cmd/dtrace/test/tst/common/usdt/tst.noreap.ksh @@ -0,0 +1,128 @@ +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or http://www.opensolaris.org/os/licensing. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright (c) 2011, Joyent, Inc. All rights reserved. +# + +if [ $# != 1 ]; then + echo expected one argument: '<'dtrace-path'>' + exit 2 +fi + +dtrace=$1 +DIR=/var/tmp/dtest.$$ + +mkdir $DIR +cd $DIR + +cat > test.c < +#include + +int +main(int argc, char **argv) +{ + DTRACE_PROBE(test_prov, probe1); +} +EOF + +cat > prov.d < 10/ + { + exit(0); + } +EOF +} + +script 2>&1 | tee test.out + +# +# It should be true that our probe was not reaped after the provider was made +# defunct: the speculative tracing action prevents reaping of any ECB in the +# enabling. +# +status=0 + +if grep D_PDESC_INVAL test.out 2> /dev/null 1>&2 ; then + status=1 +else + grep D_PROC_GRAB test.out 2> /dev/null 1>&2 + status=$? +fi + +cd / +/usr/bin/rm -rf $DIR + +exit $status diff --git a/usr/src/cmd/dtrace/test/tst/common/usdt/tst.noreapring.ksh b/usr/src/cmd/dtrace/test/tst/common/usdt/tst.noreapring.ksh new file mode 100644 index 0000000000..1260903467 --- /dev/null +++ b/usr/src/cmd/dtrace/test/tst/common/usdt/tst.noreapring.ksh @@ -0,0 +1,124 @@ +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or http://www.opensolaris.org/os/licensing. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright (c) 2011, Joyent, Inc. All rights reserved. +# + +if [ $# != 1 ]; then + echo expected one argument: '<'dtrace-path'>' + exit 2 +fi + +dtrace=$1 +DIR=/var/tmp/dtest.$$ + +mkdir $DIR +cd $DIR + +cat > test.c < +#include + +int +main(int argc, char **argv) +{ + DTRACE_PROBE(test_prov, probe1); +} +EOF + +cat > prov.d < 10/ + { + exit(0); + } +EOF +} + +$dtrace -x bufpolicy=ring -ZwqP test_prov\* > /dev/null 2>&1 & +background=$! +echo launched ring buffered enabling as pid $background +script 2>&1 | tee test.out + +# +# It should be true that our probe was not reaped after the provider was made +# defunct: the active ring buffer in the earlier enabling prevents reaping of +# any of the earlier enabling's ECBs. +# +status=0 + +if grep D_PDESC_INVAL test.out 2> /dev/null 1>&2 ; then + status=1 +else + grep D_PROC_GRAB test.out 2> /dev/null 1>&2 + status=$? +fi + +kill $background +cd / +/usr/bin/rm -rf $DIR + +exit $status diff --git a/usr/src/cmd/dtrace/test/tst/common/usdt/tst.reap.ksh b/usr/src/cmd/dtrace/test/tst/common/usdt/tst.reap.ksh new file mode 100644 index 0000000000..e9b96638fc --- /dev/null +++ b/usr/src/cmd/dtrace/test/tst/common/usdt/tst.reap.ksh @@ -0,0 +1,115 @@ +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or http://www.opensolaris.org/os/licensing. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright (c) 2011, Joyent, Inc. All rights reserved. +# + +if [ $# != 1 ]; then + echo expected one argument: '<'dtrace-path'>' + exit 2 +fi + +dtrace=$1 +DIR=/var/tmp/dtest.$$ + +mkdir $DIR +cd $DIR + +cat > test.c < +#include + +int +main(int argc, char **argv) +{ + DTRACE_PROBE(test_prov, probe1); +} +EOF + +cat > prov.d < 10/ + { + exit(0); + } +EOF +} + +script 2>&1 | tee test.out + +# +# It should be true that our probe was reaped over the course of the enabling, +# causing the embedded DTrace invocation to fail on an invalid probe (that is, +# D_PDESC_INVAL) instead of an inability to grab the underlying process +# (D_PROC_GRAB). +# +grep D_PDESC_INVAL test.out 2> /dev/null 1>&2 +status=$? + +cd / +/usr/bin/rm -rf $DIR + +exit $status diff --git a/usr/src/pkg/manifests/system-dtrace-tests.mf b/usr/src/pkg/manifests/system-dtrace-tests.mf index 233d78b624..41f730a6d6 100644 --- a/usr/src/pkg/manifests/system-dtrace-tests.mf +++ b/usr/src/pkg/manifests/system-dtrace-tests.mf @@ -1922,7 +1922,10 @@ file path=opt/SUNWdtrt/tst/common/usdt/tst.linkunpriv.ksh mode=0444 file path=opt/SUNWdtrt/tst/common/usdt/tst.multiple.ksh mode=0444 file path=opt/SUNWdtrt/tst/common/usdt/tst.multiple.ksh.out mode=0444 file path=opt/SUNWdtrt/tst/common/usdt/tst.nodtrace.ksh mode=0444 +file path=opt/SUNWdtrt/tst/common/usdt/tst.noreap.ksh mode=0444 +file path=opt/SUNWdtrt/tst/common/usdt/tst.noreapring.ksh mode=0444 file path=opt/SUNWdtrt/tst/common/usdt/tst.onlyenabled.ksh mode=0444 +file path=opt/SUNWdtrt/tst/common/usdt/tst.reap.ksh mode=0444 file path=opt/SUNWdtrt/tst/common/usdt/tst.reeval.ksh mode=0444 file path=opt/SUNWdtrt/tst/common/usdt/tst.static.ksh mode=0444 file path=opt/SUNWdtrt/tst/common/usdt/tst.static.ksh.out mode=0444 diff --git a/usr/src/uts/common/dtrace/dtrace.c b/usr/src/uts/common/dtrace/dtrace.c index e1251ff087..0edbd433ed 100644 --- a/usr/src/uts/common/dtrace/dtrace.c +++ b/usr/src/uts/common/dtrace/dtrace.c @@ -144,6 +144,7 @@ int dtrace_err_verbose; hrtime_t dtrace_deadman_interval = NANOSEC; hrtime_t dtrace_deadman_timeout = (hrtime_t)10 * NANOSEC; hrtime_t dtrace_deadman_user = (hrtime_t)30 * NANOSEC; +hrtime_t dtrace_unregister_defunct_reap = (hrtime_t)60 * NANOSEC; /* * DTrace External Variables @@ -460,11 +461,13 @@ static dtrace_probe_t *dtrace_probe_lookup_id(dtrace_id_t id); static void dtrace_enabling_provide(dtrace_provider_t *); static int dtrace_enabling_match(dtrace_enabling_t *, int *); static void dtrace_enabling_matchall(void); +static void dtrace_enabling_reap(void); static dtrace_state_t *dtrace_anon_grab(void); static uint64_t dtrace_helper(int, dtrace_mstate_t *, dtrace_state_t *, uint64_t, uint64_t); static dtrace_helpers_t *dtrace_helpers_create(proc_t *); static void dtrace_buffer_drop(dtrace_buffer_t *); +static int dtrace_buffer_consumed(dtrace_buffer_t *, hrtime_t when); static intptr_t dtrace_buffer_reserve(dtrace_buffer_t *, size_t, size_t, dtrace_state_t *, dtrace_mstate_t *); static int dtrace_state_option(dtrace_state_t *, dtrace_optid_t, @@ -7084,7 +7087,7 @@ dtrace_unregister(dtrace_provider_id_t id) { dtrace_provider_t *old = (dtrace_provider_t *)id; dtrace_provider_t *prev = NULL; - int i, self = 0; + int i, self = 0, noreap = 0; dtrace_probe_t *probe, *first = NULL; if (old->dtpv_pops.dtps_enable == @@ -7141,14 +7144,31 @@ dtrace_unregister(dtrace_provider_id_t id) continue; /* - * We have at least one ECB; we can't remove this provider. + * If we are trying to unregister a defunct provider, and the + * provider was made defunct within the interval dictated by + * dtrace_unregister_defunct_reap, we'll (asynchronously) + * attempt to reap our enablings. To denote that the provider + * should reattempt to unregister itself at some point in the + * future, we will return a differentiable error code (EAGAIN + * instead of EBUSY) in this case. */ + if (dtrace_gethrtime() - old->dtpv_defunct > + dtrace_unregister_defunct_reap) + noreap = 1; + if (!self) { mutex_exit(&dtrace_lock); mutex_exit(&mod_lock); mutex_exit(&dtrace_provider_lock); } - return (EBUSY); + + if (noreap) + return (EBUSY); + + (void) taskq_dispatch(dtrace_taskq, + (task_func_t *)dtrace_enabling_reap, NULL, TQ_SLEEP); + + return (EAGAIN); } /* @@ -7239,7 +7259,7 @@ dtrace_invalidate(dtrace_provider_id_t id) mutex_enter(&dtrace_provider_lock); mutex_enter(&dtrace_lock); - pvp->dtpv_defunct = 1; + pvp->dtpv_defunct = dtrace_gethrtime(); mutex_exit(&dtrace_lock); mutex_exit(&dtrace_provider_lock); @@ -10143,6 +10163,7 @@ dtrace_buffer_switch(dtrace_buffer_t *buf) caddr_t tomax = buf->dtb_tomax; caddr_t xamot = buf->dtb_xamot; dtrace_icookie_t cookie; + hrtime_t now = dtrace_gethrtime(); ASSERT(!(buf->dtb_flags & DTRACEBUF_NOSWITCH)); ASSERT(!(buf->dtb_flags & DTRACEBUF_RING)); @@ -10158,6 +10179,8 @@ dtrace_buffer_switch(dtrace_buffer_t *buf) buf->dtb_drops = 0; buf->dtb_errors = 0; buf->dtb_flags &= ~(DTRACEBUF_ERROR | DTRACEBUF_DROPPED); + buf->dtb_interval = now - buf->dtb_switched; + buf->dtb_switched = now; dtrace_interrupt_enable(cookie); } @@ -10565,6 +10588,36 @@ dtrace_buffer_polish(dtrace_buffer_t *buf) } } +/* + * This routine determines if data generated at the specified time has likely + * been entirely consumed at user-level. This routine is called to determine + * if an ECB on a defunct probe (but for an active enabling) can be safely + * disabled and destroyed. + */ +static int +dtrace_buffer_consumed(dtrace_buffer_t *bufs, hrtime_t when) +{ + int i; + + for (i = 0; i < NCPU; i++) { + dtrace_buffer_t *buf = &bufs[i]; + + if (buf->dtb_size == 0) + continue; + + if (buf->dtb_flags & DTRACEBUF_RING) + return (0); + + if (!buf->dtb_switched && buf->dtb_offset != 0) + return (0); + + if (buf->dtb_switched - buf->dtb_interval < when) + return (0); + } + + return (1); +} + static void dtrace_buffer_free(dtrace_buffer_t *bufs) { @@ -11055,6 +11108,85 @@ retry: } /* + * Called to reap ECBs that are attached to probes from defunct providers. + */ +static void +dtrace_enabling_reap(void) +{ + dtrace_provider_t *prov; + dtrace_probe_t *probe; + dtrace_ecb_t *ecb; + hrtime_t when; + int i; + + mutex_enter(&cpu_lock); + mutex_enter(&dtrace_lock); + + for (i = 0; i < dtrace_nprobes; i++) { + if ((probe = dtrace_probes[i]) == NULL) + continue; + + if (probe->dtpr_ecb == NULL) + continue; + + prov = probe->dtpr_provider; + + if ((when = prov->dtpv_defunct) == 0) + continue; + + /* + * We have ECBs on a defunct provider: we want to reap these + * ECBs to allow the provider to unregister. The destruction + * of these ECBs must be done carefully: if we destroy the ECB + * and the consumer later wishes to consume an EPID that + * corresponds to the destroyed ECB (and if the EPID metadata + * has not been previously consumed), the consumer will abort + * processing on the unknown EPID. To reduce (but not, sadly, + * eliminate) the possibility of this, we will only destroy an + * ECB for a defunct provider if, for the state that + * corresponds to the ECB: + * + * (a) There is no speculative tracing (which can effectively + * cache an EPID for an arbitrary amount of time). + * + * (b) The principal buffers have been switched twice since the + * provider became defunct. + * + * (c) The aggregation buffers are of zero size or have been + * switched twice since the provider became defunct. + * + * We use dts_speculates to determine (a) and call a function + * (dtrace_buffer_consumed()) to determine (b) and (c). Note + * that as soon as we've been unable to destroy one of the ECBs + * associated with the probe, we quit trying -- reaping is only + * fruitful in as much as we can destroy all ECBs associated + * with the defunct provider's probes. + */ + while ((ecb = probe->dtpr_ecb) != NULL) { + dtrace_state_t *state = ecb->dte_state; + dtrace_buffer_t *buf = state->dts_buffer; + dtrace_buffer_t *aggbuf = state->dts_aggbuffer; + + if (state->dts_speculates) + break; + + if (!dtrace_buffer_consumed(buf, when)) + break; + + if (!dtrace_buffer_consumed(aggbuf, when)) + break; + + dtrace_ecb_disable(ecb); + ASSERT(probe->dtpr_ecb != ecb); + dtrace_ecb_destroy(ecb); + } + } + + mutex_exit(&dtrace_lock); + mutex_exit(&cpu_lock); +} + +/* * DTrace DOF Functions */ /*ARGSUSED*/ diff --git a/usr/src/uts/common/dtrace/fasttrap.c b/usr/src/uts/common/dtrace/fasttrap.c index 42263e4ef2..8cfe4cd33b 100644 --- a/usr/src/uts/common/dtrace/fasttrap.c +++ b/usr/src/uts/common/dtrace/fasttrap.c @@ -24,6 +24,9 @@ * Use is subject to license terms. */ +/* + * Copyright (c) 2011, Joyent, Inc. All rights reserved. + */ #include #include @@ -273,7 +276,7 @@ fasttrap_pid_cleanup_cb(void *data) fasttrap_provider_t **fpp, *fp; fasttrap_bucket_t *bucket; dtrace_provider_id_t provid; - int i, later; + int i, later, rval; static volatile int in = 0; ASSERT(in == 0); @@ -335,9 +338,13 @@ fasttrap_pid_cleanup_cb(void *data) * clean out the unenabled probes. */ provid = fp->ftp_provid; - if (dtrace_unregister(provid) != 0) { + if ((rval = dtrace_unregister(provid)) != 0) { if (fasttrap_total > fasttrap_max / 2) (void) dtrace_condense(provid); + + if (rval == EAGAIN) + fp->ftp_marked = 1; + later += fp->ftp_marked; fpp = &fp->ftp_next; } else { @@ -363,12 +370,16 @@ fasttrap_pid_cleanup_cb(void *data) * get a chance to do that work if and when the timeout is reenabled * (if detach fails). */ - if (later > 0 && fasttrap_timeout != (timeout_id_t)1) - fasttrap_timeout = timeout(&fasttrap_pid_cleanup_cb, NULL, hz); - else if (later > 0) + if (later > 0) { + if (fasttrap_timeout != (timeout_id_t)1) { + fasttrap_timeout = + timeout(&fasttrap_pid_cleanup_cb, NULL, hz); + } + fasttrap_cleanup_work = 1; - else + } else { fasttrap_timeout = 0; + } mutex_exit(&fasttrap_cleanup_mtx); in = 0; diff --git a/usr/src/uts/common/sys/dtrace_impl.h b/usr/src/uts/common/sys/dtrace_impl.h index fed537e18b..a68b1a8a89 100644 --- a/usr/src/uts/common/sys/dtrace_impl.h +++ b/usr/src/uts/common/sys/dtrace_impl.h @@ -24,11 +24,13 @@ * Use is subject to license terms. */ +/* + * Copyright (c) 2011, Joyent, Inc. All rights reserved. + */ + #ifndef _SYS_DTRACE_IMPL_H #define _SYS_DTRACE_IMPL_H -#pragma ident "%Z%%M% %I% %E% SMI" - #ifdef __cplusplus extern "C" { #endif @@ -419,8 +421,11 @@ typedef struct dtrace_buffer { uint32_t dtb_errors; /* number of errors */ uint32_t dtb_xamot_errors; /* errors in inactive buffer */ #ifndef _LP64 - uint64_t dtb_pad1; + uint64_t dtb_pad1; /* pad out to 64 bytes */ #endif + uint64_t dtb_switched; /* time of last switch */ + uint64_t dtb_interval; /* observed switch interval */ + uint64_t dtb_pad2[6]; /* pad to avoid false sharing */ } dtrace_buffer_t; /* @@ -1139,7 +1144,7 @@ struct dtrace_provider { dtrace_pops_t dtpv_pops; /* provider operations */ char *dtpv_name; /* provider name */ void *dtpv_arg; /* provider argument */ - uint_t dtpv_defunct; /* boolean: defunct provider */ + hrtime_t dtpv_defunct; /* when made defunct */ struct dtrace_provider *dtpv_next; /* next provider */ }; -- 2.11.4.GIT