Denys Vlasenko wrote:
>> - other for the speed up stuff, for trunk
> 
>> The speed up probably increases size a bit, but I think it's worth it.
>> Comments, appreciated.
> 
> Second patch looks interesting too, but does not apply cleanly to trunk
> (and does not apply to 1.13.0). Which revision is it against?
> (If you can please rediff against current svn).

It was against relatively recent trunk. Apparently you tweaked some of
the modprobe error message not too long ago, which resulted in the
conflicts.

Attached is a rebased patch. Did some trivial tests.

Summary of patch:
- when using "modprobe -a" to probe multiple modules, it reads the
  config files only once (it's a lot faster than module-init-tools
  when probing lots of modules at once)
- it does not load symbols or aliases files at all if they are not
  needed (speeds up single modprobe calls with literal module name
  quite a bit)

This improved my embedded box's bootup time by several seconds.

- Timo
Index: modutils/modprobe.c
===================================================================
--- modutils/modprobe.c	(revision 25539)
+++ modutils/modprobe.c	(working copy)
@@ -13,23 +13,26 @@
 #include <sys/utsname.h>
 #include <fnmatch.h>
 
-struct modprobe_option {
+#define MODULE_FLAG_LOADED		0x0001
+#define MODULE_FLAG_NEED_DEPS		0x0002
+#define MODULE_FLAG_EXISTS		0x0004
+#define MODULE_FLAG_BLACKLISTED		0x0008
+
+struct module_entry {
 	char *module;
-	char *option;
+	const char *probe;
+	unsigned int flags;
+	llist_t *aliases;
+	llist_t *options;
+	llist_t *deps;
 };
 
 struct modprobe_conf {
-	char probename[MODULE_NAME_LEN];
+	llist_t *db;
+	llist_t *probes;
 	llist_t *options;
-	llist_t *aliases;
-#if ENABLE_FEATURE_MODPROBE_BLACKLIST
-#define add_to_blacklist(conf, name) llist_add_to(&conf->blacklist, name)
-#define check_blacklist(conf, name) (llist_find(conf->blacklist, name) == NULL)
-	llist_t *blacklist;
-#else
-#define add_to_blacklist(conf, name) do {} while (0)
-#define check_blacklist(conf, name) (1)
-#endif
+	int num_deps;
+	int num_symbols;
 };
 
 #define MODPROBE_OPTS	"acdlnrt:VC:" USE_FEATURE_MODPROBE_BLACKLIST("b")
@@ -46,21 +49,55 @@
 	MODPROBE_OPT_BLACKLIST	= (INSMOD_OPT_UNUSED << 9) * ENABLE_FEATURE_MODPROBE_BLACKLIST,
 };
 
-static llist_t *loaded;
-
 static int read_config(struct modprobe_conf *conf, const char *path);
 
-static void add_option(llist_t **all_opts, const char *module, const char *opts)
+static struct module_entry *_get_module(struct modprobe_conf *conf,
+					const char *module, int create)
 {
-	struct modprobe_option *o;
+	char modname[MODULE_NAME_LEN];
+	struct module_entry *e;
+	llist_t *l;
 
-	o = xzalloc(sizeof(struct modprobe_option));
-	if (module)
-		o->module = filename2modname(module, NULL);
-	o->option = xstrdup(opts);
-	llist_add_to(all_opts, o);
+	filename2modname(module, modname);
+	for (l = conf->db; l != NULL; l = l->link) {
+		e = (struct module_entry *) l->data;
+		if (strcmp(e->module, modname) == 0)
+			return e;
+	}
+	if (!create)
+		return NULL;
+
+	e = xzalloc(sizeof(*e));
+	e->module = xstrdup(modname);
+	llist_add_to(&conf->db, e);
+
+	return e;
 }
 
