Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 24 Aug 2013 21:07:04 +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: r254810 - head/sys/kern
Message-ID:  <201308242107.r7OL74v2069440@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: markj
Date: Sat Aug 24 21:07:04 2013
New Revision: 254810
URL: http://svnweb.freebsd.org/changeset/base/254810

Log:
  Remove the kld lock macros and just use the sx(9) API. Add locking in
  linker_init_kernel_modules() and linker_preload() in order to remove most
  of the checks for !cold before asserting that the kld lock is held. These
  routines are invoked by SYSINIT(9), so there's no harm in them taking the
  kld lock.

Modified:
  head/sys/kern/kern_linker.c

Modified: head/sys/kern/kern_linker.c
==============================================================================
--- head/sys/kern/kern_linker.c	Sat Aug 24 21:04:54 2013	(r254809)
+++ head/sys/kern/kern_linker.c	Sat Aug 24 21:07:04 2013	(r254810)
@@ -71,17 +71,6 @@ SYSCTL_INT(_debug, OID_AUTO, kld_debug, 
 TUNABLE_INT("debug.kld_debug", &kld_debug);
 #endif
 
-#define	KLD_LOCK()		sx_xlock(&kld_sx)
-#define	KLD_UNLOCK()		sx_xunlock(&kld_sx)
-#define	KLD_DOWNGRADE()		sx_downgrade(&kld_sx)
-#define	KLD_LOCK_READ()		sx_slock(&kld_sx)
-#define	KLD_UNLOCK_READ()	sx_sunlock(&kld_sx)
-#define	KLD_LOCKED()		sx_xlocked(&kld_sx)
-#define	KLD_LOCK_ASSERT() do {						\
-	if (!cold)							\
-		sx_assert(&kld_sx, SX_XLOCKED);				\
-} while (0)
-
 /*
  * static char *linker_search_path(const char *name, struct mod_depend
  * *verinfo);
@@ -121,7 +110,8 @@ static int linker_no_more_classes = 0;
 #define	LINKER_GET_NEXT_FILE_ID(a) do {					\
 	linker_file_t lftmp;						\
 									\
-	KLD_LOCK_ASSERT();						\
+	if (!cold)							\
+		sx_assert(&kld_sx, SA_XLOCKED);				\
 retry:									\
 	TAILQ_FOREACH(lftmp, &linker_files, link) {			\
 		if (next_file_id == lftmp->id) {			\
@@ -360,7 +350,9 @@ static void
 linker_init_kernel_modules(void)
 {
 
+	sx_xlock(&kld_sx);
 	linker_file_register_modules(linker_kernel_file);
+	sx_xunlock(&kld_sx);
 }
 
 SYSINIT(linker_kernel, SI_SUB_KLD, SI_ORDER_ANY, linker_init_kernel_modules,
@@ -377,7 +369,7 @@ linker_load_file(const char *filename, l
 	if (prison0.pr_securelevel > 0)
 		return (EPERM);
 
-	KLD_LOCK_ASSERT();
+	sx_assert(&kld_sx, SA_XLOCKED);
 	lf = linker_find_file_by_name(filename);
 	if (lf) {
 		KLD_DPF(FILE, ("linker_load_file: file %s is already loaded,"
@@ -411,10 +403,10 @@ linker_load_file(const char *filename, l
 				return (error);
 			}
 			modules = !TAILQ_EMPTY(&lf->modules);
-			KLD_UNLOCK();
+			sx_xunlock(&kld_sx);
 			linker_file_register_sysctls(lf);
 			linker_file_sysinit(lf);
-			KLD_LOCK();
+			sx_xlock(&kld_sx);
 			lf->flags |= LINKER_FILE_LINKED;
 
 			/*
@@ -465,16 +457,16 @@ linker_reference_module(const char *modn
 	modlist_t mod;
 	int error;
 
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	if ((mod = modlist_lookup2(modname, verinfo)) != NULL) {
 		*result = mod->container;
 		(*result)->refs++;
-		KLD_UNLOCK();
+		sx_xunlock(&kld_sx);
 		return (0);
 	}
 
 	error = linker_load_module(NULL, modname, NULL, verinfo, result);
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 	return (error);
 }
 
@@ -485,13 +477,13 @@ linker_release_module(const char *modnam
 	modlist_t mod;
 	int error;
 
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	if (lf == NULL) {
 		KASSERT(modname != NULL,
 		    ("linker_release_module: no file or name"));
 		mod = modlist_lookup2(modname, verinfo);
 		if (mod == NULL) {
-			KLD_UNLOCK();
+			sx_xunlock(&kld_sx);
 			return (ESRCH);
 		}
 		lf = mod->container;
@@ -499,7 +491,7 @@ linker_release_module(const char *modnam
 		KASSERT(modname == NULL && verinfo == NULL,
 		    ("linker_release_module: both file and name"));
 	error =	linker_file_unload(lf, LINKER_UNLOAD_NORMAL);
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 	return (error);
 }
 
@@ -512,7 +504,7 @@ linker_find_file_by_name(const char *fil
 	koname = malloc(strlen(filename) + 4, M_LINKER, M_WAITOK);
 	sprintf(koname, "%s.ko", filename);
 
-	KLD_LOCK_ASSERT();
+	sx_assert(&kld_sx, SA_XLOCKED);
 	TAILQ_FOREACH(lf, &linker_files, link) {
 		if (strcmp(lf->filename, koname) == 0)
 			break;
@@ -528,7 +520,7 @@ linker_find_file_by_id(int fileid)
 {
 	linker_file_t lf;
 
-	KLD_LOCK_ASSERT();
+	sx_assert(&kld_sx, SA_XLOCKED);
 	TAILQ_FOREACH(lf, &linker_files, link)
 		if (lf->id == fileid && lf->flags & LINKER_FILE_LINKED)
 			break;
@@ -541,13 +533,13 @@ linker_file_foreach(linker_predicate_t *
 	linker_file_t lf;
 	int retval = 0;
 
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	TAILQ_FOREACH(lf, &linker_files, link) {
 		retval = predicate(lf, context);
 		if (retval != 0)
 			break;
 	}
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 	return (retval);
 }
 
@@ -557,7 +549,8 @@ linker_make_file(const char *pathname, l
 	linker_file_t lf;
 	const char *filename;
 
-	KLD_LOCK_ASSERT();
+	if (!cold)
+		sx_assert(&kld_sx, SA_XLOCKED);
 	filename = linker_basename(pathname);
 
 	KLD_DPF(FILE, ("linker_make_file: new file, filename='%s' for pathname='%s'\n", filename, pathname));
@@ -591,7 +584,7 @@ linker_file_unload(linker_file_t file, i
 	if (prison0.pr_securelevel > 0)
 		return (EPERM);
 
-	KLD_LOCK_ASSERT();
+	sx_assert(&kld_sx, SA_XLOCKED);
 	KLD_DPF(FILE, ("linker_file_unload: lf->refs=%d\n", file->refs));
 
 	/* Easy case of just dropping a reference. */
