From c085a9e47a68d480da2ae31a5d130443a3a9ce0f Mon Sep 17 00:00:00 2001 From: pfg Date: Fri, 10 May 2013 21:12:55 +0000 Subject: [PATCH] MFC r248983: Dtrace: enablings on defunct providers prevent providers from unregistering Merge change from illumos: 1368 enablings on defunct providers prevent providers from unregistering Illumos Revision: 13430:8e6add739e38 Reference: https://www.illumos.org/issues/1368 Reviewed by: gnn Tested by: Fabian Keil Obtained from: Illumos git-svn-id: svn://svn.freebsd.org/base/stable/9@250484 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- .../cmd/dtrace/test/cmd/jdtrace/exception.lst | 6 +- .../test/tst/common/usdt/tst.noreap.ksh | 128 +++++++++++++++ .../test/tst/common/usdt/tst.noreapring.ksh | 124 ++++++++++++++ .../dtrace/test/tst/common/usdt/tst.reap.ksh | 115 +++++++++++++ .../opensolaris/uts/common/dtrace/dtrace.c | 151 +++++++++++++++++- .../opensolaris/uts/common/dtrace/fasttrap.c | 22 ++- .../opensolaris/uts/common/sys/dtrace_impl.h | 13 +- sys/modules/dtrace/dtrace/Makefile | 4 + 8 files changed, 546 insertions(+), 17 deletions(-) create mode 100644 cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/usdt/tst.noreap.ksh create mode 100644 cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/usdt/tst.noreapring.ksh create mode 100644 cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/usdt/tst.reap.ksh diff --git a/cddl/contrib/opensolaris/cmd/dtrace/test/cmd/jdtrace/exception.lst b/cddl/contrib/opensolaris/cmd/dtrace/test/cmd/jdtrace/exception.lst index 261f8707c..19fc3aca5 100644 --- a/cddl/contrib/opensolaris/cmd/dtrace/test/cmd/jdtrace/exception.lst +++ b/cddl/contrib/opensolaris/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/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/usdt/tst.noreap.ksh b/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/usdt/tst.noreap.ksh new file mode 100644 index 000000000..338dcdf03 --- /dev/null +++ b/cddl/contrib/opensolaris/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/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/usdt/tst.noreapring.ksh b/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/usdt/tst.noreapring.ksh new file mode 100644 index 000000000..a2e5edee3 --- /dev/null +++ b/cddl/contrib/opensolaris/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/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/usdt/tst.reap.ksh b/cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/usdt/tst.reap.ksh new file mode 100644 index 000000000..f18c585ef --- /dev/null +++ b/cddl/contrib/opensolaris/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/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c b/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c index 892c83408..3ceebb522 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c +++ b/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c @@ -182,6 +182,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 @@ -203,8 +204,8 @@ static dev_info_t *dtrace_devi; /* device info */ #if defined(sun) static vmem_t *dtrace_arena; /* probe ID arena */ static vmem_t *dtrace_minor; /* minor number arena */ -static taskq_t *dtrace_taskq; /* task queue */ #else +static taskq_t *dtrace_taskq; /* task queue */ static struct unrhdr *dtrace_arena; /* Probe ID number. */ #endif static dtrace_probe_t **dtrace_probes; /* array of all probes */ @@ -550,11 +551,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, @@ -7593,7 +7596,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 == @@ -7652,14 +7655,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); } /* @@ -7756,7 +7776,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); @@ -10719,6 +10739,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)); @@ -10734,6 +10755,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); } @@ -11222,6 +11245,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) { @@ -11692,6 +11745,85 @@ dtrace_enabling_provide(dtrace_provider_t *prv) mutex_enter(&dtrace_lock); } +/* + * 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 */ @@ -15532,6 +15664,10 @@ dtrace_open(struct cdev *dev, int oflags, int devtype, struct thread *td) #else devfs_set_cdevpriv(state, dtrace_dtr); #endif + /* This code actually belongs in dtrace_attach() */ + if (dtrace_opens == 1) + dtrace_taskq = taskq_create("dtrace_taskq", 1, maxclsyspri, + 1, INT_MAX, 0); #endif mutex_exit(&cpu_lock); @@ -15616,6 +15752,11 @@ dtrace_close(struct cdev *dev, int flags, int fmt __unused, struct thread *td) (void) kdi_dtrace_set(KDI_DTSET_DTRACE_DEACTIVATE); #else --dtrace_opens; + /* This code actually belongs in dtrace_detach() */ + if ((dtrace_opens == 0) && (dtrace_taskq != NULL)) { + taskq_destroy(dtrace_taskq); + dtrace_taskq = NULL; + } #endif mutex_exit(&dtrace_lock); diff --git a/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c b/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c index ad3b2d5c1..691b6c9cc 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c +++ b/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c @@ -175,6 +175,9 @@ static volatile uint64_t fasttrap_mod_gen; static uint32_t fasttrap_max; static uint32_t fasttrap_total; +/* + * Copyright (c) 2011, Joyent, Inc. All rights reserved. + */ #define FASTTRAP_TPOINTS_DEFAULT_SIZE 0x4000 #define FASTTRAP_PROVIDERS_DEFAULT_SIZE 0x100 @@ -317,7 +320,7 @@ fasttrap_pid_cleanup_cb(void *data) fasttrap_provider_t **fpp, *fp; fasttrap_bucket_t *bucket; dtrace_provider_id_t provid; - int i, later = 0; + int i, later = 0, rval; static volatile int in = 0; ASSERT(in == 0); @@ -378,9 +381,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 { @@ -408,12 +415,15 @@ 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 && callout_active(&fasttrap_timeout)) - callout_reset(&fasttrap_timeout, hz, &fasttrap_pid_cleanup_cb, - NULL); + 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 { + } else { #if !defined(sun) /* Nothing to be done for FreeBSD */ #endif diff --git a/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h b/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h index 6870a0b2b..4d9a4f8c2 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h +++ b/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h @@ -26,11 +26,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 @@ -429,8 +431,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; /* @@ -1162,7 +1167,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 */ }; diff --git a/sys/modules/dtrace/dtrace/Makefile b/sys/modules/dtrace/dtrace/Makefile index 8312d0c8e..4f040624b 100644 --- a/sys/modules/dtrace/dtrace/Makefile +++ b/sys/modules/dtrace/dtrace/Makefile @@ -3,6 +3,7 @@ ARCHDIR= ${MACHINE_CPUARCH} .PATH: ${.CURDIR}/../../../cddl/contrib/opensolaris/uts/common/dtrace +.PATH: ${.CURDIR}/../../../cddl/compat/opensolaris/kern .PATH: ${.CURDIR}/../../../cddl/kern .PATH: ${.CURDIR}/../../../cddl/dev/dtrace .PATH: ${.CURDIR}/../../../cddl/dev/dtrace/${ARCHDIR} @@ -26,6 +27,9 @@ SRCS+= assym.s # These are needed for assym.s SRCS+= opt_compat.h opt_kstack_pages.h opt_nfs.h opt_hwpmc_hooks.h +#This is needed for dtrace.c +SRCS += opensolaris_taskq.c + .if ${MACHINE_CPUARCH} == "i386" SRCS+= opt_apic.h .endif -- 2.45.0