tech-kern archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Locking for module_autoload() (was: Modules loading modules?)
On Sun, 1 Aug 2010, John Nemeth wrote:
I'm thinking the acquisition of module_lock should be pushed into
module_autoload() instead of having the caller acquire it for this very
reason. It makes it hard to change the way locking works in the
MODULAR code if you expect the caller to acquire the lock. I don't
know why it was done this way originally, or what the consequences (if
any) would be for making the change. Andrew, any thoughts on this?
Attached are diffs for moving the locking out of each caller and into
module_autoload() itself. Most of these changes are fairly trivial, but
a couple of them should be looked at more closely in case I've missed
any subtleties in the original code:
kern/kern_syscall.c
net/if_ppp.c
-------------------------------------------------------------------------
| 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: 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
--- dev/acpi/acpi.c 25 Jul 2010 12:54:46 -0000 1.208
+++ dev/acpi/acpi.c 2 Aug 2010 02:25:26 -0000
@@ -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: 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
--- dev/dm/dm_target.c 18 May 2010 15:10:41 -0000 1.13
+++ dev/dm/dm_target.c 2 Aug 2010 02:25:26 -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: 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
--- dev/mii/mii_physubr.c 25 Jul 2010 14:44:34 -0000 1.71
+++ dev/mii/mii_physubr.c 2 Aug 2010 02:25:27 -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: 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
--- dev/pci/pci_subr.c 25 Jul 2010 14:14:25 -0000 1.84
+++ dev/pci/pci_subr.c 2 Aug 2010 02:25:27 -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: 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
--- dev/scsipi/scsipiconf.c 25 Jul 2010 13:49:58 -0000 1.39
+++ dev/scsipi/scsipiconf.c 2 Aug 2010 02:25:27 -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: 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
--- dev/usb/usb_subr.c 27 Jul 2010 16:15:30 -0000 1.174
+++ dev/usb/usb_subr.c 2 Aug 2010 02:25:27 -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: 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
--- kern/kern_exec.c 7 Jul 2010 01:30:37 -0000 1.299
+++ kern/kern_exec.c 2 Aug 2010 02:25:27 -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: 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
--- kern/kern_module.c 26 Jun 2010 07:23:57 -0000 1.70
+++ kern/kern_module.c 2 Aug 2010 02:25:27 -0000
@@ -552,16 +552,18 @@ module_autoload(const char *filename, mo
{
int error;
- KASSERT(mutex_owned(&module_lock));
+ mutex_enter(&module_lock);
/* Nothing if the user has disabled it. */
if (!module_autoload_on) {
+ mutex_exit(&module_lock);
return EPERM;
}
/* Disallow path separators and magic symlinks. */
if (strchr(filename, '/') != NULL || strchr(filename, '@') != NULL ||
strchr(filename, '.') != NULL) {
+ mutex_exit(&module_lock);
return EPERM;
}
@@ -569,10 +571,12 @@ module_autoload(const char *filename, mo
error = kauth_authorize_system(kauth_cred_get(), KAUTH_SYSTEM_MODULE,
0, (void *)(uintptr_t)MODCTL_LOAD, (void *)(uintptr_t)1, NULL);
if (error != 0) {
+ mutex_exit(&module_lock);
return error;
}
- return module_do_load(filename, false, 0, NULL, NULL, class, true);
+ error = module_do_load(filename, false, 0, NULL, NULL, class, true);
+ mutex_exit(&module_lock);
}
/*
Index: 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
--- kern/kern_syscall.c 15 Apr 2010 20:46:08 -0000 1.4
+++ kern/kern_syscall.c 2 Aug 2010 02:25:27 -0000
@@ -208,6 +208,7 @@ sys_nomodule(struct lwp *l, const void *
mutex_exit(&module_lock);
return ERESTART;
}
+ mutex_exit(&module_lock);
/*
* Try to autoload a module to satisfy the request. If it
* works, retry the request.
@@ -224,11 +225,9 @@ sys_nomodule(struct lwp *l, const void *
sy->sy_call == sys_nomodule) {
break;
}
- mutex_exit(&module_lock);
return ERESTART;
}
}
- mutex_exit(&module_lock);
#endif /* MODULAR */
return sys_nosys(l, v, retval);
Index: 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
--- kern/sys_sig.c 1 Jul 2010 02:38:31 -0000 1.28
+++ kern/sys_sig.c 2 Aug 2010 02:25:27 -0000
@@ -353,7 +353,6 @@ sigaction1(struct lwp *l, int signum, co
if (nsa != NULL) {
if (__predict_false(vers < 2) &&
(p->p_lflag & PL_SIGCOMPAT) == 0) {
- mutex_enter(&module_lock);
if (sendsig_sigcontext_vec == NULL) {
(void)module_autoload("compat",
MODULE_CLASS_ANY);
@@ -374,7 +373,6 @@ sigaction1(struct lwp *l, int signum, co
*/
p->p_lflag |= PL_SIGCOMPAT;
mutex_exit(proc_lock);
- mutex_exit(&module_lock);
}
switch (vers) {
Index: kern/tty.c
===================================================================
RCS file: /cvsroot/src/sys/kern/tty.c,v
retrieving revision 1.237
diff -u -p -r1.237 tty.c
--- kern/tty.c 1 Jul 2010 02:38:31 -0000 1.237
+++ kern/tty.c 2 Aug 2010 02:25:28 -0000
@@ -1264,13 +1264,9 @@ 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);
+ if (ttcompatvec == NULL)
return EPASSTHROUGH;
- }
- mutex_exit(&module_lock);
}
error = (*ttcompatvec)(tp, cmd, data, flag, l);
rw_exit(&ttcompat_lock);
Index: 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
--- kern/uipc_accf.c 13 Mar 2010 23:03:39 -0000 1.11
+++ kern/uipc_accf.c 2 Aug 2010 02:25:28 -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: 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
--- kern/vfs_syscalls.c 30 Jun 2010 15:44:54 -0000 1.407
+++ kern/vfs_syscalls.c 2 Aug 2010 02:25:28 -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: 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
--- miscfs/specfs/spec_vnops.c 24 Jun 2010 13:03:17 -0000 1.130
+++ miscfs/specfs/spec_vnops.c 2 Aug 2010 02:25:28 -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);
Index: 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
--- net/if_ppp.c 5 Apr 2010 07:22:23 -0000 1.131
+++ net/if_ppp.c 2 Aug 2010 02:25:28 -0000
@@ -1820,7 +1820,6 @@ ppp_get_compressor(uint8_t ci)
if (cp != NULL)
return cp;
- mutex_enter(&module_lock);
mutex_enter(&ppp_compressors_mtx);
cp = ppp_get_compressor_noload(ci, true);
mutex_exit(&ppp_compressors_mtx);
@@ -1838,8 +1837,6 @@ ppp_get_compressor(uint8_t ci)
}
}
}
- mutex_exit(&module_lock);
-
return cp;
}
Home |
Main Index |
Thread Index |
Old Index