Patch: module-init-tools-0.6/modprobe.c - support subdirectories

The following untested patch adds subdirectory support to
module-init-tools-0.6/modprobe.c. I really need this for tools that I use for building initial ramdisks to do things like select all SCSI and IDE drivers without having to release a new version of the ramdisk maker every time a new SCSI or IDE driver is added. The patch is a net addition of four lines to modprobe.c.

I am sorry I was not able to test this change, but it would be
a lot of work for me to bring up a system without module device ID table support. I know your ChangeLog says that support is coming. I wonder if it would break your module utilities to leave the old macros device ID macros in place and let people run the existing depmod.

I also worry about about the latency of reading the
kernel symbols for all modules every time modprobe is called. If I want to have all of the modules installed, that's at least 1143 modules on x86, and this might be the standard installation of a Linux distribution. Here again, the existing depmod program could be used. What would be needed would be some code to make modprobe try modules.dep first if it exists.

Finally, I am skeptical of the benefits being worth the cost
of putting an ELF relocator and symbol table parser in the kernel. We don't need to do that just have try_mod_inc_use_count or to lock modules during a load that a new module depends on. I'd much apportioning this complexity to user space, where it does not consume unswappable memory, where it is easier to make changes, where there is slightly more protection against bugs. The user space code already has to do a little ELF parsing to figure out the dependecies. I would be interested in knowing what new thing I can do with my computer, or what performance gain or other real benefit all of this additional kernel code will buy.

Adam J. Richter     __     ______________   575 Oroville Road
[email protected]     \ /                  Milpitas, California 95035
+1 408 309-6081         | g g d r a s i l   United States of America
                         "Free Software For The Rest Of Us."

@@ -36,7 +36,7 @@
 
 #include "backwards_compat.c"
 
-#define MODULE_DIR "/lib/modules/%s/kernel/"
+#define MODULE_DIR "/lib/modules"
 #define MODULE_EXTENSION ".o"
 
 #define MODULE_NAME_LEN (64 - ELF_TYPE / 8)

@@ -227,14 +227,14 @@
 	return new;
 }
 
-static struct module *load_all_exports(const char *revision)
+static struct module *load_all_exports(const char *parent, const char *subdir,
+				       struct module *mods)
 {
-	struct module *mods = NULL;
 	struct dirent *dirent;
 	DIR *dir;
-	char dirname[strlen(revision) + sizeof(MODULE_DIR)];
+	char dirname[strlen(parent) + strlen(subdir) + 2];
 
-	sprintf(dirname, MODULE_DIR, revision);
+	sprintf(dirname, "%s/%s", parent, subdir);
 	dir = opendir(dirname);
 	if (dir) {
 		while ((dirent = readdir(dir)) != NULL) {

@@ -242,6 +242,9 @@
 			if (ends_in(dirent->d_name, MODULE_EXTENSION))
 				mods = add_module(dirname, dirent->d_name,
 						  mods);
+			else
+				mods = load_all_exports(dirname,
+							dirent->d_name, mods);
 		}
 		closedir(dir);
 	}

@@ -440,12 +443,13 @@
 	struct module *modules;
 	struct module *i;
 	char pathname[strlen(revision) + sizeof(MODULE_DIR) + strlen(modname)
-		     + sizeof(MODULE_EXTENSION)];
+		     + sizeof(MODULE_EXTENSION) + 1];
 
 	/* Create path name */
-	sprintf(pathname, MODULE_DIR "%s" MODULE_EXTENSION, revision, modname);
+	sprintf(pathname, "%s/%s/%s%s", MODULE_DIR,
+		revision, modname, MODULE_EXTENSION);
 
-	modules = load_all_exports(revision);
+	modules = load_all_exports(MODULE_DIR, revision, NULL);
 	order = 1;
 	if (get_deps(order, pathname, modules, verbose)) {
 		/* We need some other modules. */

In message <[email protected]> you write:

Hmm, I'm not entirely convinced. Moving back into subdirs introduces a namespace which isn't really there. However, as you say, it's trivial.

I'm actually tempted to push your patch for the moment, since it might be a week before device ID support is tested and polished enough to go in, with everything else I am doing. OTOH, the current method exposes kernel internals to external userspace programs, which is why I do it differently.

Yes, it's a fairly obvious optimization: I'd be very interesting in timing measurements. Another method is to use /var/cache/modprobe/uname-r.dep and use it if available and more recent than any module (removes the requirement for depmod).

Q: Doesn't putting linking code into the kernel just add bloat?
A: The total linking code is about 200 generic lines, 100
x86-specific lines. The ia64 linking code is about 500 lines (it's the most complex). Richard Henderson has a great suggestion for simplifying it furthur, which I'm implementing now. "insmod" is now a single portable system call, meaning insmod can be written in about 20 lines of code.

The previous code had to implement the two module loading system calls, the module querying system call, and the /proc/ksyms output, required a little more code than the current x86 linker.

Q: Why not just fix the old code?
A: Because having something so intimate with the kernel in userspace
greatly restricts what changes the kernel can make. Moving into the kernel means I have implemented modversions, typesafe extensible module parameters and kallsyms without altering userspace in any way. Future extensions won't have to worry about the version of modutils problem.

Hope that clarifies,
Rusty,
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

mkinitrd.
modprobe -l -t scsi.

Should we be bounds-checking the relocations? Maybe we are (I'm not familiar enough with this new module code yet.) I'm specifically thinking about the following:

/* This is where to make the change */
location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_offset
+ rel[i].r_offset;
/* This is the symbol it is referring to */
sym = (Elf32_Sym *)sechdrs[symindex].sh_offset
+ ELF32_R_SYM(rel[i].r_info);
if (!sym->st_value) {

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

In message <[email protected]> you write:

No, you didn't miss anything: I do minimal checking. I figured it was worth preventing mistakes (eg. insmod randomcrap.o), but in the end you're going to trust the module.

I'd say definitely if you know that it has happened (eg. binutils bugs or something).

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.