[PATCH] modutils: merge module_entry and module_info to common code

Timo Teräs timo.teras at iki.fi
Wed Jul 22 12:15:15 UTC 2015


This merges the in-memory module info structures of modprobe
and depmod. This allows sharing hashing by modulename code
improving depmod runtime with almost factor of 2x.

Signed-off-by: Timo Teräs <timo.teras at iki.fi>
---
 modutils/depmod.c   | 99 +++++++++++++++++++----------------------------------
 modutils/modprobe.c | 66 ++++-------------------------------
 modutils/modutils.c | 43 +++++++++++++++++++++++
 modutils/modutils.h | 29 ++++++++++++++++
 4 files changed, 114 insertions(+), 123 deletions(-)

diff --git a/modutils/depmod.c b/modutils/depmod.c
index 9713aef..b8699ba 100644
--- a/modutils/depmod.c
+++ b/modutils/depmod.c
@@ -14,6 +14,14 @@
 #include "modutils.h"
 #include <sys/utsname.h> /* uname() */
 
+struct globals {
+	module_db db;
+} FIX_ALIASING;
+#define G (*ptr_to_globals)
+#define INIT_G() do { \
+	SET_PTR_TO_GLOBALS(xzalloc(sizeof(G))); \
+} while (0)
+
 /*
  * Theory of operation:
  * - iterate over all modules and record their full path
@@ -21,21 +29,12 @@
  *   for each depends, look through our list of full paths and emit if found
  */
 
-typedef struct module_info {
-	struct module_info *next;
-	char *name, *modname;
-	llist_t *dependencies;
-	llist_t *aliases;
-	llist_t *symbols;
-	struct module_info *dnext, *dprev;
-} module_info;
-
 static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARAM,
-				void *data, int depth UNUSED_PARAM)
+				void *data UNUSED_PARAM, int depth UNUSED_PARAM)
 {
-	module_info **first = (module_info **) data;
 	char *image, *ptr;
-	module_info *info;
+	module_entry *e;
+
 	/* Arbitrary. Was sb->st_size, but that breaks .gz etc */
 	size_t len = (64*1024*1024 - 4096);
 
@@ -43,17 +42,10 @@ static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARA
 		return TRUE;
 
 	image = xmalloc_open_zipped_read_close(fname, &len);
-	info = xzalloc(sizeof(*info));
 
-	info->next = *first;
-	*first = info;
+	e = moddb_get(&G.db, bb_get_last_path_component_nostrip(fname), 1);
+	e->name = xstrdup(fname + 2); /* skip "./" */
 
-	info->dnext = info->dprev = info;
-	info->name = xstrdup(fname + 2); /* skip "./" */
-	info->modname = filename2modname(
-			bb_get_last_path_component_nostrip(fname),
-			NULL
-	);
 	for (ptr = image; ptr < image + len - 10; ptr++) {
 		if (is_prefixed_with(ptr, "depends=")) {
 			char *u;
@@ -62,11 +54,11 @@ static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARA
 			for (u = ptr; *u; u++)
 				if (*u == '-')
 					*u = '_';
-			ptr += string_to_llist(ptr, &info->dependencies, ",");
+			ptr += string_to_llist(ptr, &e->deps, ",");
 		} else if (ENABLE_FEATURE_MODUTILS_ALIAS
 		 && is_prefixed_with(ptr, "alias=")
 		) {
-			llist_add_to(&info->aliases, xstrdup(ptr + 6));
+			llist_add_to(&e->aliases, xstrdup(ptr + 6));
 			ptr += strlen(ptr);
 		} else if (ENABLE_FEATURE_MODUTILS_SYMBOLS
 		 && is_prefixed_with(ptr, "__ksymtab_")
@@ -77,7 +69,7 @@ static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARA
 			) {
 				continue;
 			}
-			llist_add_to(&info->symbols, xstrdup(ptr));
+			llist_add_to(&e->symbols, xstrdup(ptr));
 			ptr += strlen(ptr);
 		}
 	}