@@ -664,10 +657,10 @@ linker_file_unload(linker_file_t file, i
 	 */
 	if (file->flags & LINKER_FILE_LINKED) {
 		file->flags &= ~LINKER_FILE_LINKED;
-		KLD_UNLOCK();
+		sx_xunlock(&kld_sx);
 		linker_file_sysuninit(file);
 		linker_file_unregister_sysctls(file);
-		KLD_LOCK();
+		sx_xlock(&kld_sx);
 	}
 	TAILQ_REMOVE(&linker_files, file, link);
 
@@ -706,7 +699,7 @@ linker_file_add_dependency(linker_file_t
 {
 	linker_file_t *newdeps;
 
-	KLD_LOCK_ASSERT();
+	sx_assert(&kld_sx, SA_XLOCKED);
 	newdeps = malloc((file->ndeps + 1) * sizeof(linker_file_t *),
 	    M_LINKER, M_WAITOK | M_ZERO);
 	if (newdeps == NULL)
@@ -738,12 +731,12 @@ linker_file_lookup_set(linker_file_t fil
 {
 	int error, locked;
 
-	locked = KLD_LOCKED();
+	locked = sx_xlocked(&kld_sx);
 	if (!locked)
-		KLD_LOCK();
+		sx_xlock(&kld_sx);
 	error = LINKER_LOOKUP_SET(file, name, firstp, lastp, countp);
 	if (!locked)
-		KLD_UNLOCK();
+		sx_xunlock(&kld_sx);
 	return (error);
 }
 
@@ -763,12 +756,12 @@ linker_file_lookup_symbol(linker_file_t 
 	caddr_t sym;
 	int locked;
 
-	locked = KLD_LOCKED();
+	locked = sx_xlocked(&kld_sx);
 	if (!locked)
-		KLD_LOCK();
+		sx_xlock(&kld_sx);
 	sym = linker_file_lookup_symbol_internal(file, name, deps);
 	if (!locked)
-		KLD_UNLOCK();
+		sx_xunlock(&kld_sx);
 	return (sym);
 }
 
@@ -782,7 +775,7 @@ linker_file_lookup_symbol_internal(linke
 	size_t common_size = 0;
 	int i;
 
-	KLD_LOCK_ASSERT();
+	sx_assert(&kld_sx, SA_XLOCKED);
 	KLD_DPF(SYM, ("linker_file_lookup_symbol: file=%p, name=%s, deps=%d\n",
 	    file, name, deps));
 
@@ -982,9 +975,9 @@ linker_search_symbol_name(caddr_t value,
 {
 	int error;
 
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	error = linker_debug_search_symbol_name(value, buf, buflen, offset);
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 	return (error);
 }
 
@@ -1026,10 +1019,10 @@ kern_kldload(struct thread *td, const ch
 		modname = file;
 	}
 
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	error = linker_load_module(kldname, modname, NULL, NULL, &lf);
 	if (error) {
-		KLD_UNLOCK();
+		sx_xunlock(&kld_sx);
 		goto done;
 	}
 	lf->userrefs++;
@@ -1039,13 +1032,13 @@ kern_kldload(struct thread *td, const ch
 	EVENTHANDLER_INVOKE(kld_load, lf);
 
 #ifdef HWPMC_HOOKS
-	KLD_DOWNGRADE();
+	sx_downgrade(&kld_sx);
 	pkm.pm_file = lf->filename;
 	pkm.pm_address = (uintptr_t) lf->address;
 	PMC_CALL_HOOK(td, PMC_FN_KLD_LOAD, (void *) &pkm);
-	KLD_UNLOCK_READ();
+	sx_sunlock(&kld_sx);
 #else
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 #endif
 
 done:
@@ -1088,7 +1081,7 @@ kern_kldunload(struct thread *td, int fi
 		return (error);
 
 	CURVNET_SET(TD_TO_VNET(td));
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	lf = linker_find_file_by_id(fileid);
 	if (lf) {
 		KLD_DPF(FILE, ("kldunload: lf->userrefs=%d\n", lf->userrefs));
@@ -1119,13 +1112,13 @@ kern_kldunload(struct thread *td, int fi
 
 #ifdef HWPMC_HOOKS
 	if (error == 0) {
-		KLD_DOWNGRADE();
+		sx_downgrade(&kld_sx);
 		PMC_CALL_HOOK(td, PMC_FN_KLD_UNLOAD, (void *) &pkm);
-		KLD_UNLOCK_READ();
+		sx_sunlock(&kld_sx);
 	} else
-		KLD_UNLOCK();
+		sx_xunlock(&kld_sx);
 #else
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 #endif
 	CURVNET_RESTORE();
 	return (error);
@@ -1169,13 +1162,13 @@ sys_kldfind(struct thread *td, struct kl
 		goto out;
 
 	filename = linker_basename(pathname);
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	lf = linker_find_file_by_name(filename);
 	if (lf)
 		td->td_retval[0] = lf->id;
 	else
 		error = ENOENT;
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 out:
 	free(pathname, M_TEMP);
 	return (error);
@@ -1193,7 +1186,7 @@ sys_kldnext(struct thread *td, struct kl
 		return (error);
 #endif
 
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	if (uap->fileid == 0)
 		lf = TAILQ_FIRST(&linker_files);
 	else {
@@ -1214,7 +1207,7 @@ sys_kldnext(struct thread *td, struct kl
 	else
 		td->td_retval[0] = 0;
 out:
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 	return (error);
 }
 
@@ -1253,10 +1246,10 @@ kern_kldstat(struct thread *td, int file
 		return (error);
 #endif
 
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	lf = linker_find_file_by_id(fileid);
 	if (lf == NULL) {
-		KLD_UNLOCK();
+		sx_xunlock(&kld_sx);
 		return (ENOENT);
 	}
 
@@ -1274,7 +1267,7 @@ kern_kldstat(struct thread *td, int file
 	if (namelen > MAXPATHLEN)
 		namelen = MAXPATHLEN;
 	bcopy(lf->pathname, &stat->pathname[0], namelen);
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 
 	td->td_retval[0] = 0;
 	return (0);
@@ -1293,7 +1286,7 @@ sys_kldfirstmod(struct thread *td, struc
 		return (error);
 #endif
 
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	lf = linker_find_file_by_id(uap->fileid);
 	if (lf) {
 		MOD_SLOCK;
@@ -1305,7 +1298,7 @@ sys_kldfirstmod(struct thread *td, struc
 		MOD_SUNLOCK;
 	} else
 		error = ENOENT;
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 	return (error);
 }
 
@@ -1333,7 +1326,7 @@ sys_kldsym(struct thread *td, struct kld
 	symstr = malloc(MAXPATHLEN, M_TEMP, M_WAITOK);
 	if ((error = copyinstr(lookup.symname, symstr, MAXPATHLEN, NULL)) != 0)
 		goto out;
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	if (uap->fileid != 0) {
 		lf = linker_find_file_by_id(uap->fileid);
 		if (lf == NULL)
@@ -1359,7 +1352,7 @@ sys_kldsym(struct thread *td, struct kld
 		if (lf == NULL)
 			error = ENOENT;
 	}
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 out:
 	free(symstr, M_TEMP);
 	return (error);
@@ -1468,6 +1461,7 @@ linker_preload(void *arg)
 	error = 0;
 
 	modptr = NULL;
+	sx_xlock(&kld_sx);
 	while ((modptr = preload_search_next_name(modptr)) != NULL) {
 		modname = (char *)preload_search_info(modptr, MODINFO_NAME);
 		modtype = (char *)preload_search_info(modptr, MODINFO_TYPE);
@@ -1649,6 +1643,7 @@ fail:
 		TAILQ_REMOVE(&depended_files, lf, loaded);
 		linker_file_unload(lf, LINKER_UNLOAD_FORCE);
 	}
+	sx_xunlock(&kld_sx);
 	/* woohoo! we made it! */
 }
 
@@ -1948,7 +1943,7 @@ linker_hwpmc_list_objects(void)
 	int i, nmappings;
 
 	nmappings = 0;
-	KLD_LOCK_READ();
+	sx_slock(&kld_sx);
 	TAILQ_FOREACH(lf, &linker_files, link)
 		nmappings++;
 
@@ -1963,7 +1958,7 @@ linker_hwpmc_list_objects(void)
 		kobase[i].pm_address = (uintptr_t)lf->address;
 		i++;
 	}
-	KLD_UNLOCK_READ();
+	sx_sunlock(&kld_sx);
 
 	KASSERT(i > 0, ("linker_hpwmc_list_objects: no kernel objects?"));
 
@@ -1989,7 +1984,7 @@ linker_load_module(const char *kldname, 
 	char *pathname;
 	int error;
 
-	KLD_LOCK_ASSERT();
+	sx_assert(&kld_sx, SA_XLOCKED);
 	if (modname == NULL) {
 		/*
  		 * We have to load KLD
@@ -2063,7 +2058,7 @@ linker_load_dependencies(linker_file_t l
 	/*
 	 * All files are dependant on /kernel.
 	 */
-	KLD_LOCK_ASSERT();
+	sx_assert(&kld_sx, SA_XLOCKED);
 	if (linker_kernel_file) {
 		linker_kernel_file->refs++;
 		error = linker_file_add_dependency(lf, linker_kernel_file);
@@ -2155,16 +2150,16 @@ sysctl_kern_function_list(SYSCTL_HANDLER
 	error = sysctl_wire_old_buffer(req, 0);
 	if (error != 0)
 		return (error);
-	KLD_LOCK();
+	sx_xlock(&kld_sx);
 	TAILQ_FOREACH(lf, &linker_files, link) {
 		error = LINKER_EACH_FUNCTION_NAME(lf,
 		    sysctl_kern_function_list_iterate, req);
 		if (error) {
-			KLD_UNLOCK();
+			sx_xunlock(&kld_sx);
 			return (error);
 		}
 	}
-	KLD_UNLOCK();
+	sx_xunlock(&kld_sx);
 	return (SYSCTL_OUT(req, "", 1));
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201308242107.r7OL74v2069440>