+static struct module_entry *get_module(struct modprobe_conf *conf,
+				       const char *module)
+{
+	return _get_module(conf, module, 1);
+}
+
+static struct module_entry *add_probe(struct modprobe_conf *conf,
+				      const char *name)
+{
+	struct module_entry *m;
+
+	m = get_module(conf, name);
+	m->probe = name;
+	m->flags |= MODULE_FLAG_NEED_DEPS;
+	llist_add_to(&conf->probes, m);
+
+	conf->num_deps++;
+	if (ENABLE_FEATURE_MODUTILS_SYMBOLS &&
+	    strncmp(m->module, "symbol:", 7) == 0)
+		conf->num_symbols++;;
+
+	return m;
+}
+
 static int FAST_FUNC config_file_action(const char *filename,
 					struct stat *statbuf UNUSED_PARAM,
 					void *userdata,
@@ -68,8 +105,10 @@
 {
 	struct modprobe_conf *conf = (struct modprobe_conf *) userdata;
 	RESERVE_CONFIG_BUFFER(modname, MODULE_NAME_LEN);
-	char *tokens[3];
+	char *tokens[3], *rmod;
 	parser_t *p;
+	llist_t *l;
+        struct module_entry *m;
 	int rc = TRUE;
 
 	if (bb_basename(filename)[0] == '.')
@@ -84,18 +123,36 @@
 	while (config_read(p, tokens, 3, 2, "# \t", PARSE_NORMAL)) {
 		if (strcmp(tokens[0], "alias") == 0) {
 			filename2modname(tokens[1], modname);
-			if (tokens[2] &&
-			    fnmatch(modname, conf->probename, 0) == 0)
-				llist_add_to(&conf->aliases,
-					filename2modname(tokens[2], NULL));
+			if (tokens[2] == NULL)
+				continue;
+
+			for (l = conf->probes; l != NULL; l = l->link) {
+				m = (struct module_entry *) l->data;
+				if (fnmatch(modname, m->module, 0) != 0)
+					continue;
+				rmod = filename2modname(tokens[2], NULL);
+				llist_add_to(&m->aliases, rmod);
+
+				if (m->flags & MODULE_FLAG_NEED_DEPS) {
+					m->flags &= ~MODULE_FLAG_NEED_DEPS;
+					conf->num_deps--;
+				}
+
+				m = get_module(conf, rmod);
+				m->flags |= MODULE_FLAG_NEED_DEPS;
+				conf->num_deps++;
+			}
 		} else if (strcmp(tokens[0], "options") == 0) {
-			if (tokens[2])
-				add_option(&conf->options, tokens[1], tokens[2]);
+			if (tokens[2] == NULL)
+				continue;
+			m = get_module(conf, tokens[1]);
+			llist_add_to(&m->options, xstrdup(tokens[2]));
 		} else if (strcmp(tokens[0], "include") == 0) {
 			read_config(conf, tokens[1]);
 		} else if (ENABLE_FEATURE_MODPROBE_BLACKLIST &&
 			   strcmp(tokens[0], "blacklist") == 0) {
-			add_to_blacklist(conf, xstrdup(tokens[1]));
+			get_module(conf, tokens[1])->flags
+				|= MODULE_FLAG_BLACKLISTED;
 		}
 	}
 	config_close(p);
@@ -110,104 +167,100 @@
 				config_file_action, NULL, conf, 1);
 }
 
