tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Module and device configuration locking [was Re: Modules loading modules?]



On Thu, 5 Aug 2010, Paul Goyette wrote:

OK, got it.

I'll have another set of patches in a few days.

Well, it was a slow day at the office today, so I found some time to work on this! :)

Attached is the latest version of this change. For simplicity, I have broken the patches up into five separate groups:

Part 1 defines a set of new kernel locking primitives in file
kern/kern_lock.c to implement the recursive kernconfig_mutex. (I'm not really stuck on the name, so open to alternative suggestions!) All of the locking in sys/kern_module.c has been updated to use this instead of the internal module_lock mutex. Additionally, the module_autoload() routine now does its own locking, rather than requiring its caller to do it. And the description of locking in the module(9) man page is updated.

Part 2 removes all of the explicit module_{,un}lock() calls from the various xxx_verbose modules that I'd previous worked on.

Part 3 removes all of the explicit module_{,un}lock() calls from other bits and pieces of the kernel. In a couple of places, new calls to
kernconfig_{,un}lock() are inserted.

Part 4 updates rump to provide equivalent locking routines. (pooka, I would appreciate you looking at this.)

Part 5 adds a new atf test-case to the existing module tests to verify that recursion actually works!



-------------------------------------------------------------------------
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:       |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com    |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |                          | pgoyette at netbsd.org  |
-------------------------------------------------------------------------
Index: sys/sys/param.h
===================================================================
RCS file: /cvsroot/src/sys/sys/param.h,v
retrieving revision 1.373
diff -u -p -r1.373 param.h
--- sys/sys/param.h     28 Jul 2010 11:03:47 -0000      1.373
+++ sys/sys/param.h     6 Aug 2010 00:46:05 -0000
@@ -63,7 +63,7 @@
  *     2.99.9          (299000900)
  */
 
-#define        __NetBSD_Version__      599003800       /* NetBSD 5.99.38 */
+#define        __NetBSD_Version__      599003900       /* NetBSD 5.99.39 */
 
 #define __NetBSD_Prereq__(M,m,p) (((((M) * 100000000) + \
     (m) * 1000000) + (p) * 100) <= __NetBSD_Version__)
Index: sys/sys/systm.h
===================================================================
RCS file: /cvsroot/src/sys/sys/systm.h,v
retrieving revision 1.239
diff -u -p -r1.239 systm.h
--- sys/sys/systm.h     31 Jan 2010 02:04:43 -0000      1.239
+++ sys/sys/systm.h     6 Aug 2010 00:46:06 -0000
@@ -492,6 +492,11 @@ void       kernel_lock_init(void);
 void   _kernel_lock(int);
 void   _kernel_unlock(int, int *);
 
