Date: Sat, 26 Oct 2013 06:23:52 +0000 (UTC) From: Mark Johnston <markj@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r257152 - in head/sys: cddl/dev/sdt sys Message-ID: <201310260623.r9Q6NqdA034971@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: markj Date: Sat Oct 26 06:23:51 2013 New Revision: 257152 URL: http://svnweb.freebsd.org/changeset/base/257152 Log: Do some cleanup of the SDT code. In particular, * Remove the unused sdt cdev. * Don't bother keeping a list of probes in struct sdt_prov; it's not needed. * Invoke sdt_load and sdt_unload from the module handler instead of registering separate SYSINITs. * Keep to within 80 columns. * Check for errors from dtrace_unregister(). Modified: head/sys/cddl/dev/sdt/sdt.c head/sys/sys/sdt.h Modified: head/sys/cddl/dev/sdt/sdt.c ============================================================================== --- head/sys/cddl/dev/sdt/sdt.c Sat Oct 26 03:55:29 2013 (r257151) +++ head/sys/cddl/dev/sdt/sdt.c Sat Oct 26 06:23:51 2013 (r257152) @@ -24,6 +24,21 @@ * */ +/* + * This file contains a reimplementation of the statically-defined tracing (SDT) + * framework for DTrace. Probes and SDT providers are defined using the macros + * in sys/sdt.h, which append all the needed structures to linker sets. When + * this module is loaded, it iterates over all of the loaded modules and + * registers probes and providers with the DTrace framework based on the + * contents of these linker sets. + * + * A list of SDT providers is maintained here since a provider may span multiple + * modules. When a kernel module is unloaded, a provider defined in that module + * is unregistered only if no other modules refer to it. The DTrace framework is + * responsible for destroying individual probes when a kernel module is + * unloaded; in particular, probes may not span multiple kernel modules. + */ + #include "opt_kdtrace.h" #include <sys/cdefs.h> @@ -53,9 +68,8 @@ static void sdt_destroy(void *, dtrace_i static void sdt_enable(void *, dtrace_id_t, void *); static void sdt_disable(void *, dtrace_id_t, void *); -static d_open_t sdt_open; -static void sdt_load(void *); -static int sdt_unload(void *); +static void sdt_load(void); +static int sdt_unload(void); static void sdt_create_provider(struct sdt_provider *); static void sdt_create_probe(struct sdt_probe *); static void sdt_kld_load(void *, struct linker_file *); @@ -63,12 +77,6 @@ static void sdt_kld_unload_try(void *, s static MALLOC_DEFINE(M_SDT, "SDT", "DTrace SDT providers"); -static struct cdevsw sdt_cdevsw = { - .d_version = D_VERSION, - .d_open = sdt_open, - .d_name = "sdt", -}; - static dtrace_pattr_t sdt_attr = { { DTRACE_STABILITY_EVOLVING, DTRACE_STABILITY_EVOLVING, DTRACE_CLASS_COMMON }, { DTRACE_STABILITY_PRIVATE, DTRACE_STABILITY_PRIVATE, DTRACE_CLASS_UNKNOWN }, @@ -90,8 +98,6 @@ static dtrace_pops_t sdt_pops = { sdt_destroy, }; -static struct cdev *sdt_cdev; - static TAILQ_HEAD(, sdt_provider) sdt_prov_list; eventhandler_tag sdt_kld_load_tag; @@ -117,7 +123,6 @@ sdt_create_provider(struct sdt_provider newprov = malloc(sizeof(*newprov), M_SDT, M_WAITOK | M_ZERO); newprov->name = strdup(prov->name, M_SDT); prov->sdt_refs = newprov->sdt_refs = 1; - TAILQ_INIT(&newprov->probe_list); TAILQ_INSERT_TAIL(&sdt_prov_list, newprov, prov_entry); @@ -161,12 +166,14 @@ sdt_create_probe(struct sdt_probe *probe if (dtrace_probe_lookup(prov->id, mod, func, name) != DTRACE_IDNONE) return; - TAILQ_INSERT_TAIL(&prov->probe_list, probe, probe_entry); - (void)dtrace_probe_create(prov->id, mod, func, name, 1, probe); } -/* Probes are created through the SDT module load/unload hook. */ +/* + * Probes are created through the SDT module load/unload hook, so this function + * has nothing to do. It only exists because the DTrace provider framework + * requires one of provide_probes and provide_module to be defined. + */ static void sdt_provide_probes(void *arg, dtrace_probedesc_t *desc) { @@ -198,39 +205,39 @@ sdt_getargdesc(void *arg, dtrace_id_t id struct sdt_argtype *argtype; struct sdt_probe *probe = parg; - if (desc->dtargd_ndx < probe->n_args) { - TAILQ_FOREACH(argtype, &probe->argtype_list, argtype_entry) { - if (desc->dtargd_ndx == argtype->ndx) { - desc->dtargd_mapping = desc->dtargd_ndx; - strlcpy(desc->dtargd_native, argtype->type, - sizeof(desc->dtargd_native)); - if (argtype->xtype != NULL) - strlcpy(desc->dtargd_xlate, - argtype->xtype, - sizeof(desc->dtargd_xlate)); - else - desc->dtargd_xlate[0] = '\0'; + if (desc->dtargd_ndx >= probe->n_args) { + desc->dtargd_ndx = DTRACE_ARGNONE; + return; + } + + TAILQ_FOREACH(argtype, &probe->argtype_list, argtype_entry) { + if (desc->dtargd_ndx == argtype->ndx) { + desc->dtargd_mapping = desc->dtargd_ndx; + if (argtype->type == NULL) { + desc->dtargd_native[0] = '\0'; + desc->dtargd_xlate[0] = '\0'; + continue; } + strlcpy(desc->dtargd_native, argtype->type, + sizeof(desc->dtargd_native)); + if (argtype->xtype != NULL) + strlcpy(desc->dtargd_xlate, argtype->xtype, + sizeof(desc->dtargd_xlate)); } - } else - desc->dtargd_ndx = DTRACE_ARGNONE; + } } static void sdt_destroy(void *arg, dtrace_id_t id, void *parg) { - struct sdt_probe *probe; - - probe = parg; - TAILQ_REMOVE(&probe->prov->probe_list, probe, probe_entry); } /* * Called from the kernel linker when a module is loaded, before * dtrace_module_loaded() is called. This is done so that it's possible to - * register new providers when modules are loaded. We cannot do this in the - * provide_module method since it's called with the provider lock held - * and dtrace_register() will try to acquire it again. + * register new providers when modules are loaded. The DTrace framework + * explicitly disallows calling into the framework from the provide_module + * provider method, so we cannot do this there. */ static void sdt_kld_load(void *arg __unused, struct linker_file *lf) @@ -264,14 +271,15 @@ sdt_kld_load(void *arg __unused, struct } static void -sdt_kld_unload_try(void *arg __unused, struct linker_file *lf, int *error __unused) +sdt_kld_unload_try(void *arg __unused, struct linker_file *lf, int *error) { struct sdt_provider *prov, **curr, **begin, **end, *tmp; if (*error != 0) /* We already have an error, so don't do anything. */ return; - else if (linker_file_lookup_set(lf, "sdt_providers_set", &begin, &end, NULL)) + else if (linker_file_lookup_set(lf, "sdt_providers_set", &begin, &end, + NULL)) /* No DTrace providers are declared in this file. */ return; @@ -281,17 +289,20 @@ sdt_kld_unload_try(void *arg __unused, s */ for (curr = begin; curr < end; curr++) { TAILQ_FOREACH_SAFE(prov, &sdt_prov_list, prov_entry, tmp) { - if (strcmp(prov->name, (*curr)->name) == 0) { - if (prov->sdt_refs == 1) { - TAILQ_REMOVE(&sdt_prov_list, prov, - prov_entry); - dtrace_unregister(prov->id); - free(prov->name, M_SDT); - free(prov, M_SDT); - } else - prov->sdt_refs--; - break; - } + if (strcmp(prov->name, (*curr)->name) != 0) + continue; + + if (prov->sdt_refs == 1) { + if (dtrace_unregister(prov->id) != 0) { + *error = 1; + return; + } + TAILQ_REMOVE(&sdt_prov_list, prov, prov_entry); + free(prov->name, M_SDT); + free(prov, M_SDT); + } else + prov->sdt_refs--; + break; } } } @@ -306,15 +317,11 @@ sdt_linker_file_cb(linker_file_t lf, voi } static void -sdt_load(void *arg __unused) +sdt_load() { TAILQ_INIT(&sdt_prov_list); - /* Create the /dev/dtrace/sdt entry. */ - sdt_cdev = make_dev(&sdt_cdevsw, 0, UID_ROOT, GID_WHEEL, 0600, - "dtrace/sdt"); - sdt_probe_func = dtrace_probe; sdt_kld_load_tag = EVENTHANDLER_REGISTER(kld_load, sdt_kld_load, NULL, @@ -327,9 +334,10 @@ sdt_load(void *arg __unused) } static int -sdt_unload(void *arg __unused) +sdt_unload() { struct sdt_provider *prov, *tmp; + int ret; EVENTHANDLER_DEREGISTER(kld_load, sdt_kld_load_tag); EVENTHANDLER_DEREGISTER(kld_unload_try, sdt_kld_unload_try_tag); @@ -337,18 +345,17 @@ sdt_unload(void *arg __unused) sdt_probe_func = sdt_probe_stub; TAILQ_FOREACH_SAFE(prov, &sdt_prov_list, prov_entry, tmp) { + ret = dtrace_unregister(prov->id); + if (ret != 0) + return (ret); TAILQ_REMOVE(&sdt_prov_list, prov, prov_entry); - dtrace_unregister(prov->id); free(prov->name, M_SDT); free(prov, M_SDT); } - destroy_dev(sdt_cdev); - return (0); } -/* ARGSUSED */ static int sdt_modevent(module_t mod __unused, int type, void *data __unused) { @@ -356,9 +363,11 @@ sdt_modevent(module_t mod __unused, int switch (type) { case MOD_LOAD: + sdt_load(); break; case MOD_UNLOAD: + error = sdt_unload(); break; case MOD_SHUTDOWN: @@ -372,18 +381,6 @@ sdt_modevent(module_t mod __unused, int return (error); } -/* ARGSUSED */ -static int -sdt_open(struct cdev *dev __unused, int oflags __unused, int devtype __unused, - struct thread *td __unused) -{ - - return (0); -} - -SYSINIT(sdt_load, SI_SUB_DTRACE_PROVIDER, SI_ORDER_ANY, sdt_load, NULL); -SYSUNINIT(sdt_unload, SI_SUB_DTRACE_PROVIDER, SI_ORDER_ANY, sdt_unload, NULL); - DEV_MODULE(sdt, sdt_modevent, NULL); MODULE_VERSION(sdt, 1); MODULE_DEPEND(sdt, dtrace, 1, 1, 1); Modified: head/sys/sys/sdt.h ============================================================================== --- head/sys/sys/sdt.h Sat Oct 26 03:55:29 2013 (r257151) +++ head/sys/sys/sdt.h Sat Oct 26 06:23:51 2013 (r257152) @@ -134,7 +134,7 @@ SET_DECLARE(sdt_argtypes_set, struct sdt #define SDT_PROVIDER_DEFINE(prov) \ struct sdt_provider sdt_provider_##prov[1] = { \ - { #prov, { NULL, NULL }, { NULL, NULL }, 0, 0 } \ + { #prov, { NULL, NULL }, 0, 0 } \ }; \ DATA_SET(sdt_providers_set, sdt_provider_##prov); @@ -358,7 +358,6 @@ struct sdt_provider { char *name; /* Provider name. */ TAILQ_ENTRY(sdt_provider) prov_entry; /* SDT provider list entry. */ - TAILQ_HEAD(probe_list_head, sdt_probe) probe_list; uintptr_t id; /* DTrace provider ID. */ int sdt_refs; /* Number of module references. */ };
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201310260623.r9Q6NqdA034971>