-static char *gather_options(llist_t *first, const char *module, int usecmdline)
+static char *gather_options(char *opts, llist_t *o)
 {
-	struct modprobe_option *opt;
-	llist_t *n;
-	char *opts = xstrdup("");
-	int optlen = 0;
+	int optlen;
 
-	for (n = first; n != NULL; n = n->link) {
-		opt = (struct modprobe_option *) n->data;
+	if (opts == NULL)
+		opts = xstrdup("");
+	optlen = strlen(opts);
 
-		if (opt->module == NULL && !usecmdline)
-			continue;
-		if (opt->module != NULL && strcmp(opt->module, module) != 0)
-			continue;
-
-		opts = xrealloc(opts, optlen + strlen(opt->option) + 2);
-		optlen += sprintf(opts + optlen, "%s ", opt->option);
+	for (; o != NULL; o = o->link) {
+		opts = xrealloc(opts, optlen + strlen(o->data) + 2);
+		optlen += sprintf(opts + optlen, "%s ", o->data);
 	}
 	return opts;
 }
 
-static int do_modprobe(struct modprobe_conf *conf, const char *module)
+static int do_modprobe(struct modprobe_conf *conf, struct module_entry *m)
 {
-	RESERVE_CONFIG_BUFFER(modname, MODULE_NAME_LEN);
-	llist_t *deps = NULL;
-	char *fn, *options, *colon = NULL, *tokens[2];
-	parser_t *p;
+	struct module_entry *m2;
+	char *fn, *options;
 	int rc = -1;
 
-	p = config_open2(CONFIG_DEFAULT_DEPMOD_FILE, fopen_for_read);
-	/* Modprobe does not work at all without modprobe.dep,
-	 * even if the full module name is given. Returning error here
-	 * was making us later confuse user with this message:
-	 * "module /full/path/to/existing/file/module.ko not found".
-	 * It's better to die immediately, with good message: */
-	if (p == NULL)
-		bb_perror_msg_and_die("can't open '%s'", CONFIG_DEFAULT_DEPMOD_FILE);
+	if (!(m->flags & MODULE_FLAG_EXISTS))
+		return -ENOENT;
 
-	while (config_read(p, tokens, 2, 1, "# \t", PARSE_NORMAL)) {
-		colon = last_char_is(tokens[0], ':');
-		if (colon == NULL)
-			continue;
-
-		filename2modname(tokens[0], modname);
-		if (strcmp(modname, module) == 0)
-			break;
-
-		colon = NULL;
-	}
-	if (colon == NULL)
-		goto error_not_found;
-
-	colon[0] = '\0';
-	llist_add_to(&deps, xstrdup(tokens[0]));
-	if (tokens[1])
-		string_to_llist(tokens[1], &deps, " ");
-
 	if (!(option_mask32 & MODPROBE_OPT_REMOVE))
-		deps = llist_rev(deps);
+		m->deps = llist_rev(m->deps);
 
 	rc = 0;
-	while (deps && rc == 0) {
-		fn = llist_pop(&deps);
-		filename2modname(fn, modname);
+	while (m->deps && rc == 0) {
+		fn = llist_pop(&m->deps);
+		m2 = get_module(conf, fn);
 		if (option_mask32 & MODPROBE_OPT_REMOVE) {
-			if (bb_delete_module(modname, O_EXCL) != 0)
+			if (bb_delete_module(m->module, O_EXCL) != 0)
 				rc = errno;
-		} else if (llist_find(loaded, modname) == NULL) {
-			options = gather_options(conf->options, modname,
-						 strcmp(modname, module) == 0);
+		} else if (!(m2->flags & MODULE_FLAG_LOADED)) {
+			options = gather_options(NULL, m2->options);
+			if (m == m2)
+				options = gather_options(options,
+							 conf->options);
 			rc = bb_init_module(fn, options);
 			if (rc == 0)
-				llist_add_to(&loaded, xstrdup(modname));
+				m2->flags |= MODULE_FLAG_LOADED;
 			free(options);
 		}
 
 		free(fn);
 	}
 
- error_not_found:
-	config_close(p);
-
 	if (rc > 0 && !(option_mask32 & INSMOD_OPT_SILENT))
 		bb_error_msg("failed to %sload module %s: %s",
 			     (option_mask32 & MODPROBE_OPT_REMOVE) ? "un" : "",
-			     module, moderror(rc));
+			     m->probe ? m->probe : m->module, moderror(rc));
 
-	RELEASE_CONFIG_BUFFER(modname);
 	return rc;
 }
 
+static void load_modules_dep(struct modprobe_conf *conf)
+{
+	struct module_entry *m;
+	char *colon, *tokens[2];
+	parser_t *p;
+
+	p = config_open2(CONFIG_DEFAULT_DEPMOD_FILE, xfopen_for_read);
+	while (conf->num_deps &&
+	       config_read(p, tokens, 2, 1, "# \t", PARSE_NORMAL)) {
+		colon = last_char_is(tokens[0], ':');
+		if (colon == NULL)
+			continue;
+		*colon = 0;
+
+		m = _get_module(conf, tokens[0], 0);
+		if (m == NULL)
+			continue;
+
+		m->flags |= MODULE_FLAG_EXISTS;
+		if ((m->flags & MODULE_FLAG_NEED_DEPS) && (m->deps == NULL)) {
+			conf->num_deps--;
+			llist_add_to(&m->deps, xstrdup(tokens[0]));
+			if (tokens[1])
+				string_to_llist(tokens[1],
+						&m->deps, " ");
+		}
+	}
+	config_close(p);
+}
+
 int modprobe_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int modprobe_main(int argc UNUSED_PARAM, char **argv)
 {
 	struct utsname uts;
 	int rc;
 	unsigned opt;
-	llist_t *options = NULL;
+	struct modprobe_conf conf;
+	struct module_entry *m, *m2;
 
 	opt_complementary = "q-v:v-q";
 	opt = getopt32(argv, INSMOD_OPTS MODPROBE_OPTS INSMOD_ARGS,
@@ -230,11 +283,17 @@
 		}
 		return EXIT_SUCCESS;
 	}
-	if (!(opt & MODPROBE_OPT_INSERT_ALL)) {
-		/* If not -a, we have only one module name,
-		 * the rest of parameters are options */
-		add_option(&options, NULL, parse_cmdline_module_options(argv));
-		argv[1] = NULL;
+
+	memset(&conf, 0, sizeof(conf));
+	if (opt & MODPROBE_OPT_INSERT_ALL) {
+		/* Each argument is a module name */
+		for (; *argv != NULL; argv++)
+			add_probe(&conf, *argv);
+		conf.probes = llist_rev(conf.probes);
+	} else {
+		/* First argument is module name, rest are parameters */
+		m = add_probe(&conf, argv[0]);
+		llist_add_to(&conf.options, parse_cmdline_module_options(argv));
 	}
 
 	/* cache modules */
@@ -242,48 +301,41 @@
 		char *s;
 		parser_t *parser = config_open2("/proc/modules", fopen_for_read);
 		while (config_read(parser, &s, 1, 1, "# \t", PARSE_NORMAL & ~PARSE_GREEDY))
-			llist_add_to(&loaded, xstrdup(s));
+			get_module(&conf, s)->flags |= MODULE_FLAG_LOADED;
 		config_close(parser);
 	}
 
-	while (*argv) {
-		const char *arg = *argv;
-		struct modprobe_conf *conf;
+	read_config(&conf, "/etc/modprobe.conf");
+	read_config(&conf, "/etc/modprobe.d");
+	if (ENABLE_FEATURE_MODUTILS_SYMBOLS && conf.num_symbols)
+		read_config(&conf, "modules.symbols");
+	load_modules_dep(&conf);
+	if (ENABLE_FEATURE_MODUTILS_ALIAS && conf.num_deps) {
+		read_config(&conf, "modules.alias");
+		load_modules_dep(&conf);
+	}
 
-		conf = xzalloc(sizeof(*conf));
-		conf->options = options;
-		filename2modname(arg, conf->probename);
-		read_config(conf, "/etc/modprobe.conf");
-		read_config(conf, "/etc/modprobe.d");
-		if (ENABLE_FEATURE_MODUTILS_SYMBOLS
-		 && conf->aliases == NULL
-		 && strncmp(arg, "symbol:", 7) == 0
-		) {
-			read_config(conf, "modules.symbols");
-		}
-
-		if (ENABLE_FEATURE_MODUTILS_ALIAS && conf->aliases == NULL)
-			read_config(conf, "modules.alias");
-
-		if (conf->aliases == NULL) {
+	while ((m = llist_pop(&conf.probes)) != NULL) {
+		if (m->aliases == NULL) {
 			/* Try if module by literal name is found; literal
 			 * names are blacklist only if '-b' is given. */
 			if (!(opt & MODPROBE_OPT_BLACKLIST) ||
-			    check_blacklist(conf, conf->probename)) {
-				rc = do_modprobe(conf, conf->probename);
+			    !(m->flags & MODULE_FLAG_BLACKLISTED)) {
+				rc = do_modprobe(&conf, m);
 				if (rc < 0 && !(opt & INSMOD_OPT_SILENT))
-					bb_error_msg("module %s not found", arg);
+					bb_error_msg("module %s not found",
+						     m->probe);
 			}
 		} else {
 			/* Probe all aliases */
-			while (conf->aliases != NULL) {
-				char *realname = llist_pop(&conf->aliases);
-				if (check_blacklist(conf, realname))
-					do_modprobe(conf, realname);
+			while (m->aliases != NULL) {
+				char *realname = llist_pop(&m->aliases);
+				m2 = get_module(&conf, realname);
+				if (!(m2->flags & MODULE_FLAG_BLACKLISTED))
+					do_modprobe(&conf, m2);
 				free(realname);
 			}
 		}
-		argv++;
 	}
 
 	return EXIT_SUCCESS;
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to