Date: Thu, 20 Apr 2006 21:54:00 GMT From: John Birrell <jb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 95729 for review Message-ID: <200604202154.k3KLs0BB022204@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=95729 Change 95729 by jb@jb_freebsd2 on 2006/04/20 21:53:46 Use macros to translate Solaris kmem_zalloc(), kmem_alloc() and kmem_free() calls into their FreeBSD equivalents. This turns out to be a neat way to port their code. Most memory allocations in this driver occur with one or more mutex/es locked. Since dtrace is part of a released product, I'm porting this on the assumption that the locking strategy works. With witness enabled, I can't have it downgrade a M_WAITOK call to a M_NOWAIT call just because it thinks it's a not a good idea to sleep with locks held. For this code, we need to do that and we can't afford to fail because the thing that we might be trying to trace is the low memory situation. We might not be too successful, but we need to try. Anyway... that's why the mutexes are initialised with M_NOWITNESS. As a hack, I've created two mutexes (cpu_lock and mod_lock) just so that I don't have to comment them out in all the places where they are referenced. I havn't started looking for the FreeBSD equivalent of those. Use macros for the mutex_enter() and mutex_exit() calls to translate them to mtx_lock() and mtx_unlock(). Again, this avoids having to change the code in lots of places. Unfortunately dtrace_load() doesn't exist as a function in Sun's dtrace.c. They have this code in dtrace_attach(). I don't want to use that name in FreeBSD beacuse it implies that the device is attaching to a bus. I would prefer it if Sun would abstract their Solaris-specific device model from the meaty bits of code. They could do this by making their dtrace_attach() call this function. Affected files ... .. //depot/projects/dtrace/src/sys/cddl/dev/dtrace/dtrace_load.c#3 edit Differences ... ==== //depot/projects/dtrace/src/sys/cddl/dev/dtrace/dtrace_load.c#3 (text+ko) ==== @@ -37,19 +37,32 @@ #endif int error = 0; - mtx_init(&dtrace_lock,"dtrace probe state",NULL,MTX_RECURSE); - mtx_init(&dtrace_provider_lock,"dtrace provider state",NULL,MTX_RECURSE); - mtx_init(&dtrace_meta_lock,"dtrace meta-provider state",NULL,MTX_RECURSE); + /* + * XXX This is a short term hack to avoid having to comment + * out lots and lots of lock/unlock calls. + */ + mtx_init(&cpu_lock,"XXX hack",NULL,MTX_RECURSE | MTX_NOWITNESS); + mtx_init(&mod_lock,"XXX hack",NULL,MTX_RECURSE | MTX_NOWITNESS); + + /* + * Initialise the mutexes without 'witness' because the dtrace + * code is mostly written to wait for memory. To have the + * witness code change a malloc() from M_WAITOK to M_NOWAIT + * because a lock is held would surely create a panic in a + * low memory situation. And that low memory situation might be + * the very problem we are trying to trace. + */ + mtx_init(&dtrace_lock,"dtrace probe state",NULL,MTX_RECURSE | MTX_NOWITNESS); + mtx_init(&dtrace_provider_lock,"dtrace provider state",NULL,MTX_RECURSE | MTX_NOWITNESS); + mtx_init(&dtrace_meta_lock,"dtrace meta-provider state",NULL,MTX_RECURSE | MTX_NOWITNESS); /* Create the /dev/dtrace entry. */ dtrace_dev = make_dev(&dtrace_cdevsw, DTRACE_MINOR, UID_ROOT, GID_WHEEL, 0660, "dtrace"); -#ifdef DOODAD - mtx_lock(&cpu_lock); -#endif - mtx_lock(&dtrace_provider_lock); - mtx_lock(&dtrace_lock); + mutex_enter(&cpu_lock); + mutex_enter(&dtrace_provider_lock); + mutex_enter(&dtrace_lock); #ifdef DOODAD if (ddi_soft_state_init(&dtrace_softstate, @@ -104,6 +117,7 @@ dtrace_state_cache = kmem_cache_create("dtrace_state_cache", sizeof (dtrace_dstate_percpu_t) * NCPU, DTRACE_STATE_ALIGN, NULL, NULL, NULL, NULL, NULL, 0); +#endif ASSERT(MUTEX_HELD(&cpu_lock)); dtrace_bymod = dtrace_hash_create(offsetof(dtrace_probe_t, dtpr_mod), @@ -124,6 +138,7 @@ dtrace_retain_max = 1; } +#ifdef DOODAD /* * Now discover our toxic ranges. */ @@ -140,7 +155,6 @@ (void) dtrace_register("dtrace", &dtrace_provider_attr, DTRACE_PRIV_NONE, 0, &dtrace_provider_ops, NULL, &id); -#ifdef DOODAD ASSERT(dtrace_provider != NULL); ASSERT((dtrace_provider_id_t)dtrace_provider == id); @@ -161,10 +175,11 @@ if (dtrace_helptrace_enabled) { ASSERT(dtrace_helptrace_buffer == NULL); dtrace_helptrace_buffer = - kmem_zalloc(dtrace_helptrace_bufsize, KM_SLEEP); + malloc(dtrace_helptrace_bufsize, M_DTRACE, M_NOWAIT | M_ZERO); dtrace_helptrace_next = 0; } +#ifdef DOODAD /* * If there are already providers, we must ask them to provide their * probes, and then match any anonymous enabling against them. Note @@ -199,8 +214,8 @@ } #endif - mtx_unlock(&dtrace_lock); - mtx_unlock(&dtrace_provider_lock); + mutex_exit(&dtrace_lock); + mutex_exit(&dtrace_provider_lock); #ifdef DOODAD if (state != NULL) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200604202154.k3KLs0BB022204>