Mercurial > illumos > illumos-gate
changeset 935:375bf7616dfc
6332949 V240 panics with kmem_flags set, buffer corruption in streams/Dtrace while testing arbitrary patches
6340028 assertion failed: dtrace_deferred_pid != NULL, file: ../../common/dtrace/dtrace.c, line: 12485
author | ahl |
---|---|
date | Wed, 16 Nov 2005 09:13:23 -0800 |
parents | 0a0d422a7556 |
children | bb51223d93d7 |
files | usr/src/uts/common/dtrace/dtrace.c usr/src/uts/common/dtrace/fasttrap.c usr/src/uts/common/sys/dtrace_impl.h |
diffstat | 3 files changed, 64 insertions(+), 35 deletions(-) [+] |
line wrap: on
line diff
--- a/usr/src/uts/common/dtrace/dtrace.c Wed Nov 16 06:57:14 2005 -0800 +++ b/usr/src/uts/common/dtrace/dtrace.c Wed Nov 16 09:13:23 2005 -0800 @@ -6696,6 +6696,7 @@ next = help->dthps_next; help->dthps_next = NULL; help->dthps_prev = NULL; + help->dthps_deferred = 0; help = next; } @@ -12080,6 +12081,7 @@ if (help->dthps_next == NULL && help->dthps_prev == NULL && dtrace_deferred_pid != help) { + help->dthps_deferred = 1; help->dthps_pid = p->p_pid; help->dthps_next = dtrace_deferred_pid; help->dthps_prev = NULL; @@ -12397,7 +12399,7 @@ if (dhp != NULL) { uintptr_t daddr = (uintptr_t)dof; - int err = 0; + int err = 0, count = 0; /* * Look for helper probes. @@ -12413,10 +12415,13 @@ err = 1; break; } + + count++; } dhp->dofhp_dof = (uint64_t)(uintptr_t)dof; - if (err == 0 && dtrace_helper_provider_add(dhp) == 0) + if (err == 0 && count > 0 && + dtrace_helper_provider_add(dhp) == 0) destroy = 0; else dhp = NULL; @@ -12506,7 +12511,10 @@ } } else { mutex_enter(&dtrace_lock); - ASSERT(dtrace_deferred_pid != NULL); + ASSERT(help->dthps_deferred == 0 || + help->dthps_next != NULL || + help->dthps_prev != NULL || + help == dtrace_deferred_pid); /* * Remove the helper from the deferred list.
--- a/usr/src/uts/common/dtrace/fasttrap.c Wed Nov 16 06:57:14 2005 -0800 +++ b/usr/src/uts/common/dtrace/fasttrap.c Wed Nov 16 09:13:23 2005 -0800 @@ -158,8 +158,8 @@ static fasttrap_provider_t *fasttrap_provider_lookup(pid_t, const char *, const dtrace_pattr_t *); +static void fasttrap_provider_retire(pid_t, const char *, int); static void fasttrap_provider_free(fasttrap_provider_t *); -static void fasttrap_provider_retire(fasttrap_provider_t *); static fasttrap_proc_t *fasttrap_proc_lookup(pid_t); static void fasttrap_proc_release(fasttrap_proc_t *); @@ -441,8 +441,6 @@ static void fasttrap_exec_exit(proc_t *p) { - fasttrap_provider_t *provider; - ASSERT(p == curproc); ASSERT(MUTEX_HELD(&p->p_lock)); @@ -451,10 +449,11 @@ /* * We clean up the pid provider for this process here; user-land * static probes are handled by the meta-provider remove entry point. + * Note that the consumer count is not artificially elevated on the + * pid provider as it is on USDT providers so there's no need to drop + * it here. */ - if ((provider = fasttrap_provider_lookup(p->p_pid, - FASTTRAP_PID_NAME, NULL)) != NULL) - fasttrap_provider_retire(provider); + fasttrap_provider_retire(p->p_pid, FASTTRAP_PID_NAME, 0); mutex_enter(&p->p_lock); } @@ -1282,6 +1281,7 @@ uid_t uid = (uid_t)-1; ASSERT(strlen(name) < sizeof (fp->ftp_name)); + ASSERT(pattr != NULL); bucket = &fasttrap_provs.fth_table[FASTTRAP_PROVS_INDEX(pid, name)]; mutex_enter(&bucket->ftb_mtx); @@ -1305,12 +1305,6 @@ mutex_exit(&bucket->ftb_mtx); /* - * If we didn't want to create a new provider, just return failure. - */ - if (pattr == NULL) - return (NULL); - - /* * Make sure the process exists, isn't a child created as the result * of a vfork(2), and isn't a zombie (but may be in fork). Record the * process's uid to pass to dtrace_register(). @@ -1428,9 +1422,28 @@ } static void -fasttrap_provider_retire(fasttrap_provider_t *provider) +fasttrap_provider_retire(pid_t pid, const char *name, int ccount) { - dtrace_provider_id_t provid = provider->ftp_provid; + fasttrap_provider_t *fp; + fasttrap_bucket_t *bucket; + dtrace_provider_id_t provid; + + ASSERT(strlen(name) < sizeof (fp->ftp_name)); + ASSERT(ccount == 0 || ccount == 1); + + bucket = &fasttrap_provs.fth_table[FASTTRAP_PROVS_INDEX(pid, name)]; + mutex_enter(&bucket->ftb_mtx); + + for (fp = bucket->ftb_data; fp != NULL; fp = fp->ftp_next) { + if (fp->ftp_pid == pid && strcmp(fp->ftp_name, name) == 0 && + !fp->ftp_retired) + break; + } + + if (fp == NULL) { + mutex_exit(&bucket->ftb_mtx); + return; + } /* * Mark the provider to be removed in our post-processing step, @@ -1440,11 +1453,22 @@ * setting the retired flag indicates that we're done with this * provider; setting the proc to be defunct indicates that all * tracepoints associated with the traced process should be ignored. + * We also drop the consumer count here by the amount specified. + * + * We obviously need to take the bucket lock before the provider lock + * to perform the lookup, but we need to drop the provider lock + * before calling into the DTrace framework since we acquire the + * provider lock in callbacks invoked from the DTrace framework. The + * bucket lock therefore protects the integrity of the provider hash + * table. */ - provider->ftp_proc->ftpc_defunct = 1; - provider->ftp_retired = 1; - provider->ftp_marked = 1; - mutex_exit(&provider->ftp_mtx); + mutex_enter(&fp->ftp_mtx); + fp->ftp_proc->ftpc_defunct = 1; + fp->ftp_retired = 1; + fp->ftp_marked = 1; + fp->ftp_ccount -= ccount; + provid = fp->ftp_provid; + mutex_exit(&fp->ftp_mtx); /* * We don't have to worry about invalidating the same provider twice @@ -1453,6 +1477,8 @@ */ dtrace_invalidate(provid); + mutex_exit(&bucket->ftb_mtx); + fasttrap_pid_cleanup(); } @@ -1750,19 +1776,13 @@ static void fasttrap_meta_remove(void *arg, dtrace_helper_provdesc_t *dhpv, pid_t pid) { - fasttrap_provider_t *provider; - - if ((provider = fasttrap_provider_lookup(pid, - dhpv->dthpv_provname, NULL)) != NULL) { - /* - * Drop the consumer count now that we're done with this - * provider. If there are no other consumers retire it now. - */ - if (--provider->ftp_ccount == 0) - fasttrap_provider_retire(provider); - else - mutex_exit(&provider->ftp_mtx); - } + /* + * Clean up the USDT provider. There may be active consumers of the + * provider busy adding probes, no damage will actually befall the + * provider until that count has dropped to zero. This just puts + * the provider on death row. + */ + fasttrap_provider_retire(pid, dhpv->dthpv_provname, 1); } static dtrace_mops_t fasttrap_mops = { @@ -2118,7 +2138,7 @@ * waiting for any other consumer to finish with * this provider. A thread must first acquire the * bucket lock so there's no chance of another thread - * blocking on the providers lock. + * blocking on the provider's lock. */ mutex_enter(&fp->ftp_mtx); mutex_exit(&fp->ftp_mtx);
--- a/usr/src/uts/common/sys/dtrace_impl.h Wed Nov 16 06:57:14 2005 -0800 +++ b/usr/src/uts/common/sys/dtrace_impl.h Wed Nov 16 09:13:23 2005 -0800 @@ -1005,6 +1005,7 @@ uint_t dthps_nprovs; /* count of providers */ int dthps_generation; /* current generation */ pid_t dthps_pid; /* pid of associated proc */ + int dthps_deferred; /* helper in deferred list */ struct dtrace_helpers *dthps_next; /* next pointer */ struct dtrace_helpers *dthps_prev; /* prev pointer */ } dtrace_helpers_t;