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."
Here is an updated version of my patch to
module-init-tools-0.6/modprobe.c to allow it to recursively search
subdirectories so that we can have module directory tree again.
This version I have actually run and it seems to work.
Unfortunately, about one out of four modules that I load
will get an error from the module loading system call (memory
allocation failure). I don't think that that is due to my
modprobe.c changes, but it could be.
This version of the patch adds 39 lines, alas, but I think the
ability to have a module directory hierarchy if you want to is worth it
(e.g., it is a lot easier to select a class of drivers for inclusion in
a boot image).
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."
--- module-init-tools-0.6/modprobe.c 2002-10-30 20:03:06.000000000 -0800
+++ module-init-tools-0.6.ajr/modprobe.c 2002-11-18 12:23:08.000000000 -0800
@@ -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)
@@ -114,7 +114,7 @@
static int ends_in(const char *name, const char *ext)
{
- unsigned int namelen, extlen, i;
+ unsigned int namelen, extlen;
/* Grab lengths */
namelen = strlen(name);
@@ -122,11 +122,8 @@
if (namelen < extlen) return 0;
- /* Look backwards */
- for (i = 0; i < extlen; i++)
- if (name[namelen - i] != ext[extlen - i]) return 0;
-
- return 1;
+ name += namelen - extlen;
+ return (strcmp(name, ext) == 0);
}
static void *load_section(int fd, unsigned long shdroff,
@@ -201,8 +198,8 @@
Elf_Ehdr hdr;
int fd;
- new = malloc(sizeof(*new) + strlen(dirname) + strlen(entry) + 1);
- sprintf(new->name, "%s%s", dirname, entry);
+ new = malloc(sizeof(*new) + strlen(dirname) + strlen(entry) + 2);
+ sprintf(new->name, "%s/%s", dirname, entry);
new->next = last;
new->order = 0;
fd = open(new->name, O_RDONLY);
@@ -227,14 +224,46 @@
return new;
}
-static struct module *load_all_exports(const char *revision)
+static char *find_mod(const char *parent, const char *child,
+ const char *target)
+{
+ struct dirent *dirent;
+ DIR *dir;
+ char path[strlen(parent) + strlen(child) + 2];
+ struct stat statbuf;
+ char *result = NULL;
+
+ sprintf(path, "%s/%s", parent, child);
+
+ if (strcmp(child, target) == 0 &&
+ stat(path, &statbuf) == 0 &&
+ S_ISREG(statbuf.st_mode))
+ return strdup(path);
+
+ dir = opendir(path);
+ if (dir) {
+ while ((dirent = readdir(dir)) != NULL) {
+ if (!strcmp(dirent->d_name, "..") ||
+ !strcmp(dirent->d_name, "."))
+ continue;
+
+ result = find_mod(path, dirent->d_name, target);
+ if (result)
+ break;
+ }
+ closedir(dir);
+ }
+ return result;
+}
+
+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 +271,10 @@
if (ends_in(dirent->d_name, MODULE_EXTENSION))
mods = add_module(dirname, dirent->d_name,
mods);
+ else if (strcmp(dirent->d_name, "..") &&
+ strcmp(dirent->d_name, "."))
+ mods = load_all_exports(dirname,
+ dirent->d_name, mods);
}
closedir(dir);
}
@@ -273,6 +306,7 @@
found = m;
/* If we didn't need to load it
already, we do now. */
+
found->order = order;
}
}
@@ -425,6 +459,7 @@
ret = syscall(__NR_init_module, map, len, "");
if (ret != 0) {
+ fprintf (stderr, "AJR init_module(map %p, len %d) failed, ret = %d.\n", map, len, ret);
if (dont_fail)
fatal("Error inserting %s: %s\n",name,moderror(errno));
else
@@ -439,13 +474,16 @@
unsigned int order;
struct module *modules;
struct module *i;
- char pathname[strlen(revision) + sizeof(MODULE_DIR) + strlen(modname)
- + sizeof(MODULE_EXTENSION)];
+ char *pathname;
+ char modname_ext[strlen(modname) + sizeof(MODULE_EXTENSION)];
- /* Create path name */
- sprintf(pathname, MODULE_DIR "%s" MODULE_EXTENSION, revision, modname);
+ sprintf (modname_ext, "%s%s", modname, MODULE_EXTENSION);
+ pathname = find_mod(MODULE_DIR, revision, modname_ext);
+ if (!pathname)
+ fatal("No module %s in %s/%s or subdirectories.\n",
+ modname_ext, MODULE_DIR, revision);
- 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. */
@@ -475,6 +513,7 @@
}
if (verbose) printf("Loading %s\n", pathname);
insmod(pathname, 1);
+ free(pathname);
}
static struct option options[] = { { "verbose", 0, NULL, 'v' },
In message <[email protected]> you write:
> 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.
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 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'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.
> 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.
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).
> Finally, I am skeptical of the benefits being worth the cost
> of putting an ELF relocator and symbol table parser in the kernel.
>From the (just posted) FAQ:
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.
On Tue, 19 Nov 2002 17:42:38 +1100,
Rusty Russell <[email protected]> wrote:
>In message <[email protected]> you write:
>> 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.
>
>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.
mkinitrd.
modprobe -l -t scsi.
Rusty Russell wrote:
>In message <[email protected]> you [Adam Richter] write:
>Hmm, I'm not entirely convinced. Moving back into subdirs introduces
>a namespace which isn't really there.
As you may already have realized, the phrase "introduces a
namespace which isn't really there" does not show something that you
would be prevented or enabled to do, nor does it argue any real
trade-off (performance, kernel footprint, lines of code to maintain,
etc.). In comparison, "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" does identify useful functionality that the
module tree enables.
>However, as you say, it's trivial.
Yes, I see it as this code complexity trade-off:
- Add ~50 lines to modprobe
vs.
- Need to release new ramdisk builders or have users
maintain custom tables to be able to select of modules
by functional description (for example, "USB controllers").
Users who do not do this may not appreciate it, but it's
an important capability for keeping boot image sizes under
control.
Also, from a system administratiion standpoint, you can more
readily tell if, say, you forgot to build sound or firewire entirely,
as opposed to just a few specific drivers.
>> 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'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.
I am not attached to a particular format. It's just that the
disruption of stoppping the device ID tables from working is
unnecessary. Integrating my change as temporary is fine.
>OTOH, the current method exposes
>kernel internals to external userspace programs, which is why I do it
>differently.
At the moment you don't do it, so I can't compare. I can only
point out that requiring a particular ELF format also exposes
internals in another way and that something needs to read the hardware
support advertisements from every module and generate tables
translating that hardware support to module names. Also bear in mind
that you many need to generate those tables for a kernel different
from the one you are currently running.
>> 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.
>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).
OK, once your new module scheme works well enough so that
I can complete a boot, I should be able to do "time modprobe foo"
pretty readily.
In the meantime, I think I can guess which approach will be
faster and which one will scale better as the number of modules
available increases (read ~6000 symbols from ~1000 modules and compute
transitive closure for a set of symbols or read and walk a tree from
one file).
Also, although depmod currently does not do this, it could be
changed to catch any reference loops, making it easier to ensure that
distributions do not ship with these problems, even for obscure
hardware.
The code for descending into directories could also go away,
given modules.dep.
Finally, since you seem to think that the trade-off of user
level simplicity at the expense of kernel complexity in this case is
worth it, you might like to consider that this change could reduce or
completely eliminate all references to ELF in modprobe and insmod.
depmod would still have such dependency, but it can be a bigger
program.
>> Finally, I am skeptical of the benefits being worth the cost
>> of putting an ELF relocator and symbol table parser in the kernel.
>From the (just posted) FAQ:
Great. I'll submit comments on it separately.
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."
Minor self-correction. Regarding using modules.dep, I wrote:
>[...] you might like to consider that this change could reduce or
>completely eliminate all references to ELF in modprobe and insmod.
^^^^^^^^^^^^^^^^^^^^^^
That should just read "in modprobe."
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."
On Tue, Nov 19, 2002 at 05:42:38PM +1100, Rusty Russell wrote:
> A: The total linking code is about 200 generic lines, 100
> x86-specific lines.
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:
> On Tue, Nov 19, 2002 at 05:42:38PM +1100, Rusty Russell wrote:
> > A: The total linking code is about 200 generic lines, 100
> > x86-specific lines.
>
> 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) {
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.