- Lock down the ``module'' structure by adding an SX lock that is used by

  all the global bits of ``module'' data.  This commit adds a few generic
  macros, MOD_SLOCK, MOD_XLOCK, etc., that are meant to be used as ways
  of accessing the SX lock.  It is also the first step in helping to lock
  down the kernel linker and module systems.

Reviewed by: jhb, jake, smp@

1 files changed, 58 insertions, 19 deletions

diff --git a/sys/kern/kern_module.c b/sys/kern/kern_module.c
index 438550d..f407466 100644
--- a/sys/kern/kern_module.c
+++ b/sys/kern/kern_module.c
@@ -33,11 +33,12 @@
 #include <sys/malloc.h>
 #include <sys/sysproto.h>
 #include <sys/sysent.h>
-#include <sys/module.h>
-#include <sys/linker.h>
 #include <sys/proc.h>
 #include <sys/lock.h>
 #include <sys/mutex.h>
+#include <sys/sx.h>
+#include <sys/module.h>
+#include <sys/linker.h>
 
 static MALLOC_DEFINE(M_MODULE, "module", "module data structures");
 
@@ -57,6 +58,7 @@ struct module {
 #define MOD_EVENT(mod, type)	(mod)->handler((mod), (type), (mod)->arg)
 
 static modulelist_t modules;
+struct sx modules_sx;
 static int nextid = 1;
 static void module_shutdown(void *, int);
 
@@ -66,11 +68,11 @@ modevent_nop(module_t mod, int what, void *arg)
 	return (0);
 }
 