@@ -86,24 +78,13 @@ static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARA
 	return TRUE;
 }
 
-static module_info *find_module(module_info *modules, const char *modname)
-{
-	module_info *m;
-
-	for (m = modules; m != NULL; m = m->next)
-		if (strcmp(m->modname, modname) == 0)
-			return m;
-	return NULL;
-}
-
-static void order_dep_list(module_info *modules, module_info *start,
-			llist_t *add)
+static void order_dep_list(module_entry *start, llist_t *add)
 {
-	module_info *m;
+	module_entry *m;
 	llist_t *n;
 
 	for (n = add; n != NULL; n = n->link) {
-		m = find_module(modules, n->data);
+		m = moddb_get(&G.db, n->data, 0);
 		if (m == NULL)
 			continue;
 
@@ -118,7 +99,7 @@ static void order_dep_list(module_info *modules, module_info *start,
 		start->dprev = m;
 
 		/* recurse */
-		order_dep_list(modules, start, m->dependencies);
+		order_dep_list(start, m->deps);
 	}
 }
 
@@ -184,12 +165,15 @@ enum {
 int depmod_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int depmod_main(int argc UNUSED_PARAM, char **argv)
 {
-	module_info *modules, *m, *dep;
+	module_entry *m, *dep;
 	const char *moddir_base = "/";
 	char *moddir, *version;
 	struct utsname uts;
+	unsigned i;
 	int tmp;
 
+	INIT_G();
+
 	getopt32(argv, "aAb:eF:nruqC:", &moddir_base, NULL, NULL);
 	argv += optind;
 
@@ -211,23 +195,23 @@ int depmod_main(int argc UNUSED_PARAM, char **argv)
 		free(moddir);
 
 	/* Scan modules */
-	modules = NULL;
 	if (*argv) {
 		do {
-			parse_module(*argv, /*sb (unused):*/ NULL, &modules, 0);
+			parse_module(*argv, /*sb (unused):*/ NULL, NULL, 0);
 		} while (*++argv);
 	} else {
 		recursive_action(".", ACTION_RECURSE,
-				parse_module, NULL, &modules, 0);
+				parse_module, NULL, NULL, 0);
 	}
 
 	/* Generate dependency and alias files */
 	if (!(option_mask32 & OPT_n))
 		xfreopen_write(CONFIG_DEFAULT_DEPMOD_FILE, stdout);
-	for (m = modules; m != NULL; m = m->next) {
+
+	moddb_foreach_module(&G.db, m, i) {
 		printf("%s:", m->name);
 
-		order_dep_list(modules, m, m->dependencies);
+		order_dep_list(m, m->deps);
 		while (m->dnext != m) {
 			dep = m->dnext;
 			printf(" %s", dep->name);
@@ -243,10 +227,7 @@ int depmod_main(int argc UNUSED_PARAM, char **argv)
 #if ENABLE_FEATURE_MODUTILS_ALIAS
 	if (!(option_mask32 & OPT_n))
 		xfreopen_write("modules.alias", stdout);
-	for (m = modules; m != NULL; m = m->next) {
-		char modname[MODULE_NAME_LEN];
-		const char *fname = bb_basename(m->name);
-		filename2modname(fname, modname);
+	moddb_foreach_module(&G.db, m, i) {
 		while (m->aliases) {
 			/*
 			 * Last word used to be a basename
@@ -256,34 +237,24 @@ int depmod_main(int argc UNUSED_PARAM, char **argv)
 			 */
 			printf("alias %s %s\n",
 				(char*)llist_pop(&m->aliases),
-				modname);
+				m->modname);
 		}
 	}
 #endif
 #if ENABLE_FEATURE_MODUTILS_SYMBOLS
 	if (!(option_mask32 & OPT_n))
 		xfreopen_write("modules.symbols", stdout);
-	for (m = modules; m != NULL; m = m->next) {
-		char modname[MODULE_NAME_LEN];
-		const char *fname = bb_basename(m->name);
-		filename2modname(fname, modname);
+	moddb_foreach_module(&G.db, m, i) {
 		while (m->symbols) {
 			printf("alias symbol:%s %s\n",
 				(char*)llist_pop(&m->symbols),
-				modname);
+				m->modname);
 		}
 	}
 #endif
 
-	if (ENABLE_FEATURE_CLEAN_UP) {
-		while (modules) {
-			module_info *old = modules;
-			modules = modules->next;
-			free(old->name);
-			free(old->modname);
-			free(old);
-		}
-	}
+	if (ENABLE_FEATURE_CLEAN_UP)
+		moddb_free(&G.db);
 
 	return EXIT_SUCCESS;
 }
diff --git a/modutils/modprobe.c b/modutils/modprobe.c
index 996de40..8c8eb25 100644
--- a/modutils/modprobe.c
+++ b/modutils/modprobe.c
@@ -150,19 +150,6 @@ static const char modprobe_longopts[] ALIGN1 =
 #define MODULE_FLAG_FOUND_IN_MODDEP     0x0004
 #define MODULE_FLAG_BLACKLISTED         0x0008
 
-struct module_entry { /* I'll call it ME. */
-	unsigned flags;
-	char *modname; /* stripped of /path/, .ext and s/-/_/g */
-	const char *probed_name; /* verbatim as seen on cmdline */
-	char *options; /* options from config files */
-	llist_t *realnames; /* strings. if this module is an alias, */
-	/* real module name is one of these. */
-//Can there really be more than one? Example from real kernel?
-	llist_t *deps; /* strings. modules we depend on */
-};
-
-#define DB_HASH_SIZE 256
-
 struct globals {
 	llist_t *probes; /* MEs of module(s) requested on cmdline */
 	char *cmdline_mopts; /* module options from cmdline */
@@ -170,7 +157,7 @@ struct globals {
 	/* bool. "Did we have 'symbol:FOO' requested on cmdline?" */
 	smallint need_symbols;
 	struct utsname uts;
-	llist_t *db[DB_HASH_SIZE]; /* MEs of all modules ever seen (caching for speed) */
+	module_db db;
 } FIX_ALIASING;
 #define G (*ptr_to_globals)
 #define INIT_G() do { \
@@ -195,51 +182,9 @@ static char *gather_options_str(char *opts, const char *append)
 	return opts;
 }
 
-/* These three functions called many times, optimizing for speed.
- * Users reported minute-long delays when they runn iptables repeatedly
- * (iptables use modprobe to install needed kernel modules).
- */
-static struct module_entry *helper_get_module(const char *module, int create)
+static struct module_entry *get_or_add_modentry(const char *module)
 {
-	char modname[MODULE_NAME_LEN];
-	struct module_entry *e;
-	llist_t *l;
-	unsigned i;
-	unsigned hash;
-
-	filename2modname(module, modname);
-
-	hash = 0;
-	for (i = 0; modname[i]; i++)
-		hash = ((hash << 5) + hash) + modname[i];
-	hash %= DB_HASH_SIZE;
-
-	for (l = G.db[hash]; l; l = l->link) {
-		e = (struct module_entry *) l->data;
-		if (strcmp(e->modname, modname) == 0)
-			return e;
-	}
-	if (!create)
-		return NULL;
-
-	e = xzalloc(sizeof(*e));
-	e->modname = xstrdup(modname);
-	llist_add_to(&G.db[hash], e);
-
-	return e;
-}
-static ALWAYS_INLINE struct module_entry *get_or_add_modentry(const char *module)
-{
-	return helper_get_module(module, 1);
-}
-/* So far this function always gets a module pathname, never an alias name.
- * The crucial difference is that pathname needs dirname stripping,
- * while alias name must NOT do it!
- * Testcase where dirname stripping is likely to go wrong: "modprobe devname:snd/timer"
- */
-static ALWAYS_INLINE struct module_entry *get_modentry(const char *pathname)
-{
-	return helper_get_module(bb_get_last_path_component_nostrip(pathname), 0);
+	return moddb_get(&G.db, module, 1);
 }
 
 static void add_probe(const char *name)
@@ -505,7 +450,7 @@ static void load_modules_dep(void)
 			continue;
 		*colon = '\0';
 
-		m = get_modentry(tokens[0]);
+		m = moddb_get(&G.db, bb_get_last_path_component_nostrip(tokens[0]), 0);
 		if (m == NULL)
 			continue;
 
@@ -666,5 +611,8 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
 		} while (me->realnames != NULL);
 	}
 
+	if (ENABLE_FEATURE_CLEAN_UP)
+		moddb_free(&G.db);
+
 	return (rc != 0);
 }
diff --git a/modutils/modutils.c b/modutils/modutils.c
index 84300d9..ed26697 100644
--- a/modutils/modutils.c
+++ b/modutils/modutils.c
@@ -16,6 +16,49 @@ extern int delete_module(const char *module, unsigned int flags);
 # define delete_module(mod, flags) syscall(__NR_delete_module, mod, flags)
 #endif
 
+module_entry * FAST_FUNC moddb_get(module_db *db, const char *module, int create)
+{
+	char modname[MODULE_NAME_LEN];
+	struct module_entry *e;
+	unsigned i, hash;
+
+	filename2modname(module, modname);
+
+	hash = 0;
+	for (i = 0; modname[i]; i++)
+		hash = ((hash << 5) + hash) + modname[i];
+	hash %= MODULE_HASH_SIZE;
+
+	for (e = db->buckets[hash]; e; e = e->next)
+		if (strcmp(e->modname, modname) == 0)
+			return e;
+	if (!create)
+		return NULL;
+
+	e = xzalloc(sizeof(*e));
+	e->modname = xstrdup(modname);
+	e->next = db->buckets[hash];
+	db->buckets[hash] = e;
+	e->dnext = e->dprev = e;
+
+	return e;
+}
+
+void FAST_FUNC moddb_free(module_db *db)
+{
+	module_entry *e, *n;
+	unsigned i;
+
+	for (i = 0; i < MODULE_HASH_SIZE; i++) {
+		for (e = db->buckets[i]; e; e = n) {
+			n = e->next;
+			free(e->name);
+			free(e->modname);
+			free(e);
+		}
+	}
+}
+
 void FAST_FUNC replace(char *s, char what, char with)
 {
 	while (*s) {
diff --git a/modutils/modutils.h b/modutils/modutils.h
index 5f059c7..0c0cb46 100644
--- a/modutils/modutils.h
+++ b/modutils/modutils.h
@@ -16,6 +16,35 @@ PUSH_AND_SET_FUNCTION_VISIBILITY_TO_HIDDEN
 /* linux/include/linux/module.h has 64, but this is also used
  * internally for the maximum alias name length, which can be quite long */
 #define MODULE_NAME_LEN 256
+#define MODULE_HASH_SIZE 256
+
+typedef struct module_entry {
+	struct module_entry *next;
+	char *name, *modname;
+	llist_t *deps;
+	IF_MODPROBE(
+		llist_t *realnames;
+		unsigned flags;
+		const char *probed_name; /* verbatim as seen on cmdline */
+		char *options; /* options from config files */
+	)
+	IF_DEPMOD(
+		llist_t *aliases;
+		llist_t *symbols;
+		struct module_entry *dnext, *dprev;
+	)
+} module_entry;
+
+typedef struct module_db {
+	module_entry *buckets[MODULE_HASH_SIZE];
+} module_db;
+
+#define moddb_foreach_module(db, module, index) \
+	for ((index) = 0; (index) < MODULE_HASH_SIZE; (index)++) \
+		for (module = (db)->buckets[index]; module; module = module->next)
+
+module_entry *moddb_get(module_db *db, const char *s, int create) FAST_FUNC;
+void moddb_free(module_db *db) FAST_FUNC;
 
 void replace(char *s, char what, char with) FAST_FUNC;
 char *replace_underscores(char *s) FAST_FUNC;
-- 
2.4.5



More information about the busybox mailing list