Date: Mon, 27 Mar 2006 13:34:48 GMT From: John Baldwin <jhb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 94108 for review Message-ID: <200603271334.k2RDYmx6016563@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=94108 Change 94108 by jhb@jhb_tibook on 2006/03/27 13:34:15 Rework linking for the kernel linker. I started by expanding the existing kld_mtx to cover the members of the linker_file objects as well as the list and ended up making it an sx lock that single threads the entire linker. Given that the linker is not used in any critical paths and not used all that often, this is probably sufficent and it's probably not worth it to persue more finely grained locking for the linker. Affected files ... .. //depot/projects/smpng/sys/kern/kern_linker.c#43 edit .. //depot/projects/smpng/sys/kern/link_elf.c#32 edit .. //depot/projects/smpng/sys/kern/link_elf_obj.c#8 edit .. //depot/projects/smpng/sys/notes#68 edit .. //depot/projects/smpng/sys/sys/linker.h#16 edit Differences ... ==== //depot/projects/smpng/sys/kern/kern_linker.c#43 (text+ko) ==== @@ -55,6 +55,10 @@ int kld_debug = 0; #endif +#define KLD_LOCK() do { sx_xlock(&kld_sx); mtx_lock(&Giant); } while (0) +#define KLD_UNLOCK() do { mtx_unlock(&Giant); sx_xunlock(&kld_sx); } while (0) +#define KLD_LOCK_ASSERT() sx_assert(&kld_sx, SX_XLOCKED) + /* * static char *linker_search_path(const char *name, struct mod_depend * *verinfo); @@ -68,7 +72,7 @@ linker_file_t linker_kernel_file; -static struct mtx kld_mtx; /* kernel linker mutex */ +static struct sx kld_sx; /* kernel linker lock */ static linker_class_list_t classes; static linker_file_list_t linker_files; @@ -78,17 +82,15 @@ #define LINKER_GET_NEXT_FILE_ID(a) do { \ linker_file_t lftmp; \ \ + KLD_LOCK_ASSERT(); \ retry: \ - mtx_lock(&kld_mtx); \ TAILQ_FOREACH(lftmp, &linker_files, link) { \ if (next_file_id == lftmp->id) { \ next_file_id++; \ - mtx_unlock(&kld_mtx); \ goto retry; \ } \ } \ (a) = next_file_id; \ - mtx_unlock(&kld_mtx); /* Hold for safe read of id variable */ \ } while(0) @@ -103,8 +105,12 @@ typedef struct modlist *modlist_t; static modlisthead_t found_modules; -static modlist_t modlist_lookup2(const char *name, - struct mod_depend *verinfo); +static int linker_file_add_dependency(linker_file_t file, + linker_file_t dep); +static int linker_load_module_internal(const char *kldname, + const char *modname, struct linker_file *parent, + struct mod_depend *verinfo, struct linker_file **lfpp); +static modlist_t modlist_lookup2(const char *name, struct mod_depend *verinfo); static char * linker_strdup(const char *str) @@ -120,7 +126,7 @@ linker_init(void *arg) { - mtx_init(&kld_mtx, "kernel linker", NULL, MTX_DEF); + sx_init(&kld_sx, "kernel linker"); TAILQ_INIT(&classes); TAILQ_INIT(&linker_files); } @@ -161,7 +167,7 @@ KLD_DPF(FILE, ("linker_file_sysinit: calling SYSINITs for %s\n", lf->filename)); - if (linker_file_lookup_set(lf, "sysinit_set", &start, &stop, NULL) != 0) + if (LINKER_LOOKUP_SET(lf, "sysinit_set", &start, &stop, NULL) != 0) return; /* * Perform a bubble sort of the system initialization objects by @@ -203,8 +209,7 @@ KLD_DPF(FILE, ("linker_file_sysuninit: calling SYSUNINITs for %s\n", lf->filename)); - if (linker_file_lookup_set(lf, "sysuninit_set", &start, &stop, - NULL) != 0) + if (LINKER_LOOKUP_SET(lf, "sysuninit_set", &start, &stop, NULL) != 0) return; /* @@ -248,7 +253,7 @@ ("linker_file_register_sysctls: registering SYSCTLs for %s\n", lf->filename)); - if (linker_file_lookup_set(lf, "sysctl_set", &start, &stop, NULL) != 0) + if (LINKER_LOOKUP_SET(lf, "sysctl_set", &start, &stop, NULL) != 0) return; for (oidp = start; oidp < stop; oidp++) @@ -263,7 +268,7 @@ KLD_DPF(FILE, ("linker_file_unregister_sysctls: registering SYSCTLs" " for %s\n", lf->filename)); - if (linker_file_lookup_set(lf, "sysctl_set", &start, &stop, NULL) != 0) + if (LINKER_LOOKUP_SET(lf, "sysctl_set", &start, &stop, NULL) != 0) return; for (oidp = start; oidp < stop; oidp++) @@ -280,8 +285,8 @@ KLD_DPF(FILE, ("linker_file_register_modules: registering modules" " in %s\n", lf->filename)); - if (linker_file_lookup_set(lf, "modmetadata_set", &start, - &stop, 0) != 0) { + if (LINKER_LOOKUP_SET(lf, "modmetadata_set", &start, &stop, + NULL) != 0) { /* * This fallback should be unnecessary, but if we get booted * from boot2 instead of loader and we are missing our @@ -325,21 +330,21 @@ { linker_class_t lc; linker_file_t lf; - int foundfile, error = 0; + int foundfile, error; /* Refuse to load modules if securelevel raised */ if (securelevel > 0) return (EPERM); + KLD_LOCK_ASSERT(); lf = linker_find_file_by_name(filename); if (lf) { KLD_DPF(FILE, ("linker_load_file: file %s is already loaded," " incrementing refs\n", filename)); *result = lf; lf->refs++; - goto out; + return (0); } - lf = NULL; foundfile = 0; /* @@ -360,15 +365,15 @@ if (lf) { error = linker_file_register_modules(lf); if (error == EEXIST) { - linker_file_unload(lf, LINKER_UNLOAD_FORCE); - goto out; + linker_file_unload_internal(lf, + LINKER_UNLOAD_FORCE); + return (error); } linker_file_register_sysctls(lf); linker_file_sysinit(lf); lf->flags |= LINKER_FILE_LINKED; *result = lf; - error = 0; - goto out; + return (0); } } /* @@ -397,51 +402,51 @@ linker_file_t *result) { modlist_t mod; + int error; + KLD_LOCK(); if ((mod = modlist_lookup2(modname, verinfo)) != NULL) { *result = mod->container; (*result)->refs++; + KLD_UNLOCK(); return (0); } - return (linker_load_module(NULL, modname, NULL, verinfo, result)); + error = linker_load_module_internal(NULL, modname, NULL, verinfo, + result); + KLD_UNLOCK(); + return (error); } linker_file_t linker_find_file_by_name(const char *filename) { - linker_file_t lf = 0; + linker_file_t lf; char *koname; koname = malloc(strlen(filename) + 4, M_LINKER, M_WAITOK); - if (koname == NULL) - goto out; sprintf(koname, "%s.ko", filename); - mtx_lock(&kld_mtx); + KLD_LOCK_ASSERT(); TAILQ_FOREACH(lf, &linker_files, link) { if (strcmp(lf->filename, koname) == 0) break; if (strcmp(lf->filename, filename) == 0) break; } - mtx_unlock(&kld_mtx); -out: - if (koname) - free(koname, M_LINKER); + free(koname, M_LINKER); return (lf); } linker_file_t linker_find_file_by_id(int fileid) { - linker_file_t lf = 0; - - mtx_lock(&kld_mtx); + linker_file_t lf; + + KLD_LOCK_ASSERT(); TAILQ_FOREACH(lf, &linker_files, link) if (lf->id == fileid) break; - mtx_unlock(&kld_mtx); return (lf); } @@ -451,39 +456,47 @@ linker_file_t lf; const char *filename; + KLD_LOCK_ASSERT(); lf = NULL; filename = linker_basename(pathname); KLD_DPF(FILE, ("linker_make_file: new file, filename=%s\n", filename)); lf = (linker_file_t)kobj_create((kobj_class_t)lc, M_LINKER, M_WAITOK); if (lf == NULL) - goto out; + return (NULL); lf->refs = 1; lf->userrefs = 0; lf->flags = 0; lf->filename = linker_strdup(filename); - LINKER_GET_NEXT_FILE_ID(lf->id); lf->ndeps = 0; lf->deps = NULL; STAILQ_INIT(&lf->common); TAILQ_INIT(&lf->modules); - mtx_lock(&kld_mtx); + LINKER_GET_NEXT_FILE_ID(lf->id); TAILQ_INSERT_TAIL(&linker_files, lf, link); - mtx_unlock(&kld_mtx); -out: return (lf); } int linker_file_unload(linker_file_t file, int flags) { + int error; + + KLD_LOCK(); + error = linker_file_unload_internal(file, flags); + KLD_UNLOCK(); + return (error); +} + +/* Not static as it is shared with the linker backends. */ +int +linker_file_unload_internal(linker_file_t file, int flags) +{ module_t mod, next; modlist_t ml, nextml; struct common_symbol *cp; int error, i; - error = 0; - /* Refuse to unload modules if securelevel raised. */ if (securelevel > 0) return (EPERM); @@ -493,6 +506,7 @@ return (error); #endif + KLD_LOCK_ASSERT(); KLD_DPF(FILE, ("linker_file_unload: lf->refs=%d\n", file->refs)); if (file->refs == 1) { KLD_DPF(FILE, ("linker_file_unload: file is unloading," @@ -512,7 +526,7 @@ if ((error = module_unload(mod, flags)) != 0) { KLD_DPF(FILE, ("linker_file_unload: module %p" " vetoes unload\n", mod)); - goto out; + return (error); } else MOD_XLOCK; module_release(mod); @@ -520,9 +534,8 @@ MOD_XUNLOCK; } file->refs--; - if (file->refs > 0) { - goto out; - } + if (file->refs > 0) + return (0); for (ml = TAILQ_FIRST(&found_modules); ml; ml = nextml) { nextml = TAILQ_NEXT(ml, link); if (ml->container == file) { @@ -539,13 +552,11 @@ linker_file_sysuninit(file); linker_file_unregister_sysctls(file); } - mtx_lock(&kld_mtx); TAILQ_REMOVE(&linker_files, file, link); - mtx_unlock(&kld_mtx); if (file->deps) { for (i = 0; i < file->ndeps; i++) - linker_file_unload(file->deps[i], flags); + linker_file_unload_internal(file->deps[i], flags); free(file->deps, M_LINKER); file->deps = NULL; } @@ -561,15 +572,15 @@ file->filename = NULL; } kobj_delete((kobj_t) file, M_LINKER); -out: - return (error); + return (0); } -int +static int linker_file_add_dependency(linker_file_t file, linker_file_t dep) { linker_file_t *newdeps; + KLD_LOCK_ASSERT(); newdeps = malloc((file->ndeps + 1) * sizeof(linker_file_t *), M_LINKER, M_WAITOK | M_ZERO); if (newdeps == NULL) @@ -594,8 +605,12 @@ linker_file_lookup_set(linker_file_t file, const char *name, void *firstp, void *lastp, int *countp) { + int error; - return (LINKER_LOOKUP_SET(file, name, firstp, lastp, countp)); + KLD_LOCK(); + error = LINKER_LOOKUP_SET(file, name, firstp, lastp, countp); + KLD_UNLOCK(); + return (error); } caddr_t @@ -607,6 +622,7 @@ size_t common_size = 0; int i; + KLD_LOCK(); KLD_DPF(SYM, ("linker_file_lookup_symbol: file=%p, name=%s, deps=%d\n", file, name, deps)); @@ -622,6 +638,7 @@ else { KLD_DPF(SYM, ("linker_file_lookup_symbol: symbol" ".value=%p\n", symval.value)); + KLD_UNLOCK(); return (symval.value); } } @@ -632,6 +649,7 @@ if (address) { KLD_DPF(SYM, ("linker_file_lookup_symbol:" " deps value=%p\n", address)); + KLD_UNLOCK(); return (address); } } @@ -648,6 +666,7 @@ if (strcmp(cp->name, name) == 0) { KLD_DPF(SYM, ("linker_file_lookup_symbol:" " old common value=%p\n", cp->address)); + KLD_UNLOCK(); return (cp->address); } } @@ -658,10 +677,6 @@ cp = malloc(sizeof(struct common_symbol) + common_size + strlen(name) + 1, M_LINKER, M_WAITOK | M_ZERO); - if (cp == NULL) { - KLD_DPF(SYM, ("linker_file_lookup_symbol: nomem\n")); - return (0); - } cp->address = (caddr_t)(cp + 1); cp->name = cp->address + common_size; strcpy(cp->name, name); @@ -670,8 +685,10 @@ KLD_DPF(SYM, ("linker_file_lookup_symbol: new common" " value=%p\n", cp->address)); + KLD_UNLOCK(); return (cp->address); } + KLD_UNLOCK(); KLD_DPF(SYM, ("linker_file_lookup_symbol: fail\n")); return (0); } @@ -683,7 +700,7 @@ * * Note that we do not obey list locking protocols here. We really don't need * DDB to hang because somebody's got the lock held. We'll take the chance - * that the files list is inconsistant instead. + * that the files list is inconsistent instead. */ int @@ -781,13 +798,13 @@ modname = pathname; } - mtx_lock(&Giant); - error = linker_load_module(kldname, modname, NULL, NULL, &lf); + KLD_LOCK(); + error = linker_load_module_internal(kldname, modname, NULL, NULL, &lf); if (error == 0) { lf->userrefs++; td->td_retval[0] = lf->id; } - mtx_unlock(&Giant); + KLD_UNLOCK(); out: free(pathname, M_TEMP); return (error); @@ -808,7 +825,7 @@ if ((error = suser(td)) != 0) return (error); - mtx_lock(&Giant); + KLD_LOCK(); lf = linker_find_file_by_id(fileid); if (lf) { KLD_DPF(FILE, ("kldunload: lf->userrefs=%d\n", lf->userrefs)); @@ -821,13 +838,13 @@ error = EBUSY; } else { lf->userrefs--; - error = linker_file_unload(lf, flags); + error = linker_file_unload_internal(lf, flags); if (error) lf->userrefs++; } } else error = ENOENT; - mtx_unlock(&Giant); + KLD_UNLOCK(); return (error); } @@ -878,13 +895,13 @@ goto out; filename = linker_basename(pathname); - mtx_lock(&Giant); + KLD_LOCK(); lf = linker_find_file_by_name(filename); - mtx_unlock(&Giant); - if (lf) + if (lf) { td->td_retval[0] = lf->id; - else + } else error = ENOENT; + KLD_UNLOCK(); out: free(pathname, M_TEMP); return (error); @@ -905,15 +922,12 @@ return (error); #endif - mtx_lock(&Giant); - + KLD_LOCK(); if (uap->fileid == 0) { - mtx_lock(&kld_mtx); if (TAILQ_FIRST(&linker_files)) td->td_retval[0] = TAILQ_FIRST(&linker_files)->id; else td->td_retval[0] = 0; - mtx_unlock(&kld_mtx); goto out; } lf = linker_find_file_by_id(uap->fileid); @@ -925,7 +939,7 @@ } else error = ENOENT; out: - mtx_unlock(&Giant); + KLD_UNLOCK(); return (error); } @@ -935,10 +949,18 @@ int kldstat(struct thread *td, struct kldstat_args *uap) { + struct kld_file_stat stat; linker_file_t lf; - int error = 0; - int namelen, version; - struct kld_file_stat *stat; + int error, namelen; + + /* + * Check the version of the user's structure. + */ + error = copyin(uap->stat, &stat, sizeof(struct kld_file_stat)); + if (error) + return (error); + if (stat.version != sizeof(struct kld_file_stat)) + return (EINVAL); #ifdef MAC error = mac_check_kld_stat(td->td_ucred); @@ -946,43 +968,26 @@ return (error); #endif - mtx_lock(&Giant); - + KLD_LOCK(); lf = linker_find_file_by_id(uap->fileid); if (lf == NULL) { - error = ENOENT; - goto out; + KLD_UNLOCK(); + return (ENOENT); } - stat = uap->stat; - /* - * Check the version of the user's structure. - */ - if ((error = copyin(&stat->version, &version, sizeof(version))) != 0) - goto out; - if (version != sizeof(struct kld_file_stat)) { - error = EINVAL; - goto out; - } namelen = strlen(lf->filename) + 1; if (namelen > MAXPATHLEN) namelen = MAXPATHLEN; - if ((error = copyout(lf->filename, &stat->name[0], namelen)) != 0) - goto out; - if ((error = copyout(&lf->refs, &stat->refs, sizeof(int))) != 0) - goto out; - if ((error = copyout(&lf->id, &stat->id, sizeof(int))) != 0) - goto out; - if ((error = copyout(&lf->address, &stat->address, - sizeof(caddr_t))) != 0) - goto out; - if ((error = copyout(&lf->size, &stat->size, sizeof(size_t))) != 0) - goto out; + bcopy(lf->filename, &stat.name[0], namelen); + stat.refs = lf.refs; + stat.id = lf.id; + stat.address = lf.address; + stat.size = lf.size; + KLD_UNLOCK(); td->td_retval[0] = 0; -out: - mtx_unlock(&Giant); - return (error); + + return (copyout(&stat, uap->stat, sizeof(struct kld_file_stat))); } /* @@ -1001,7 +1006,7 @@ return (error); #endif - mtx_lock(&Giant); + KLD_LOCK(); lf = linker_find_file_by_id(uap->fileid); if (lf) { MOD_SLOCK; @@ -1013,7 +1018,7 @@ MOD_SUNLOCK; } else error = ENOENT; - mtx_unlock(&Giant); + KLD_UNLOCK(); return (error); } @@ -1044,12 +1049,12 @@ symstr = malloc(MAXPATHLEN, M_TEMP, M_WAITOK); if ((error = copyinstr(lookup.symname, symstr, MAXPATHLEN, NULL)) != 0) goto out; - mtx_lock(&Giant); + KLD_LOCK(); if (uap->fileid != 0) { lf = linker_find_file_by_id(uap->fileid); if (lf == NULL) { error = ENOENT; - mtx_unlock(&Giant); + KLD_UNLOCK(); goto out; } if (LINKER_LOOKUP_SYMBOL(lf, symstr, &sym) == 0 && @@ -1060,7 +1065,6 @@ } else error = ENOENT; } else { - mtx_lock(&kld_mtx); TAILQ_FOREACH(lf, &linker_files, link) { if (LINKER_LOOKUP_SYMBOL(lf, symstr, &sym) == 0 && LINKER_SYMBOL_VALUES(lf, sym, &symval) == 0) { @@ -1071,11 +1075,10 @@ break; } } - mtx_unlock(&kld_mtx); if (lf == NULL) error = ENOENT; } - mtx_unlock(&Giant); + KLD_UNLOCK(); out: free(symstr, M_TEMP); return (error); @@ -1179,6 +1182,7 @@ modlist_t mod; struct sysinit **si_start, **si_stop; + KLD_LOCK(); TAILQ_INIT(&loaded_files); TAILQ_INIT(&depended_files); TAILQ_INIT(&found_modules); @@ -1215,7 +1219,7 @@ /* * First get a list of stuff in the kernel. */ - if (linker_file_lookup_set(linker_kernel_file, MDT_SETNAME, &start, + if (LINKER_LOOKUP_SET(linker_kernel_file, MDT_SETNAME, &start, &stop, NULL) == 0) linker_addmodules(linker_kernel_file, start, stop, 1); @@ -1225,8 +1229,7 @@ */ restart: TAILQ_FOREACH(lf, &loaded_files, loaded) { - error = linker_file_lookup_set(lf, MDT_SETNAME, &start, - &stop, NULL); + error = LINKER_LOOKUP_SET(lf, MDT_SETNAME, &start, &stop, NULL); /* * First, look to see if we would successfully link with this * stuff. @@ -1276,7 +1279,7 @@ nver) != NULL) { printf("module %s already" " present!\n", modname); - linker_file_unload(lf, + linker_file_unload_internal(lf, LINKER_UNLOAD_FORCE); TAILQ_REMOVE(&loaded_files, lf, loaded); @@ -1303,7 +1306,7 @@ */ TAILQ_FOREACH(lf, &loaded_files, loaded) { printf("KLD file %s is missing dependencies\n", lf->filename); - linker_file_unload(lf, LINKER_UNLOAD_FORCE); + linker_file_unload_internal(lf, LINKER_UNLOAD_FORCE); TAILQ_REMOVE(&loaded_files, lf, loaded); } @@ -1319,8 +1322,7 @@ panic("cannot add dependency"); } lf->userrefs++; /* so we can (try to) kldunload it */ - error = linker_file_lookup_set(lf, MDT_SETNAME, &start, - &stop, NULL); + error = LINKER_LOOKUP_SET(lf, MDT_SETNAME, &start, &stop, NULL); if (!error) { for (mdp = start; mdp < stop; mdp++) { mp = *mdp; @@ -1347,16 +1349,17 @@ if (error) { printf("KLD file %s - could not finalize loading\n", lf->filename); - linker_file_unload(lf, LINKER_UNLOAD_FORCE); + linker_file_unload_internal(lf, LINKER_UNLOAD_FORCE); continue; } linker_file_register_modules(lf); - if (linker_file_lookup_set(lf, "sysinit_set", &si_start, + if (LINKER_LOOKUP_SET(lf, "sysinit_set", &si_start, &si_stop, NULL) == 0) sysinit_add(si_start, si_stop); linker_file_register_sysctls(lf); lf->flags |= LINKER_FILE_LINKED; } + KLD_UNLOCK(); /* woohoo! we made it! */ } @@ -1660,11 +1663,26 @@ struct linker_file *parent, struct mod_depend *verinfo, struct linker_file **lfpp) { + int error; + + KLD_LOCK(); + error = linker_load_module_internal(kldname, modname, parent, + verinfo, lfpp); + KLD_UNLOCK(); + return (error); +} + +static int +linker_load_module_internal(const char *kldname, const char *modname, + struct linker_file *parent, struct mod_depend *verinfo, + struct linker_file **lfpp) +{ linker_file_t lfdep; const char *filename; char *pathname; int error; + KLD_LOCK_ASSERT(); if (modname == NULL) { /* * We have to load KLD @@ -1696,17 +1714,15 @@ * provide different versions of the same modules. */ filename = linker_basename(pathname); - if (linker_find_file_by_name(filename)) { + if (linker_find_file_by_name(filename)) error = EEXIST; - goto out; - } - do { + else do { error = linker_load_file(pathname, &lfdep); if (error) break; if (modname && verinfo && modlist_lookup2(modname, verinfo) == NULL) { - linker_file_unload(lfdep, LINKER_UNLOAD_FORCE); + linker_file_unload_internal(lfdep, LINKER_UNLOAD_FORCE); error = ENOENT; break; } @@ -1718,9 +1734,7 @@ if (lfpp) *lfpp = lfdep; } while (0); -out: - if (pathname) - free(pathname, M_LINKER); + free(pathname, M_LINKER); return (error); } @@ -1740,16 +1754,16 @@ int ver, error = 0, count; /* - * All files are dependant on /kernel. + * All files are dependent on /kernel. */ + KLD_LOCK_ASSERT(); if (linker_kernel_file) { linker_kernel_file->refs++; error = linker_file_add_dependency(lf, linker_kernel_file); if (error) return (error); } - if (linker_file_lookup_set(lf, MDT_SETNAME, &start, &stop, - &count) != 0) + if (LINKER_LOOKUP_SET(lf, MDT_SETNAME, &start, &stop, &count) != 0) return (0); for (mdp = start; mdp < stop; mdp++) { mp = *mdp; @@ -1792,7 +1806,8 @@ break; continue; } - error = linker_load_module(NULL, modname, lf, verinfo, NULL); + error = linker_load_module_internal(NULL, modname, lf, verinfo, + NULL); if (error) { printf("KLD %s: depends on %s - not available\n", lf->filename, modname); @@ -1833,16 +1848,16 @@ error = sysctl_wire_old_buffer(req, 0); if (error != 0) return (error); - mtx_lock(&kld_mtx); + KLD_LOCK(); TAILQ_FOREACH(lf, &linker_files, link) { error = LINKER_EACH_FUNCTION_NAME(lf, sysctl_kern_function_list_iterate, req); if (error) { - mtx_unlock(&kld_mtx); + KLD_UNLOCK(); return (error); } } - mtx_unlock(&kld_mtx); + KLD_UNLOCK(); return (SYSCTL_OUT(req, "", 1)); } ==== //depot/projects/smpng/sys/kern/link_elf.c#32 (text+ko) ==== @@ -494,7 +494,7 @@ error = parse_dynamic(ef); if (error) { - linker_file_unload(lf, LINKER_UNLOAD_FORCE); + linker_file_unload_internal(lf, LINKER_UNLOAD_FORCE); return error; } link_elf_reloc_local(lf); @@ -852,7 +852,7 @@ out: if (error && lf) - linker_file_unload(lf, LINKER_UNLOAD_FORCE); + linker_file_unload_internal(lf, LINKER_UNLOAD_FORCE); if (shdr) free(shdr, M_LINKER); if (firstpage) ==== //depot/projects/smpng/sys/kern/link_elf_obj.c#8 (text+ko) ==== @@ -348,7 +348,7 @@ out: /* preload not done this way */ - linker_file_unload(lf, LINKER_UNLOAD_FORCE); + linker_file_unload_internal(lf, LINKER_UNLOAD_FORCE); return (error); } @@ -783,7 +783,7 @@ out: if (error && lf) - linker_file_unload(lf, LINKER_UNLOAD_FORCE); + linker_file_unload_internal(lf, LINKER_UNLOAD_FORCE); if (hdr) free(hdr, M_LINKER); VOP_UNLOCK(nd.ni_vp, 0, td); ==== //depot/projects/smpng/sys/notes#68 (text+ko) ==== @@ -79,6 +79,31 @@ recursion) and panic if we try to sleep with any held to provide a cheaper version of the current WITNESS check that doesn't bog the system down quite as bad. +- Fix kernel linker locking + + Convert mutex over to sx lock, needs to be held when calling the following + functions + - private: + + linker_file_sys[un]init? + + linker_file_[un]register_sysctls? + + linker_file_register_modules? + + linker_load_file? + + linker_load_module_internal + + linker_file_add_dependency (make private) + - public: + - linker_find_file_by_name? (make private?) + - linker_find_file_by_id? (XXX: ndis) + + linker_make_file + + linker_file_unload_internal + + linker_load_dependencies + - ndis XXX + - functions that grab the lock + - private: + - public: + + linker_reference_module + + linker_file_lookup_symbol + + linker_load_module + + linker_file_lookup_set + + linker_file_unload Active child branches: - jhb_intr - fast ithreads and MSI? (perhaps do MSI in jhb_acpipci) ==== //depot/projects/smpng/sys/sys/linker.h#16 (text+ko) ==== @@ -100,11 +100,6 @@ extern linker_file_t linker_kernel_file; /* - * Add a new file class to the linker. - */ -int linker_add_class(linker_class_t _cls); - -/* * Load a kernel module. */ int linker_load_module(const char *_kldname, const char *_modname, @@ -128,21 +123,11 @@ linker_file_t linker_find_file_by_id(int _fileid); /* - * Called from a class handler when a file is laoded. - */ -linker_file_t linker_make_file(const char* _filename, linker_class_t _cls); - -/* * Unload a file, freeing up memory. */ int linker_file_unload(linker_file_t _file, int flags); /* - * Add a dependency to a file. - */ -int linker_file_add_dependency(linker_file_t _file, linker_file_t _dep); - -/* * Lookup a symbol in a file. If deps is TRUE, look in dependencies * if not found in file. */ @@ -158,10 +143,12 @@ void *_start, void *_stop, int *_count); /* - * This routine is responsible for finding dependencies of userland - * initiated kldload(2)'s of files. + * Functions soley for use by the linker class handlers. */ +int linker_add_class(linker_class_t _cls); +int linker_file_unload_internal(linker_file_t _file, int flags); int linker_load_dependencies(linker_file_t _lf); +linker_file_t linker_make_file(const char* _filename, linker_class_t _cls); /* * DDB Helpers, tuned specifically for ddb/db_kld.c
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200603271334.k2RDYmx6016563>