-
 static void
 module_init(void *arg)
 {
 
+	sx_init(&modules_sx, "module subsystem sx lock");
 	TAILQ_INIT(&modules);
 	EVENTHANDLER_REGISTER(shutdown_post_sync, module_shutdown, NULL,
 	    SHUTDOWN_PRI_DEFAULT);
@@ -83,8 +85,10 @@ module_shutdown(void *arg1, int arg2)
 {
 	module_t mod;
 
+	MOD_SLOCK;
 	TAILQ_FOREACH(mod, &modules, link)
 		MOD_EVENT(mod, MOD_SHUTDOWN);
+	MOD_SUNLOCK;
 }
 
 void
@@ -94,14 +98,18 @@ module_register_init(const void *arg)
 	int error;
 	module_t mod;
 
+	MOD_SLOCK;
 	mod = module_lookupbyname(data->name);
 	if (mod == NULL)
 		panic("module_register_init: module named %s not found\n",
 		    data->name);
+	MOD_SUNLOCK;
 	error = MOD_EVENT(mod, MOD_LOAD);
 	if (error) {
 		MOD_EVENT(mod, MOD_UNLOAD);
+		MOD_XLOCK;
 		module_release(mod);
+		MOD_XUNLOCK;
 		printf("module_register_init: MOD_LOAD (%s, %p, %p) error"
 		    " %d\n", data->name, (void *)data->evhand, data->priv,
 		    error); 
@@ -114,16 +122,20 @@ module_register(const moduledata_t *data, linker_file_t container)
 	size_t namelen;
 	module_t newmod;
 
+	MOD_SLOCK;
 	newmod = module_lookupbyname(data->name);
 	if (newmod != NULL) {
+		MOD_SUNLOCK;
 		printf("module_register: module %s already exists!\n",
 		    data->name);
 		return (EEXIST);
 	}
+	MOD_SUNLOCK;
 	namelen = strlen(data->name) + 1;
 	newmod = malloc(sizeof(struct module) + namelen, M_MODULE, M_WAITOK);
 	if (newmod == NULL)
 		return (ENOMEM);
+	MOD_XLOCK;
 	newmod->refs = 1;
 	newmod->id = nextid++;
 	newmod->name = (char *)(newmod + 1);
@@ -136,6 +148,7 @@ module_register(const moduledata_t *data, linker_file_t container)
 	if (container)
 		TAILQ_INSERT_TAIL(&container->modules, newmod, flink);
 	newmod->file = container;
+	MOD_XUNLOCK;
 	return (0);
 }
 
@@ -143,6 +156,8 @@ void
 module_reference(module_t mod)
 {
 
+	MOD_XLOCK_ASSERT;
+
 	MOD_DPF(REFS, ("module_reference: before, refs=%d\n", mod->refs));
 	mod->refs++;
 }
@@ -151,17 +166,21 @@ void
 module_release(module_t mod)
 {
 
+	MOD_XLOCK_ASSERT;
+
 	if (mod->refs <= 0)
 		panic("module_release: bad reference count");
 
 	MOD_DPF(REFS, ("module_release: before, refs=%d\n", mod->refs));
-
+	
 	mod->refs--;
 	if (mod->refs == 0) {
 		TAILQ_REMOVE(&modules, mod, link);
 		if (mod->file)
 			TAILQ_REMOVE(&mod->file->modules, mod, flink);
+		MOD_XUNLOCK;
 		free(mod, M_MODULE);
+		MOD_XLOCK;
 	}
 }
 
@@ -171,6 +190,8 @@ module_lookupbyname(const char *name)
 	module_t mod;
 	int err;
 
+	MOD_LOCK_ASSERT;
+
 	TAILQ_FOREACH(mod, &modules, link) {
 		err = strcmp(mod->name, name);
 		if (err == 0)
@@ -182,26 +203,28 @@ module_lookupbyname(const char *name)
 module_t
 module_lookupbyid(int modid)
 {
-	module_t mod;
+        module_t mod;
 
-	TAILQ_FOREACH(mod, &modules, link) {
-		if (mod->id == modid)
-			return (mod);
-	}
-	return (NULL);
+        MOD_LOCK_ASSERT;
+
+        TAILQ_FOREACH(mod, &modules, link)
+                if (mod->id == modid)
+                        return(mod);
+        return (NULL);
 }
 
 int
 module_unload(module_t mod)
 {
 
-	return (MOD_EVENT(mod, MOD_UNLOAD));
+        return (MOD_EVENT(mod, MOD_UNLOAD));
 }
 
 int
 module_getid(module_t mod)
 {
 
+	MOD_LOCK_ASSERT;
 	return (mod->id);
 }
 
@@ -209,6 +232,7 @@ module_t
 module_getfnext(module_t mod)
 {
 
+	MOD_LOCK_ASSERT;
 	return (TAILQ_NEXT(mod, flink));
 }
 
@@ -216,6 +240,7 @@ void
 module_setspecific(module_t mod, modspecific_t *datap)
 {
 
+	MOD_XLOCK_ASSERT;
 	mod->data = *datap;
 }
 
@@ -234,6 +259,7 @@ modnext(struct thread *td, struct modnext_args *uap)
 	mtx_lock(&Giant);
 
 	td->td_retval[0] = -1;
+	MOD_SLOCK;
 	if (SCARG(uap, modid) == 0) {
 		mod = TAILQ_FIRST(&modules);
 		if (mod)
@@ -252,6 +278,7 @@ modnext(struct thread *td, struct modnext_args *uap)
 	else
 		td->td_retval[0] = 0;
 done2:
+	MOD_SUNLOCK;
 	mtx_unlock(&Giant);
 	return (error);
 }
@@ -269,6 +296,7 @@ modfnext(struct thread *td, struct modfnext_args *uap)
 
 	mtx_lock(&Giant);
 
+	MOD_SLOCK;
 	mod = module_lookupbyid(SCARG(uap, modid));
 	if (mod == NULL) {
 		error = ENOENT;
@@ -279,6 +307,7 @@ modfnext(struct thread *td, struct modfnext_args *uap)
 		else
 			td->td_retval[0] = 0;
 	}
+	MOD_SUNLOCK;
 	mtx_unlock(&Giant);
 	return (error);
 }
@@ -297,18 +326,26 @@ int
 modstat(struct thread *td, struct modstat_args *uap)
 {
 	module_t mod;
+	modspecific_t data;
 	int error = 0;
-	int namelen;
-	int version;
+	int id, namelen, refs, version;
 	struct module_stat *stat;
+	char *name;
 
 	mtx_lock(&Giant);
 
+	MOD_SLOCK;
 	mod = module_lookupbyid(SCARG(uap, modid));
 	if (mod == NULL) {
+		MOD_SUNLOCK;
 		error = ENOENT;
 		goto out;
 	}
+	id = mod->id;
+	refs = mod->refs;
+	name = mod->name;
+	data = mod->data;
+	MOD_SUNLOCK;
 	stat = SCARG(uap, stat);
 
 	/*
@@ -324,20 +361,20 @@ modstat(struct thread *td, struct modstat_args *uap)
 	namelen = strlen(mod->name) + 1;
 	if (namelen > MAXMODNAME)
 		namelen = MAXMODNAME;
-	if ((error = copyout(mod->name, &stat->name[0], namelen)) != 0)
+	if ((error = copyout(name, &stat->name[0], namelen)) != 0)
 		goto out;
 
-	if ((error = copyout(&mod->refs, &stat->refs, sizeof(int))) != 0)
+	if ((error = copyout(&refs, &stat->refs, sizeof(int))) != 0)
 		goto out;
-	if ((error = copyout(&mod->id, &stat->id, sizeof(int))) != 0)
+	if ((error = copyout(&id, &stat->id, sizeof(int))) != 0)
 		goto out;
 
 	/*
 	 * >v1 stat includes module data.
 	 */
 	if (version == sizeof(struct module_stat)) {
-		if ((error = copyout(&mod->data, &stat->data, 
-		    sizeof(mod->data))) != 0)
+		if ((error = copyout(&data, &stat->data, 
+		    sizeof(data))) != 0)
 			goto out;
 	}
 	td->td_retval[0] = 0;
@@ -360,11 +397,13 @@ modfind(struct thread *td, struct modfind_args *uap)
 		goto out;
 
 	mtx_lock(&Giant);
+	MOD_SLOCK;
 	mod = module_lookupbyname(name);
 	if (mod == NULL)
 		error = ENOENT;
 	else
-		td->td_retval[0] = mod->id;
+		td->td_retval[0] = module_getid(mod);
+	MOD_SUNLOCK;
 	mtx_unlock(&Giant);
 out:
 	return (error);