Date: Sun, 29 Dec 2013 21:52:32 -0500 From: Mark Johnston <markj@freebsd.org> To: Mike Ma <mikemandarine@gmail.com> Cc: "dtrace@freebsd.org" <dtrace@freebsd.org> Subject: Re: [CFT] Use vmem in dtrace Message-ID: <20131230024945.GA23245@charmander.home> In-Reply-To: <CA%2Bcqwfe_v_XKdYGd-ocbEg4Ey5QHwRW1KoxSx5BAY0BPKMg6sw@mail.gmail.com> References: <CA%2Bcqwfe_v_XKdYGd-ocbEg4Ey5QHwRW1KoxSx5BAY0BPKMg6sw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Dec 13, 2013 at 08:17:03AM +0100, Mike Ma wrote: > Hi there, > > I'm a GSoC student this year, and Pedro guided me to dtrace recently. > > I'm sending my patch for a vmem task listed in the TODO list (12th item), > as there is vmem subsystem in FreeBSD 10/11 available now. > Basically, it is to revert changes from this patch > http://lists.freebsd.org/pipermail/p4-projects/2008-January/023466.html > And the main difference is that vmem_create and vmem_alloc take different > number of parameters on FreeBSD. > > I did run dtrace testsuite on my own machine. > And I hope someone here can try my patch. I've placed a modified form of your patch here: http://people.freebsd.org/~markj/patches/dtrace_vmem.diff I had to changes a few things for it to work for me: - vmem_alloc returns resources in its last argument, which cannot be NULL (else one invariably gets a page fault on line 1130 of subr_vmem.c). - the probe id arena should be created before the kld_* event handlers are registered, since they call dtrace_probe_create(), which uses the arena. - for some reason, the snprintf in dtrace_state_create() was placed under an #ifdef sun, which is not correct. If the modified patch looks ok to you, I will commit it. -Mark > > Any comments are appreciated. > Thanks. > > Cheers, > Mike > > > -- > Cheers, > Mike > Index: sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c > =================================================================== > --- sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c (revision 257743) > +++ sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c (working copy) > @@ -211,7 +211,7 @@ > static vmem_t *dtrace_minor; /* minor number arena */ > #else > static taskq_t *dtrace_taskq; /* task queue */ > -static struct unrhdr *dtrace_arena; /* Probe ID number. */ > +static vmem_t *dtrace_arena; /* probe ID arena */ > #endif > static dtrace_probe_t **dtrace_probes; /* array of all probes */ > static int dtrace_nprobes; /* number of probes */ > @@ -7829,11 +7829,7 @@ > kmem_free(probe->dtpr_mod, strlen(probe->dtpr_mod) + 1); > kmem_free(probe->dtpr_func, strlen(probe->dtpr_func) + 1); > kmem_free(probe->dtpr_name, strlen(probe->dtpr_name) + 1); > -#if defined(sun) > - vmem_free(dtrace_arena, (void *)(uintptr_t)(probe->dtpr_id), 1); > -#else > - free_unr(dtrace_arena, probe->dtpr_id); > -#endif > + vmem_free(dtrace_arena, (vmem_addr_t)(probe->dtpr_id), 1); > kmem_free(probe, sizeof (dtrace_probe_t)); > } > > @@ -7950,11 +7946,7 @@ > kmem_free(probe->dtpr_func, strlen(probe->dtpr_func) + 1); > kmem_free(probe->dtpr_name, strlen(probe->dtpr_name) + 1); > kmem_free(probe, sizeof (dtrace_probe_t)); > -#if defined(sun) > - vmem_free(dtrace_arena, (void *)((uintptr_t)i + 1), 1); > -#else > - free_unr(dtrace_arena, i + 1); > -#endif > + vmem_free(dtrace_arena, (vmem_addr_t)i + 1, 1); > } > > mutex_exit(&dtrace_lock); > @@ -7990,12 +7982,8 @@ > mutex_enter(&dtrace_lock); > } > > -#if defined(sun) > - id = (dtrace_id_t)(uintptr_t)vmem_alloc(dtrace_arena, 1, > - VM_BESTFIT | VM_SLEEP); > -#else > - id = alloc_unr(dtrace_arena); > -#endif > + id = (dtrace_id_t)vmem_alloc(dtrace_arena, 1, > + M_BESTFIT | M_WAITOK, NULL); > probe = kmem_zalloc(sizeof (dtrace_probe_t), KM_SLEEP); > > probe->dtpr_id = id; > @@ -10184,7 +10172,8 @@ > aggid = (dtrace_aggid_t)(uintptr_t)vmem_alloc(state->dts_aggid_arena, 1, > VM_BESTFIT | VM_SLEEP); > #else > - aggid = alloc_unr(state->dts_aggid_arena); > + aggid = (dtrace_aggid_t)(uintptr_t)vmem_alloc(state->dts_aggid_arena, 1, > + M_BESTFIT | M_WAITOK, NULL); > #endif > > if (aggid - 1 >= state->dts_naggregations) { > @@ -10237,7 +10226,7 @@ > #if defined(sun) > vmem_free(state->dts_aggid_arena, (void *)(uintptr_t)aggid, 1); > #else > - free_unr(state->dts_aggid_arena, aggid); > + vmem_free(state->dts_aggid_arena, (vmem_addr_t)aggid, 1); > #endif > > ASSERT(state->dts_aggregations[aggid - 1] == agg); > @@ -13211,10 +13200,10 @@ > state = kmem_zalloc(sizeof(dtrace_state_t), KM_SLEEP); > #endif > > +#if defined(sun) > state->dts_epid = DTRACE_EPIDNONE + 1; > > (void) snprintf(c, sizeof (c), "dtrace_aggid_%d", m); > -#if defined(sun) > state->dts_aggid_arena = vmem_create(c, (void *)1, UINT32_MAX, 1, > NULL, NULL, NULL, 0, VM_SLEEP | VMC_IDENTIFIER); > > @@ -13229,9 +13218,10 @@ > if (devp != NULL) > *devp = state->dts_dev; > #else > - state->dts_aggid_arena = new_unrhdr(1, INT_MAX, &dtrace_unr_mtx); > + state->dts_aggid_arena = vmem_create(c, (vmem_addr_t)1, UINT32_MAX, 1, > + 0, M_WAITOK); > state->dts_dev = dev; > -#endif > +#endif > > /* > * We allocate NCPU buffers. On the one hand, this can be quite > @@ -14036,11 +14026,7 @@ > dtrace_format_destroy(state); > > if (state->dts_aggid_arena != NULL) { > -#if defined(sun) > vmem_destroy(state->dts_aggid_arena); > -#else > - delete_unrhdr(state->dts_aggid_arena); > -#endif > state->dts_aggid_arena = NULL; > } > #if defined(sun) > @@ -15375,7 +15361,7 @@ > #if defined(sun) > vmem_free(dtrace_arena, (void *)(uintptr_t)probe->dtpr_id, 1); > #else > - free_unr(dtrace_arena, probe->dtpr_id); > + vmem_free(dtrace_arena, (vmem_addr_t)probe->dtpr_id, 1); > #endif > kmem_free(probe, sizeof (dtrace_probe_t)); > } > Index: sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h > =================================================================== > --- sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h (revision 257743) > +++ sys/cddl/contrib/opensolaris/uts/common/sys/dtrace_impl.h (working copy) > @@ -1139,11 +1139,7 @@ > int dts_nspeculations; /* number of speculations */ > int dts_naggregations; /* number of aggregations */ > dtrace_aggregation_t **dts_aggregations; /* aggregation array */ > -#if defined(sun) > vmem_t *dts_aggid_arena; /* arena for aggregation IDs */ > -#else > - struct unrhdr *dts_aggid_arena; /* arena for aggregation IDs */ > -#endif > uint64_t dts_errors; /* total number of errors */ > uint32_t dts_speculations_busy; /* number of spec. busy */ > uint32_t dts_speculations_unavail; /* number of spec unavail */ > Index: sys/cddl/dev/dtrace/dtrace_load.c > =================================================================== > --- sys/cddl/dev/dtrace/dtrace_load.c (revision 257743) > +++ sys/cddl/dev/dtrace/dtrace_load.c (working copy) > @@ -84,9 +84,9 @@ > mutex_enter(&cpu_lock); > > ASSERT(MUTEX_HELD(&cpu_lock)); > + dtrace_arena = vmem_create("dtrace", (vmem_addr_t)1, > + UINT32_MAX, 1, 0, M_WAITOK | M_BESTFIT); > > - dtrace_arena = new_unrhdr(1, INT_MAX, &dtrace_unr_mtx); > - > dtrace_state_cache = kmem_cache_create("dtrace_state_cache", > sizeof (dtrace_dstate_percpu_t) * NCPU, DTRACE_STATE_ALIGN, > NULL, NULL, NULL, NULL, NULL, 0); > Index: sys/cddl/dev/dtrace/dtrace_unload.c > =================================================================== > --- sys/cddl/dev/dtrace/dtrace_unload.c (revision 257743) > +++ sys/cddl/dev/dtrace/dtrace_unload.c (working copy) > @@ -104,7 +104,7 @@ > > kmem_cache_destroy(dtrace_state_cache); > > - delete_unrhdr(dtrace_arena); > + vmem_destroy(dtrace_arena); > > if (dtrace_toxrange != NULL) { > kmem_free(dtrace_toxrange, 0); > _______________________________________________ > freebsd-dtrace@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-dtrace > To unsubscribe, send any mail to "freebsd-dtrace-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131230024945.GA23245>