From 27dc78b087f76a451b4350a81fc0e7b01d020fb8 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Sun, 7 Aug 2022 18:45:29 +0000 Subject: [PATCH 1/2] module(9): Call callbacks in topological order on load. They are called in reverse topological order on unload. dtrace_sdt will rely on this soon to ensure provider definitions are processed before their uses. --- sys/kern/kern_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/kern/kern_module.c b/sys/kern/kern_module.c index d2457df91969..7ad2b5bffc83 100644 --- a/sys/kern/kern_module.c +++ b/sys/kern/kern_module.c @@ -1889,7 +1889,7 @@ module_register_callbacks(void (*load)(struct module *), kernconfig_lock(); TAILQ_INSERT_TAIL(&modcblist, modcb, modcb_list); - TAILQ_FOREACH(mod, &module_list, mod_chain) + TAILQ_FOREACH_REVERSE(mod, &module_list, modlist, mod_chain) load(mod); kernconfig_unlock(); From 8cecbaf821160dc6ec18ae283a464f079024ac64 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Sun, 7 Aug 2022 16:08:49 +0000 Subject: [PATCH 2/2] dtrace_sdt: Register sdt providers and probes in loaded modules too. --- external/cddl/osnet/dev/sdt/sdt.c | 186 +++++++++++++++++++++++------- sys/modules/dtrace/sdt/Makefile | 1 + 2 files changed, 148 insertions(+), 39 deletions(-) diff --git a/external/cddl/osnet/dev/sdt/sdt.c b/external/cddl/osnet/dev/sdt/sdt.c index b18ba093f3cd..a23f71f3cf1a 100644 --- a/external/cddl/osnet/dev/sdt/sdt.c +++ b/external/cddl/osnet/dev/sdt/sdt.c @@ -52,6 +52,9 @@ __KERNEL_RCSID(0, "$NetBSD: sdt.c,v 1.21 2022/03/28 12:33:20 riastradh Exp $"); #endif #include #include +#ifdef __NetBSD__ +#include +#endif #ifdef __FreeBSD__ #include #include @@ -112,6 +115,10 @@ static dtrace_pops_t sdt_pops = { }; #ifdef __NetBSD__ +struct linker_file { + struct module lf_mod; +}; + static int sdt_open(dev_t dev, int flags, int mode, struct lwp *l) { @@ -203,14 +210,19 @@ sdt_create_probe(struct sdt_probe *probe) SDT_KASSERT(prov != NULL, ("probe defined without a provider")); -#ifdef __FreeBSD__ /* If no module name was specified, use the module filename. */ if (*probe->mod == 0) { +#ifdef __NetBSD__ + const char *modname = (probe->sdtp_lf == NULL ? "netbsd" : + module_name(&probe->sdtp_lf->lf_mod)); + strlcpy(mod, modname, sizeof(mod)); +#endif +#ifdef __FreeBSD__ len = strlcpy(mod, probe->sdtp_lf->filename, sizeof(mod)); if (len > 3 && strcmp(mod + len - 3, ".ko") == 0) mod[len - 3] = '\0'; - } else #endif + } else strlcpy(mod, probe->mod, sizeof(mod)); /* @@ -257,6 +269,9 @@ sdt_enable(void *arg __unused, dtrace_id_t id, void *parg) struct sdt_probe *probe = parg; probe->id = id; +#ifdef __NetBSD__ + module_hold(&probe->sdtp_lf->lf_mod); +#endif #ifdef __FreeBSD__ probe->sdtp_lf->nenabled++; if (strcmp(probe->prov->name, "lockstat") == 0) @@ -270,6 +285,9 @@ sdt_disable(void *arg __unused, dtrace_id_t id, void *parg) { struct sdt_probe *probe = parg; +#ifdef __NetBSD__ + module_rele(&probe->sdtp_lf->lf_mod); +#endif #ifdef __FreeBSD__ SDT_KASSERT(probe->sdtp_lf->nenabled > 0, ("no probes enabled")); if (strcmp(probe->prov->name, "lockstat") == 0) @@ -410,63 +428,154 @@ __link_set_decl(sdt_providers_set, struct sdt_provider); __link_set_decl(sdt_probes_set, struct sdt_probe); __link_set_decl(sdt_argtypes_set, struct sdt_argtype); -/* - * Unfortunately we don't have linker set functions and event handlers - * to support loading and unloading probes in modules... Currently if - * modules have probes, if the modules are loaded when sdt is loaded - * they will work, but they will crash unloading. - */ +struct module_callbacks *sdt_link_set_callbacks; + static void -sdt_link_set_load(void) +sdt_link_set_load_provider(struct sdt_provider *const *provider) +{ + + sdt_create_provider(*provider); +} + +static void +sdt_link_set_unload_provider(struct sdt_provider *const *curr) +{ + struct sdt_provider *prov, *tmp; + + /* + * Go through all the providers declared in this linker file and + * unregister any that aren't declared in another loaded file. + */ + TAILQ_FOREACH_SAFE(prov, &sdt_prov_list, prov_entry, tmp) { + if (strcmp(prov->name, (*curr)->name) != 0) + continue; + + if (prov->sdt_refs == 1) { + if (dtrace_unregister(prov->id) != 0) { + return; + } + TAILQ_REMOVE(&sdt_prov_list, prov, prov_entry); + free(__UNCONST(prov->name), M_SDT); + free(prov, M_SDT); + } else + prov->sdt_refs--; + break; + } +} + +static void +sdt_link_set_load_probe(struct sdt_probe *const *probe, struct module *mod) +{ + struct linker_file *lf = mod == NULL ? NULL : + container_of(mod, struct linker_file, lf_mod); + + (*probe)->sdtp_lf = lf; + sdt_create_probe(*probe); + TAILQ_INIT(&(*probe)->argtype_list); +} + +static void +sdt_link_set_load_argtype(struct sdt_argtype *const *argtype) +{ + + (*argtype)->probe->n_args++; + TAILQ_INSERT_TAIL(&(*argtype)->probe->argtype_list, + *argtype, argtype_entry); +} + +static void +sdt_link_set_load_module(struct module *mod) { struct sdt_provider * const *provider; struct sdt_probe * const *probe; struct sdt_argtype * const *argtype; + void *p; + size_t size, n; - __link_set_foreach(provider, sdt_providers_set) { - sdt_create_provider(*provider); + /* + * Skip builtin modules -- they are handled separately with + * __link_set_foreach. + */ + if (module_source(mod) == MODULE_SOURCE_KERNEL) + return; + + if (kobj_find_section(mod->mod_kobj, "link_set_sdt_providers_set", + &p, &size) == 0) { + n = size/sizeof(*provider); + for (provider = p; n --> 0; provider++) + sdt_link_set_load_provider(provider); } - __link_set_foreach(probe, sdt_probes_set) { - (*probe)->sdtp_lf = NULL; // XXX: we don't support it - sdt_create_probe(*probe); - TAILQ_INIT(&(*probe)->argtype_list); + if (kobj_find_section(mod->mod_kobj, "link_set_sdt_probes_set", + &p, &size) == 0) { + n = size/sizeof(*probe); + for (probe = p; n --> 0; probe++) + sdt_link_set_load_probe(probe, mod); } - __link_set_foreach(argtype, sdt_argtypes_set) { - (*argtype)->probe->n_args++; - TAILQ_INSERT_TAIL(&(*argtype)->probe->argtype_list, - *argtype, argtype_entry); + if (kobj_find_section(mod->mod_kobj, "link_set_sdt_argtypes_set", + &p, &size) == 0) { + n = size/sizeof(*argtype); + for (argtype = p; n --> 0; argtype++) + sdt_link_set_load_argtype(argtype); } } static void -sdt_link_set_unload(void) +sdt_link_set_unload_module(struct module *mod) { - struct sdt_provider * const *curr, *prov, *tmp; + struct sdt_provider *const *curr; + void *p; + size_t size, n; /* - * Go through all the providers declared in this linker file and - * unregister any that aren't declared in another loaded file. + * Skip builtin modules -- they are handled separately with + * __link_set_foreach. */ - __link_set_foreach(curr, sdt_providers_set) { - TAILQ_FOREACH_SAFE(prov, &sdt_prov_list, prov_entry, tmp) { - if (strcmp(prov->name, (*curr)->name) != 0) - continue; + if (module_source(mod) == MODULE_SOURCE_KERNEL) + return; - if (prov->sdt_refs == 1) { - if (dtrace_unregister(prov->id) != 0) { - return; - } - TAILQ_REMOVE(&sdt_prov_list, prov, prov_entry); - free(__UNCONST(prov->name), M_SDT); - free(prov, M_SDT); - } else - prov->sdt_refs--; - break; - } + if (kobj_find_section(mod->mod_kobj, "link_set_sdt_providers_set", + &p, &size) == 0) { + n = size/sizeof(*curr); + for (curr = p; n --> 0; curr++) + sdt_link_set_unload_provider(curr); } } + +static void +sdt_link_set_load(void) +{ + struct sdt_provider * const *provider; + struct sdt_probe * const *probe; + struct sdt_argtype * const *argtype; + + __link_set_foreach(provider, sdt_providers_set) { + sdt_link_set_load_provider(provider); + } + __link_set_foreach(probe, sdt_probes_set) { + sdt_link_set_load_probe(probe, NULL); + } + __link_set_foreach(argtype, sdt_argtypes_set) { + sdt_link_set_load_argtype(argtype); + } + + sdt_link_set_callbacks = module_register_callbacks( + sdt_link_set_load_module, sdt_link_set_unload_module); +} + +static void +sdt_link_set_unload(void) +{ + struct sdt_provider *const *curr; + + module_unregister_callbacks(sdt_link_set_callbacks); + sdt_link_set_callbacks = NULL; + + __link_set_foreach(curr, sdt_providers_set) { + sdt_link_set_unload_provider(curr); + } +} #endif @@ -507,7 +616,6 @@ sdt_unload(void) #ifdef __NetBSD__ sdt_exit(); - sdt_link_set_unload(); #endif TAILQ_FOREACH_SAFE(prov, &sdt_prov_list, prov_entry, tmp) { diff --git a/sys/modules/dtrace/sdt/Makefile b/sys/modules/dtrace/sdt/Makefile index 9f96f71a9064..5129401efa69 100644 --- a/sys/modules/dtrace/sdt/Makefile +++ b/sys/modules/dtrace/sdt/Makefile @@ -11,5 +11,6 @@ CPPFLAGS+= -I${NETBSDSRCDIR}/external/cddl/osnet/sys \ -I${NETBSDSRCDIR}/external/cddl/osnet/dist/uts/common CPPFLAGS+= -Wno-unknown-pragmas +CPPFLAGS+= -Wno-sign-compare .include