+void   kernconfig_lock_init(void);
+void   kernconfig_lock(void);
+void   kernconfig_unlock(void);
+bool   kernconfig_is_held(void);
+
 #if defined(MULTIPROCESSOR) || defined(_MODULE)
 #define        KERNEL_LOCK(count, lwp)                 \
 do {                                           \
Index: sys/sys/module.h
===================================================================
RCS file: /cvsroot/src/sys/sys/module.h,v
retrieving revision 1.24
diff -u -p -r1.24 module.h
--- sys/sys/module.h    26 Jun 2010 07:23:57 -0000      1.24
+++ sys/sys/module.h    6 Aug 2010 00:46:07 -0000
@@ -115,7 +115,6 @@ __link_set_add_rodata(modules, name##_mo
 TAILQ_HEAD(modlist, module);
 
 extern struct vm_map   *module_map;
-extern kmutex_t                module_lock;
 extern u_int           module_count;
 extern u_int           module_builtinlist;
 extern struct modlist  module_list;
Index: sys/kern/kern_lock.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_lock.c,v
retrieving revision 1.150
diff -u -p -r1.150 kern_lock.c
--- sys/kern/kern_lock.c        20 Dec 2009 20:42:23 -0000      1.150
+++ sys/kern/kern_lock.c        6 Aug 2010 00:46:07 -0000
@@ -39,6 +39,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_lock.c,
 #include <sys/systm.h>
 #include <sys/kernel.h>
 #include <sys/lockdebug.h>
+#include <sys/mutex.h>
 #include <sys/cpu.h>
 #include <sys/syslog.h>
 #include <sys/atomic.h>
@@ -56,6 +57,10 @@ bool kernel_lock_dodebug;
 __cpu_simple_lock_t kernel_lock[CACHE_LINE_SIZE / sizeof(__cpu_simple_lock_t)]
     __aligned(CACHE_LINE_SIZE);
 
+static kmutex_t kernconfig_mutex;
+static lwp_t *kernconfig_lwp;
+static int kernconfig_recurse;
+
 void
 assert_sleepable(void)
 {
@@ -306,3 +311,59 @@ _kernel_unlock(int nlocks, int *countp)
        if (countp != NULL)
                *countp = olocks;
 }
+
+/*
+ * Functions for manipulating the kernel configuration lock.  This
+ * recursive lock should be used to protect all additions and removals
+ * of of kernel functionality, such as device configuration and loading
+ * of modular kernel components.
+ */
+
+void
+kernconfig_lock_init(void)
+{
+
+       mutex_init(&kernconfig_mutex, MUTEX_DEFAULT, IPL_NONE);
+       kernconfig_lwp = NULL;
+       kernconfig_recurse = 0;
+}
+
+void
+kernconfig_lock(void)
+{
+       lwp_t   *l;
+
+       /*
+        * OK to check this unlocked, since it could only be set to
+        * curlwp by the current thread itself, and not by an interrupt
+        * or any other LWP.
+        */
+       l = curlwp;
+       if (kernconfig_lwp == l) {
+               kernconfig_recurse++;
+               KASSERT(kernconfig_recurse != 0);
+       } else {
+               mutex_enter(&kernconfig_mutex);
+               kernconfig_lwp = l;
+               kernconfig_recurse = 1;
+       }
+}
+
+void
+kernconfig_unlock(void)
+{
+
+       KASSERT(kernconfig_is_held());
+       KASSERT(kernconfig_recurse != 0);
+       if (--kernconfig_recurse == 0) {
+               kernconfig_lwp = 0;
+               mutex_exit(&kernconfig_mutex);
+       }
+}
+
+bool
+kernconfig_is_held(void)
+{
+
+       return (mutex_owned(&kernconfig_mutex));
+}
Index: sys/kern/kern_module.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_module.c,v
retrieving revision 1.70
diff -u -p -r1.70 kern_module.c
--- sys/kern/kern_module.c      26 Jun 2010 07:23:57 -0000      1.70
+++ sys/kern/kern_module.c      6 Aug 2010 00:46:08 -0000
@@ -72,7 +72,6 @@ static int    module_verbose_on;
 static int     module_autoload_on = 1;
 u_int          module_count;
 u_int          module_builtinlist;
-kmutex_t       module_lock;
 u_int          module_autotime = 10;
 u_int          module_gen = 1;
 static kcondvar_t module_thread_cv;
@@ -223,7 +222,7 @@ module_builtin_add(modinfo_t *const *mip
                modp[i] = module_newmodule(MODULE_SOURCE_KERNEL);
                modp[i]->mod_info = mip[i+mipskip];
        }
-       mutex_enter(&module_lock);
+       kernconfig_lock();
 
        /* do this in three stages for error recovery and atomicity */
 
@@ -263,7 +262,7 @@ module_builtin_add(modinfo_t *const *mip
        }
 
  out:
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
        if (rv != 0) {
                for (i = 0; i < nmodinfo; i++) {
                        if (modp[i])
@@ -291,13 +290,13 @@ module_builtin_remove(modinfo_t *mi, boo
                if (rv)
                        return rv;
 
-               mutex_enter(&module_lock);
+               kernconfig_lock();
                rv = module_do_unload(mi->mi_name, true);
                if (rv) {
                        goto out;
                }
        } else {
-               mutex_enter(&module_lock);
+               kernconfig_lock();
        }
        TAILQ_FOREACH(mod, &module_builtins, mod_chain) {
                if (strcmp(mod->mod_info->mi_name, mi->mi_name) == 0)
@@ -312,7 +311,7 @@ module_builtin_remove(modinfo_t *mi, boo
        }
 
  out:
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
        return rv;
 }
 
@@ -332,8 +331,7 @@ module_init(void)
        if (module_map == NULL) {
                module_map = kernel_map;
        }
-       mutex_init(&module_lock, MUTEX_DEFAULT, IPL_NONE);
-       cv_init(&module_thread_cv, "modunload");
+       cv_init(&module_thread_cv, "mod_unld");
        mutex_init(&module_thread_lock, MUTEX_DEFAULT, IPL_NONE);
 
 #ifdef MODULAR /* XXX */
@@ -388,11 +386,11 @@ module_builtin_require_force(void)
 {
        module_t *mod;
 
-       mutex_enter(&module_lock);
+       kernconfig_lock();
        TAILQ_FOREACH(mod, &module_builtins, mod_chain) {
                module_require_force(mod);
        }
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
 }
 
 static struct sysctllog *module_sysctllog;
@@ -444,7 +442,7 @@ module_init_class(modclass_t class)
        module_t *mod;
        modinfo_t *mi;
 
-       mutex_enter(&module_lock);
+       kernconfig_lock();
        /*
         * Builtins first.  These will not depend on pre-loaded modules
         * (because the kernel would not link).
@@ -493,7 +491,7 @@ module_init_class(modclass_t class)
                TAILQ_INSERT_TAIL(&module_builtins, mod, mod_chain);
        }
 
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
 }
 
 /*
@@ -534,10 +532,10 @@ module_load(const char *filename, int fl
                return error;
        }
 
-       mutex_enter(&module_lock);
+       kernconfig_lock();
        error = module_do_load(filename, false, flags, props, NULL, class,
            false);
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
 
        return error;
 }
@@ -552,27 +550,31 @@ module_autoload(const char *filename, mo
 {
        int error;
 
-       KASSERT(mutex_owned(&module_lock));
+       kernconfig_lock();
 
        /* Nothing if the user has disabled it. */
        if (!module_autoload_on) {
+               kernconfig_unlock();
                return EPERM;
        }
 
         /* Disallow path separators and magic symlinks. */
         if (strchr(filename, '/') != NULL || strchr(filename, '@') != NULL ||
             strchr(filename, '.') != NULL) {
+               kernconfig_unlock();
                return EPERM;
        }
 
        /* Authorize. */
        error = kauth_authorize_system(kauth_cred_get(), KAUTH_SYSTEM_MODULE,
            0, (void *)(uintptr_t)MODCTL_LOAD, (void *)(uintptr_t)1, NULL);
-       if (error != 0) {
-               return error;
-       }
 
-       return module_do_load(filename, false, 0, NULL, NULL, class, true);
+       if (error == 0)
+               error = module_do_load(filename, false, 0, NULL, NULL, class,
+                   true);
+
+       kernconfig_unlock();
+       return error;
 }
 
 /*
@@ -592,9 +594,9 @@ module_unload(const char *name)
                return error;
        }
 
-       mutex_enter(&module_lock);
+       kernconfig_lock();
        error = module_do_unload(name, true);
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
 
        return error;
 }
@@ -609,7 +611,7 @@ module_lookup(const char *name)
 {
        module_t *mod;
 
-       KASSERT(mutex_owned(&module_lock));
+       KASSERT(kernconfig_is_held);
 
        TAILQ_FOREACH(mod, &module_list, mod_chain) {
                if (strcmp(mod->mod_info->mi_name, name) == 0) {
@@ -632,14 +634,14 @@ module_hold(const char *name)
 {
        module_t *mod;
 
-       mutex_enter(&module_lock);
+       kernconfig_lock();
        mod = module_lookup(name);
        if (mod == NULL) {
-               mutex_exit(&module_lock);
+               kernconfig_unlock();
                return ENOENT;
        }
        mod->mod_refcnt++;
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
 
        return 0;
 }
@@ -654,14 +656,14 @@ module_rele(const char *name)
 {
        module_t *mod;
 
-       mutex_enter(&module_lock);
+       kernconfig_lock();
        mod = module_lookup(name);
        if (mod == NULL) {
-               mutex_exit(&module_lock);
+               kernconfig_unlock();
                panic("module_rele: gone");
        }
        mod->mod_refcnt--;
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
 }
 
 /*
@@ -674,7 +676,7 @@ module_enqueue(module_t *mod)
 {
        int i;
 
-       KASSERT(mutex_owned(&module_lock));
+       KASSERT(kernconfig_is_held);
 
        /*
         * If there are requisite modules, put at the head of the queue.
@@ -712,7 +714,7 @@ module_do_builtin(const char *name, modu
        size_t len;
        int error;
 
-       KASSERT(mutex_owned(&module_lock));
+       KASSERT(kernconfig_is_held);
 
        /*
         * Search the list to see if we have a module by this name.
@@ -820,7 +822,7 @@ module_do_load(const char *name, bool is
        int error;
        size_t len;
 
-       KASSERT(mutex_owned(&module_lock));
+       KASSERT(kernconfig_is_held);
 
        filedict = NULL;
        error = 0;
@@ -1103,7 +1105,7 @@ module_do_unload(const char *name, bool 
        int error;
        u_int i;
 
-       KASSERT(mutex_owned(&module_lock));
+       KASSERT(kernconfig_is_held);
 
        mod = module_lookup(name);
        if (mod == NULL) {
@@ -1223,7 +1225,7 @@ int
 module_find_section(const char *name, void **addr, size_t *size)
 {
 
-       KASSERT(mutex_owned(&module_lock));
+       KASSERT(kernconfig_is_held);
        KASSERT(module_active != NULL);
 
        return kobj_find_section(module_active->mod_kobj, name, addr, size);
@@ -1244,7 +1246,7 @@ module_thread(void *cookie)
        int error;
 
        for (;;) {
-               mutex_enter(&module_lock);
+               kernconfig_lock();
                for (mod = TAILQ_FIRST(&module_list); mod != NULL; mod = next) {
                        next = TAILQ_NEXT(mod, mod_chain);
                        if (mod->mod_source == MODULE_SOURCE_KERNEL)
@@ -1271,7 +1273,7 @@ module_thread(void *cookie)
                                (void)module_do_unload(mi->mi_name, false);
                        }
                }
-               mutex_exit(&module_lock);
+               kernconfig_unlock();
 
                mutex_enter(&module_thread_lock);
                (void)cv_timedwait(&module_thread_cv, &module_thread_lock,
Index: sys/kern/init_main.c
===================================================================
RCS file: /cvsroot/src/sys/kern/init_main.c,v
retrieving revision 1.422
diff -u -p -r1.422 init_main.c
--- sys/kern/init_main.c        26 Jun 2010 07:23:57 -0000      1.422
+++ sys/kern/init_main.c        6 Aug 2010 00:46:09 -0000
@@ -303,6 +303,7 @@ main(void)
        kernel_lock_init();
        once_init();
        mutex_init(&cpu_lock, MUTEX_DEFAULT, IPL_NONE);
+       kernconfig_lock_init();
 
        /* Initialize the device switch tables. */
        devsw_init();
Index: share/man/man9/module.9
===================================================================
RCS file: /cvsroot/src/share/man/man9/module.9,v
retrieving revision 1.7
diff -u -p -r1.7 module.9
--- share/man/man9/module.9     4 Aug 2010 18:52:49 -0000       1.7
+++ share/man/man9/module.9     6 Aug 2010 00:46:10 -0000
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd August 4, 2010
+.Dd August XXX, 2010
 .Dt MODULE 9
 .Os
 .Sh NAME
@@ -75,6 +75,7 @@ The
 .Vt modinfo_t
 type resides within the module itself, and contains module header info.
 .El
+The module subsystem is protected by the global kernconfig_mutex.
 .Sh FUNCTIONS
 .Bl -tag -width abcd
 .It Fn MODULE "class" "name" "required"
@@ -283,21 +284,6 @@ A module cannot be unloaded if its refer
 .It Fn module_rele "name"
 Decrement the reference count of a module.
 .El
-.Sh LOCK PROTOCOL
-The
-.Nm
-subsystem is protected with the global
-.Vt module_mutex .
-This
-.Xr mutex 9
-must be acquired before calling any of these routines.
-As an exception, the
-.Fn module_load
-routine acquires the mutex itself, so one does not need to acquire it
-before calling
-.Fn module_load .
-Loading of a module and its required modules occurs as an atomic
-operation, and either completely succeeds or completely fails.
 .Sh CODE REFERENCES
 This section describes places within the
 .Nx
Index: sys/dev/acpi/acpi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.208
diff -u -p -r1.208 acpi.c
--- sys/dev/acpi/acpi.c 25 Jul 2010 12:54:46 -0000      1.208
+++ sys/dev/acpi/acpi.c 6 Aug 2010 00:46:11 -0000
@@ -119,7 +119,7 @@ static int acpi_dbgr = 0x00;
  * subsystems that ACPI supercedes) when ACPI is active.
  */
 int    acpi_active;
-int    acpi_force_load;
+int    acpi_force_load = 0;
 int    acpi_suspended = 0;
 
 struct acpi_softc *acpi_softc;
@@ -226,11 +226,8 @@ acpi_null(void)
 void
 acpi_load_verbose(void)
 {
-       if (acpi_verbose_loaded == 0) {
-               mutex_enter(&module_lock);
+       if (acpi_verbose_loaded == 0)
                module_autoload("acpiverbose", MODULE_CLASS_MISC);
-               mutex_exit(&module_lock);
-       }
 }
 
 void
Index: sys/dev/mii/mii_physubr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/mii/mii_physubr.c,v
retrieving revision 1.71
diff -u -p -r1.71 mii_physubr.c
--- sys/dev/mii/mii_physubr.c   25 Jul 2010 14:44:34 -0000      1.71
+++ sys/dev/mii/mii_physubr.c   6 Aug 2010 00:46:11 -0000
@@ -71,11 +71,8 @@ const char *mii_get_descr_stub(int oui, 
  */
 void mii_load_verbose(void)
 {
-       if (mii_verbose_loaded == 0) {
-               mutex_enter(&module_lock);
+       if (mii_verbose_loaded == 0)
                module_autoload("miiverbose", MODULE_CLASS_MISC);
-               mutex_exit(&module_lock);
-       }
 }  
 
 static void mii_phy_statusmsg(struct mii_softc *);
Index: sys/dev/pci/pci_subr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pci_subr.c,v
retrieving revision 1.84
diff -u -p -r1.84 pci_subr.c
--- sys/dev/pci/pci_subr.c      25 Jul 2010 14:14:25 -0000      1.84
+++ sys/dev/pci/pci_subr.c      6 Aug 2010 00:46:12 -0000
@@ -323,11 +323,8 @@ int pciverbose_loaded = 0;
  */
 void pci_load_verbose(void)
 {
-       if (pciverbose_loaded == 0) {
-               mutex_enter(&module_lock);
+       if (pciverbose_loaded == 0)
                module_autoload("pciverbose", MODULE_CLASS_MISC);
-               mutex_exit(&module_lock);
-       }
 }
 
 const char *pci_findvendor_stub(pcireg_t id_reg)
Index: sys/dev/scsipi/scsipiconf.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/scsipiconf.c,v
retrieving revision 1.39
diff -u -p -r1.39 scsipiconf.c
--- sys/dev/scsipi/scsipiconf.c 25 Jul 2010 13:49:58 -0000      1.39
+++ sys/dev/scsipi/scsipiconf.c 6 Aug 2010 00:46:13 -0000
@@ -107,11 +107,8 @@ scsipi_command(struct scsipi_periph *per
 void
 scsipi_load_verbose(void)
 {
-       if (scsi_verbose_loaded == 0) {
-               mutex_enter(&module_lock);
+       if (scsi_verbose_loaded == 0)
                module_autoload("scsiverbose", MODULE_CLASS_MISC);
-               mutex_exit(&module_lock);
-       }
 }
 
 /*
Index: sys/dev/usb/usb_subr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.174
diff -u -p -r1.174 usb_subr.c
--- sys/dev/usb/usb_subr.c      27 Jul 2010 16:15:30 -0000      1.174
+++ sys/dev/usb/usb_subr.c      6 Aug 2010 00:46:14 -0000
@@ -122,11 +122,8 @@ int usb_verbose_loaded = 0;
  */
 void usb_load_verbose(void)
 {
-       if (usb_verbose_loaded == 0) {
-               mutex_enter(&module_lock);
+       if (usb_verbose_loaded == 0)
                module_autoload("usbverbose", MODULE_CLASS_MISC);
-               mutex_exit(&module_lock);
-       }
 }
 
 void get_usb_vendor_stub(char *v, usb_vendor_id_t v_id)
Index: sys/kern/kern_exec.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_exec.c,v
retrieving revision 1.299
diff -u -p -r1.299 kern_exec.c
--- sys/kern/kern_exec.c        7 Jul 2010 01:30:37 -0000       1.299
+++ sys/kern/kern_exec.c        6 Aug 2010 00:46:15 -0000
@@ -500,17 +500,13 @@ exec_autoload(void)
        char const * const *list;
        int i;
 
-       mutex_enter(&module_lock);
        list = (nexecs == 0 ? native : compat);
        for (i = 0; list[i] != NULL; i++) {
                if (module_autoload(list[i], MODULE_CLASS_MISC) != 0) {
                        continue;
                }
-               mutex_exit(&module_lock);
                yield();
-               mutex_enter(&module_lock);
        }
-       mutex_exit(&module_lock);
 #endif
 }
 
Index: sys/kern/kern_syscall.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_syscall.c,v
retrieving revision 1.4
diff -u -p -r1.4 kern_syscall.c
--- sys/kern/kern_syscall.c     15 Apr 2010 20:46:08 -0000      1.4
+++ sys/kern/kern_syscall.c     6 Aug 2010 00:46:15 -0000
@@ -45,6 +45,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_syscall
 #include <sys/syscall.h>
 #include <sys/syscallargs.h>
 #include <sys/syscallvar.h>
+#include <sys/systm.h>
 #include <sys/xcall.h>
 
 int
@@ -199,13 +200,13 @@ sys_nomodule(struct lwp *l, const void *
 
        /*
         * Restart the syscall if we interrupted a module unload that
-        * failed.  Acquiring module_lock delays us until any unload
+        * failed.  Acquiring kernconfig_lock delays us until any unload
         * has been completed or rolled back.
         */
-       mutex_enter(&module_lock);
+       kernconfig_lock();
        sy = l->l_sysent;
        if (sy->sy_call != sys_nomodule) {
-               mutex_exit(&module_lock);
+               kernconfig_unlock();
                return ERESTART;
        }
        /*
@@ -224,11 +225,11 @@ sys_nomodule(struct lwp *l, const void *
                            sy->sy_call == sys_nomodule) {
                                break;
                        }
-                       mutex_exit(&module_lock);
+                       kernconfig_unlock();
                        return ERESTART;
                }
        }
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
 #endif /* MODULAR */
 
        return sys_nosys(l, v, retval);
@@ -240,7 +241,7 @@ syscall_establish(const struct emul *em,
        struct sysent *sy;
        int i;
 
-       KASSERT(mutex_owned(&module_lock));
+       KASSERT(kernconfig_is_held());
 
        if (em == NULL) {
                em = &emul_netbsd;
@@ -277,7 +278,7 @@ syscall_disestablish(const struct emul *
        lwp_t *l;
        int i;
 
-       KASSERT(mutex_owned(&module_lock));
+       KASSERT(kernconfig_is_held());
 
        if (em == NULL) {
                em = &emul_netbsd;
@@ -320,7 +321,7 @@ syscall_disestablish(const struct emul *
                /*
                 * We lose: one or more calls are still in use.  Put back
                 * the old entrypoints and act like nothing happened.
-                * When we drop module_lock, any system calls held in
+                * When we drop kernconfig_lock, any system calls held in
                 * sys_nomodule() will be restarted.
                 */
                for (i = 0; sp[i].sp_call != NULL; i++) {
Index: sys/kern/sys_module.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_module.c,v
retrieving revision 1.11
diff -u -p -r1.11 sys_module.c
--- sys/kern/sys_module.c       5 Mar 2010 18:35:01 -0000       1.11
+++ sys/kern/sys_module.c       6 Aug 2010 00:46:16 -0000
@@ -144,11 +144,11 @@ sys_modctl(struct lwp *l, const struct s
                if (error != 0) {
                        break;
                }
-               mutex_enter(&module_lock);
+               kernconfig_lock();
                mslen = (module_count+module_builtinlist+1) * sizeof(modstat_t);
                mso = kmem_zalloc(mslen, KM_SLEEP);
                if (mso == NULL) {
-                       mutex_exit(&module_lock);
+                       kernconfig_unlock();
                        return ENOMEM;
                }
                ms = mso;
@@ -187,7 +187,7 @@ sys_modctl(struct lwp *l, const struct s
                        ms->ms_source = mod->mod_source;
                        ms++;
                }
-               mutex_exit(&module_lock);
+               kernconfig_unlock();
                error = copyout(mso, iov.iov_base,
                    min(mslen - sizeof(modstat_t), iov.iov_len));
                kmem_free(mso, mslen);
Index: sys/kern/sys_sig.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_sig.c,v
retrieving revision 1.28
diff -u -p -r1.28 sys_sig.c
--- sys/kern/sys_sig.c  1 Jul 2010 02:38:31 -0000       1.28
+++ sys/kern/sys_sig.c  6 Aug 2010 00:46:17 -0000
@@ -347,13 +347,13 @@ sigaction1(struct lwp *l, int signum, co
         *
         * If version < 2, we try to autoload the compat module.  Note
         * that we interlock with the unload check in compat_modcmd()
-        * using module_lock.  If the autoload fails, we don't try it
+        * using kernconfig_lock.  If the autoload fails, we don't try it
         * again for this process.
         */
        if (nsa != NULL) {
                if (__predict_false(vers < 2) &&
                    (p->p_lflag & PL_SIGCOMPAT) == 0) {
-                       mutex_enter(&module_lock);
+                       kernconfig_lock();
                        if (sendsig_sigcontext_vec == NULL) {
                                (void)module_autoload("compat",
                                    MODULE_CLASS_ANY);
@@ -374,7 +374,7 @@ sigaction1(struct lwp *l, int signum, co
                         */
                        p->p_lflag |= PL_SIGCOMPAT;
                        mutex_exit(proc_lock);
-                       mutex_exit(&module_lock);
+                       kernconfig_unlock();
                }
 
                switch (vers) {
Index: sys/kern/tty.c
===================================================================
RCS file: /cvsroot/src/sys/kern/tty.c,v
retrieving revision 1.237
diff -u -p -r1.237 tty.c
--- sys/kern/tty.c      1 Jul 2010 02:38:31 -0000       1.237
+++ sys/kern/tty.c      6 Aug 2010 00:46:18 -0000
@@ -1264,13 +1264,10 @@ ttioctl(struct tty *tp, u_long cmd, void
                                break;
                        }
                        rw_exit(&ttcompat_lock);
-                       mutex_enter(&module_lock);
                        (void)module_autoload("compat", MODULE_CLASS_ANY);
                        if (ttcompatvec == NULL) {
-                               mutex_exit(&module_lock);
                                return EPASSTHROUGH;
                        }
-                       mutex_exit(&module_lock);
                }
                error = (*ttcompatvec)(tp, cmd, data, flag, l);
                rw_exit(&ttcompat_lock);
Index: sys/kern/uipc_accf.c
===================================================================
RCS file: /cvsroot/src/sys/kern/uipc_accf.c,v
retrieving revision 1.11
diff -u -p -r1.11 uipc_accf.c
--- sys/kern/uipc_accf.c        13 Mar 2010 23:03:39 -0000      1.11
+++ sys/kern/uipc_accf.c        6 Aug 2010 00:46:19 -0000
@@ -167,10 +167,8 @@ accept_filt_get(char *name)
                /* Try to autoload a module to satisfy the request. */
                strcpy(buf, "accf_");
                strlcat(buf, name, sizeof(buf));
-               mutex_enter(&module_lock);
                gen = module_gen;
                (void)module_autoload(buf, MODULE_CLASS_ANY);
-               mutex_exit(&module_lock);
        } while (gen != module_gen);
 
        return p;
Index: sys/kern/vfs_syscalls.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.407
diff -u -p -r1.407 vfs_syscalls.c
--- sys/kern/vfs_syscalls.c     30 Jun 2010 15:44:54 -0000      1.407
+++ sys/kern/vfs_syscalls.c     6 Aug 2010 00:46:19 -0000
@@ -278,9 +278,7 @@ mount_get_vfsops(const char *fstype, str
                return 0;
 
        /* If we can autoload a vfs module, try again */
-       mutex_enter(&module_lock);
        (void)module_autoload(fstypename, MODULE_CLASS_VFS);
-       mutex_exit(&module_lock);
 
        if ((*vfsops = vfs_getopsbyname(fstypename)) != NULL)
                return 0;
Index: sys/miscfs/specfs/spec_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/miscfs/specfs/spec_vnops.c,v
retrieving revision 1.130
diff -u -p -r1.130 spec_vnops.c
--- sys/miscfs/specfs/spec_vnops.c      24 Jun 2010 13:03:17 -0000      1.130
+++ sys/miscfs/specfs/spec_vnops.c      6 Aug 2010 00:46:20 -0000
@@ -459,9 +459,7 @@ spec_open(void *v)
                                break;
                        
                        /* Try to autoload device module */
-                       mutex_enter(&module_lock);
                        (void) module_autoload(name, MODULE_CLASS_DRIVER);
-                       mutex_exit(&module_lock);
                } while (gen != module_gen);
 
                vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
@@ -512,9 +510,7 @@ spec_open(void *v)
                        VOP_UNLOCK(vp);
 
                         /* Try to autoload device module */
-                       mutex_enter(&module_lock);
                        (void) module_autoload(name, MODULE_CLASS_DRIVER);
-                       mutex_exit(&module_lock);
                        
                        vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
                } while (gen != module_gen);
Index: sys/net/if_ppp.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ppp.c,v
retrieving revision 1.131
diff -u -p -r1.131 if_ppp.c
--- sys/net/if_ppp.c    5 Apr 2010 07:22:23 -0000       1.131
+++ sys/net/if_ppp.c    6 Aug 2010 00:46:21 -0000
@@ -1820,7 +1820,7 @@ ppp_get_compressor(uint8_t ci)
        if (cp != NULL)
                return cp;
 
-       mutex_enter(&module_lock);
+       kernconfig_lock();
        mutex_enter(&ppp_compressors_mtx);
        cp = ppp_get_compressor_noload(ci, true);
        mutex_exit(&ppp_compressors_mtx);
@@ -1838,7 +1838,7 @@ ppp_get_compressor(uint8_t ci)
                        }
                }
        }
-       mutex_exit(&module_lock);
+       kernconfig_unlock();
 
        return cp;
 }
Index: sys/dev/dm/dm_target.c
===================================================================
RCS file: /cvsroot/src/sys/dev/dm/dm_target.c,v
retrieving revision 1.13
diff -u -p -r1.13 dm_target.c
--- sys/dev/dm/dm_target.c      18 May 2010 15:10:41 -0000      1.13
+++ sys/dev/dm/dm_target.c      6 Aug 2010 00:46:22 -0000
@@ -83,9 +83,7 @@ dm_target_autoload(const char *dm_target
                gen = module_gen;
 
                /* Try to autoload target module */
-               mutex_enter(&module_lock);
                (void) module_autoload(name, MODULE_CLASS_MISC);
-               mutex_exit(&module_lock);
        } while (gen != module_gen);
 
        mutex_enter(&dm_target_mutex);
Index: sys/rump/librump/rumpkern/rump_private.h
===================================================================
RCS file: /cvsroot/src/sys/rump/librump/rumpkern/rump_private.h,v
retrieving revision 1.54
diff -u -p -r1.54 rump_private.h
--- sys/rump/librump/rumpkern/rump_private.h    14 Jun 2010 21:04:56 -0000      
1.54
+++ sys/rump/librump/rumpkern/rump_private.h    6 Aug 2010 00:46:24 -0000
@@ -51,6 +51,10 @@ extern kauth_cred_t rump_cred;
 
 extern struct rumpuser_mtx *rump_giantlock;
 
+extern kmutex_t kernconfig_mutex;
+extern lwp_t *kernconfig_lwp;
+extern int kernconfig_recurse;
+
 #define UIO_VMSPACE_SYS (&vmspace0)
 
 extern int rump_threads;
Index: sys/rump/librump/rumpkern/klock.c
===================================================================
RCS file: /cvsroot/src/sys/rump/librump/rumpkern/klock.c,v
retrieving revision 1.2
diff -u -p -r1.2 klock.c
--- sys/rump/librump/rumpkern/klock.c   18 May 2010 15:16:10 -0000      1.2
+++ sys/rump/librump/rumpkern/klock.c   6 Aug 2010 00:46:24 -0000
@@ -38,6 +38,10 @@ __KERNEL_RCSID(0, "$NetBSD: klock.c,v 1.
 
 #include "rump_private.h"
 
+static kmutex_t rump_kernconfig_mutex;
+static lwp_t *rump_kernconfig_lwp;
+static int rump_kernconfig_recurse;
+
 /*
  * giant lock
  */
@@ -122,6 +126,64 @@ _kernel_unlock(int nlocks, int *countp)
        }
 }
 
+/*
+ * Handle the kernel configuration lock
+ *
+ * Functions for manipulating the kernel configuration lock.  This
+ * recursive lock should be used to protect all additions and removals
+ * of of kernel functionality, such as device configuration and loading
+ * of modular kernel components.
+ */
+
+void
+kernconfig_lock_init(void)
+{
+
+       mutex_init(&rump_kernconfig_mutex, MUTEX_DEFAULT, IPL_NONE);
+       rump_kernconfig_lwp = NULL;
+       rump_kernconfig_recurse = 0;
+}
+
+void
+kernconfig_lock(void)
+{
+       lwp_t   *l;
+
+       /*
+        * OK to check this unlocked, since it could only be set to
+        * curlwp by the current thread itself, and not by an interrupt
+        * or any other LWP.
+        */
+       l = curlwp;
+       if (rump_kernconfig_lwp == l) {
+               rump_kernconfig_recurse++;
+               KASSERT(rump_kernconfig_recurse != 0);
+       } else {
+               mutex_enter(&rump_kernconfig_mutex);
+               rump_kernconfig_lwp = l;
+               rump_kernconfig_recurse = 1;
+       }
+}
+
+void
+kernconfig_unlock(void)
+{
+
+       KASSERT(kernconfig_is_held());
+       KASSERT(rump_kernconfig_recurse != 0);
+       if (--rump_kernconfig_recurse == 0) {
+               rump_kernconfig_lwp = 0;
+               mutex_exit(&rump_kernconfig_mutex);
+       }
+}
+
+bool
+kernconfig_is_held(void)
+{        
+        
+       return (mutex_owned(&rump_kernconfig_mutex));
+}     
+
 void
 rump_user_unschedule(int nlocks, int *countp, void *interlock)
 {
Index: distrib/sets/lists/tests/module.mi
===================================================================
RCS file: /cvsroot/src/distrib/sets/lists/tests/module.mi,v
retrieving revision 1.3
diff -u -p -r1.3 module.mi
--- distrib/sets/lists/tests/module.mi  15 Dec 2009 03:01:16 -0000      1.3
+++ distrib/sets/lists/tests/module.mi  6 Aug 2010 00:46:23 -0000
@@ -5,5 +5,7 @@
 ./usr/tests/modules/Atffile                    tests-sys-tests         atf
 ./usr/tests/modules/k_helper                   tests-sys-tests         atf
 ./usr/tests/modules/k_helper/k_helper.kmod     tests-sys-tests         atf
+./usr/tests/modules/k_helper2                  tests-sys-tests         atf
+./usr/tests/modules/k_helper2/k_helper2.kmod   tests-sys-tests         atf
 ./usr/tests/modules/t_modctl                   tests-sys-tests         atf
 ./usr/tests/modules/t_modload                  tests-sys-tests         atf
Index: tests/modules/Makefile
===================================================================
RCS file: /cvsroot/src/tests/modules/Makefile,v
retrieving revision 1.5
diff -u -p -r1.5 Makefile
--- tests/modules/Makefile      13 Jul 2010 21:13:28 -0000      1.5
+++ tests/modules/Makefile      6 Aug 2010 00:46:25 -0000
@@ -15,5 +15,6 @@ LDADD=                -lprop
 TESTS_SH=      t_modload
 
 SUBDIR=                k_helper
+SUBDIR+=       k_helper2
 
 .include <bsd.test.mk>
Index: tests/modules/t_modctl.c
===================================================================
RCS file: /cvsroot/src/tests/modules/t_modctl.c,v
retrieving revision 1.3
diff -u -p -r1.3 t_modctl.c
--- tests/modules/t_modctl.c    4 Jan 2009 17:56:57 -0000       1.3
+++ tests/modules/t_modctl.c    6 Aug 2010 00:46:26 -0000
@@ -397,6 +397,46 @@ ATF_TC_CLEANUP(cmd_load_props, tc)
        unload_cleanup("k_helper");
 }
 
+ATF_TC_WITH_CLEANUP(cmd_load_recurse);
+ATF_TC_HEAD(cmd_load_recurse, tc)
+{
+       atf_tc_set_md_var(tc, "descr", "Tests for the MODCTL_LOAD command, "
+           "with recursive module_load()");
+       atf_tc_set_md_var(tc, "require.user", "root");
+}
+ATF_TC_BODY(cmd_load_recurse, tc)
+{
+       prop_dictionary_t props;
+       char filename[MAXPATHLEN];
+
+       require_modular();
+
+       printf("Loading module with request to load another module\n");
+       props = prop_dictionary_create();
+       snprintf(filename, sizeof(filename), "%s/k_helper2/k_helper2.kmod",
+           atf_tc_get_config_var(tc, "srcdir"));
+       prop_dictionary_set(props, "prop_recurse",
+           prop_string_create_cstring(filename));
+       load(props, true, "%s/k_helper/k_helper.kmod",
+           atf_tc_get_config_var(tc, "srcdir"));
+       {
+               int ok;
+               ATF_CHECK(get_sysctl("vendor.k_helper.prop_int_load",
+                   &ok, sizeof(ok)));
+               ATF_CHECK(ok == 0);
+               ATF_CHECK(get_sysctl("vendor.k_helper2.present",
+                   &ok, sizeof(ok)));
+               ATF_CHECK(ok);
+       }
+       unload("k_helper", true);
+       unload("k_helper2", true);
+}
+ATF_TC_CLEANUP(cmd_load_recurse, tc)
+{
+       unload_cleanup("k_helper");
+       unload_cleanup("k_helper2");
+}
+
 ATF_TC_WITH_CLEANUP(cmd_stat);
 ATF_TC_HEAD(cmd_stat, tc)
 {
@@ -467,6 +507,7 @@ ATF_TP_ADD_TCS(tp)
        ATF_TP_ADD_TC(tp, cmd_load);
        ATF_TP_ADD_TC(tp, cmd_load_props);
        ATF_TP_ADD_TC(tp, cmd_stat);
+       ATF_TP_ADD_TC(tp, cmd_load_recurse);
        ATF_TP_ADD_TC(tp, cmd_unload);
 
        return atf_no_error();
Index: tests/modules/k_helper/k_helper.c
===================================================================
RCS file: /cvsroot/src/tests/modules/k_helper/k_helper.c,v
retrieving revision 1.3
diff -u -p -r1.3 k_helper.c
--- tests/modules/k_helper/k_helper.c   28 Apr 2008 20:24:12 -0000      1.3
+++ tests/modules/k_helper/k_helper.c   6 Aug 2010 00:46:27 -0000
@@ -50,7 +50,8 @@ static int present = 1;
 static int prop_str_ok;
 static char prop_str_val[128];
 static int prop_int_ok;
-static int prop_int_val;
+static int64_t prop_int_val;
+static int prop_int_load;
 
 #define K_HELPER 0x12345678
 #define K_HELPER_PRESENT 0
@@ -58,6 +59,7 @@ static int prop_int_val;
 #define K_HELPER_PROP_STR_VAL 2
 #define K_HELPER_PROP_INT_OK 3
 #define K_HELPER_PROP_INT_VAL 4
+#define K_HELPER_PROP_INT_LOAD 5
 
 SYSCTL_SETUP(sysctl_k_helper_setup, "sysctl k_helper subtree setup")
 {
@@ -98,10 +100,17 @@ SYSCTL_SETUP(sysctl_k_helper_setup, "sys
 
        sysctl_createv(clog, 0, NULL, NULL,
                       CTLFLAG_PERMANENT,
-                      CTLTYPE_INT, "prop_int_val",
+                      CTLTYPE_QUAD, "prop_int_val",
                       SYSCTL_DESCR("String property's value"),
                       NULL, 0, &prop_int_val, 0,
                       CTL_VENDOR, K_HELPER, K_HELPER_PROP_INT_VAL, CTL_EOL);
+
+       sysctl_createv(clog, 0, NULL, NULL,
+                      CTLFLAG_PERMANENT,
+                      CTLTYPE_INT, "prop_int_load",
+                      SYSCTL_DESCR("Status of recursive modload"),
+                      NULL, 0, &prop_int_load, 0,
+                      CTL_VENDOR, K_HELPER, K_HELPER_PROP_INT_LOAD, CTL_EOL);
 }
 
 /* --------------------------------------------------------------------- */
@@ -143,6 +152,17 @@ k_helper_init(prop_dictionary_t props)
        if (!prop_int_ok)
                prop_int_val = -1;
 
+       p = prop_dictionary_get(props, "prop_recurse");
+       if (p != NULL && prop_object_type(p) == PROP_TYPE_STRING) {
+               const char *recurse_name = prop_string_cstring_nocopy(p);
+               if (recurse_name != NULL)
+                       prop_int_load = module_load(recurse_name,
+                           MODCTL_NO_PROP, NULL, MODULE_CLASS_ANY);
+               else
+                       prop_int_load = -1;
+       } else
+               prop_int_load = -2;
+
        sysctl_k_helper_setup(&clog);
 
        return 0;
--- /dev/null   2010-08-05 17:40:34.000000000 -0700
+++ tests/modules/k_helper2/k_helper2.c 2010-08-02 22:59:22.000000000 -0700
@@ -0,0 +1,115 @@
+/*     $NetBSD$ */
+/*
+ * Copyright (c) 2010 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND
+ * CONTRIBUTORS ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
+ * GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER
+ * IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN
+ * IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/cdefs.h>
+__KERNEL_RCSID(0, "$NetBSD$");
+
+#include <sys/param.h>
+#include <sys/kernel.h>
+#include <sys/module.h>
+#include <sys/sysctl.h>
+
+#include <prop/proplib.h>
+
+MODULE(MODULE_CLASS_MISC, k_helper2, NULL);
+
+/* --------------------------------------------------------------------- */
+/* Sysctl interface to query information about the module.               */
+/* --------------------------------------------------------------------- */
+
+/* TODO: Change the integer variables below that represent booleans to
+ * bools, once sysctl(8) supports CTLTYPE_BOOL nodes. */
+
+static struct sysctllog *clog;
+static int present = 1;
+
+#define K_HELPER2 0x23456781
+#define K_HELPER_PRESENT 0
+
+SYSCTL_SETUP(sysctl_k_helper2_setup, "sysctl k_helper subtree setup")
+{
+
+       sysctl_createv(clog, 0, NULL, NULL,
+                      CTLFLAG_PERMANENT,
+                      CTLTYPE_NODE, "k_helper2", NULL,
+                      NULL, 0, NULL, 0,
+                      CTL_VENDOR, K_HELPER2, CTL_EOL);
+
+       sysctl_createv(clog, 0, NULL, NULL,
+                      CTLFLAG_PERMANENT,
+                      CTLTYPE_INT, "present",
+                      SYSCTL_DESCR("Whether the module was loaded or not"),
+                      NULL, 0, &present, 0,
+                      CTL_VENDOR, K_HELPER2, K_HELPER_PRESENT, CTL_EOL);
+}
+
+/* --------------------------------------------------------------------- */
+/* Module management.                                                    */
+/* --------------------------------------------------------------------- */
+
+static
+int
+k_helper2_init(prop_dictionary_t props)
+{
+       sysctl_k_helper2_setup(&clog);
+
+       return 0;
+}
+
+static
+int
+k_helper2_fini(void *arg)
+{
+
+       sysctl_teardown(&clog);
+
+       return 0;
+}
+
+static
+int
+k_helper2_modcmd(modcmd_t cmd, void *arg)
+{
+       int ret;
+
+       switch (cmd) {
+       case MODULE_CMD_INIT:
+               ret = k_helper2_init(arg);
+               break;
+
+       case MODULE_CMD_FINI:
+               ret = k_helper2_fini(arg);
+               break;
+
+       case MODULE_CMD_STAT:
+       default:
+               ret = ENOTTY;
+       }
+
+       return ret;
+}


Home | Main Index | Thread Index | Old Index