2002-11-21 15:32:48

by Adam J. Richter

[permalink] [raw]
Subject: Patch?: module-init-tools/modprobe.c - use modules.dep

The following patch changes modprobe in module-init-tools-0.8
to use modules.dep.

Benefits:

- deletes a net of 594 lines of source code

- shrinks modprobe from 14kB to 10kB (stripped, dynamically linked),
which is useful for boot images

- should make modprobe as fast on systems with a lot of modules as
it was with the user level module loader,

- Restores the "include" command to the aliases file, which makes
it simpler to have separate files for automatically generated
aliases and user customizations.

- minor: eliminates ELF dependence from modprobe user level code


Drawbacks:

- It makes modprobe require that depmod had been run at some
point (although it isn't necessary to put depmod on a boot
image to use modprobe; you just need it when you want to add
a module that you want modprobe to know about). The current
depmod implementation is bigger than 594 lines of code, but
also generates hardware device tables, so systems that do
hardware autoconfiguration this way currently need depmod anyhow.

- I have not tested these changes much, because the in-kernel
module loader reports "memory allocation failure" for many
modules that I try to load. This does not appear to be
related to my changes.

- I do not currently see a positive balance of real benefits
to putting the module linker in unswappable kernel memory.
If you're running a system with a user level module loader,
you are probably better off staying with that.

Note to lmkl readers: this patch is again
module-init-tools-0.8.dwmw2, which is a modification done by David
Woodhouse of module-init-tools-0.7. It is not an official release,
and it requires a kernel patch which changes the system call interface
for loading modules (to pass the module name). I am posting a patch
against it instead of 0.7 because I don't want people applying this
patch and then breaking their systems due to the interface change. I
am also attaching David's patches to this message (after checking with
him by email), but please do not refer to David's changes or mine as
releases of module-init-tools, as they both depend on David's kernel
changes, which may or may not be integrated into Linus's future
releases.

--
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."


Attachments:
(No filename) (2.46 kB)
mod-init-tools.diff (30.55 kB)
dwmw2-module-init-tools.patch (2.24 kB)
dwmw2-kernel.patch (5.32 kB)
Download all attachments

2002-11-25 05:45:50

by Rusty Russell

[permalink] [raw]
Subject: Re: Patch?: module-init-tools/modprobe.c - use modules.dep

In message <[email protected]> you write:
>
> --xHFwDpU9dbj6ez1V
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
>
> The following patch changes modprobe in module-init-tools-0.8
> to use modules.dep.
>
> Benefits:
>
> - deletes a net of 594 lines of source code
>
> - shrinks modprobe from 14kB to 10kB (stripped, dynamically linked),
> which is useful for boot images
>
> - should make modprobe as fast on systems with a lot of modules as
> it was with the user level module loader,
>
> - Restores the "include" command to the aliases file, which makes
> it simpler to have separate files for automatically generated
> aliases and user customizations.
>
> - minor: eliminates ELF dependence from modprobe user level code

Hmm, I like it. But I prefer to pull the depmod code into the source
too, to keep it all under one roof.

The ELF dependence will go back in eventually, but that's trivial.

Hmm, Adam, do you want to reverse positions and become
module-init-tools maintainer? I'll send patches to you, instead of
vice versa. I'll release a 0.8 with the patches I have so far, then
hand it over if you want.

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

2002-11-25 19:10:37

by Adam J. Richter

[permalink] [raw]
Subject: Re: Patch?: module-init-tools/modprobe.c - use modules.dep

Rusty Russell wrote:
>In message <[email protected]> you [Adam Richter] write:
>>
>> --xHFwDpU9dbj6ez1V
>> Content-Type: text/plain; charset=us-ascii
>> Content-Disposition: inline
>>
>> The following patch changes modprobe in module-init-tools-0.8
>> to use modules.dep.
>>
>> Benefits:
>>
>> - deletes a net of 594 lines of source code
>>
>> - shrinks modprobe from 14kB to 10kB (stripped, dynamically linked),
>> which is useful for boot images
>>
>> - should make modprobe as fast on systems with a lot of modules as
>> it was with the user level module loader,
>>
>> - Restores the "include" command to the aliases file, which makes
>> it simpler to have separate files for automatically generated
>> aliases and user customizations.
>>
>> - minor: eliminates ELF dependence from modprobe user level code

>Hmm, I like it. But I prefer to pull the depmod code into the source
>too, to keep it all under one roof.

I have been thinking about splitting depmod into two programs:
the program as originally designed that generates modules.dep and one
that generates hardware support files. The latter could be
distributed in the Linux kernel tree and perhaps installed in
/lib/modules/<version>/bin/ to make it easy to change support table
formats as needed.

>The ELF dependence will go back in eventually, but that's trivial.

I'm guessing this is for symbols. If it's for something other
reason, I'd be curious to know it.

>Hmm, Adam, do you want to reverse positions and become
>module-init-tools maintainer? I'll send patches to you, instead of
>vice versa. I'll release a 0.8 with the patches I have so far, then
>hand it over if you want.

>Thoughts?
>Rusty.

I'm honored by the offer, but I have not seen any convincing
accounting of real benefits and costs that shows that it is a win to
have the module loader in kernel memory. I might be interested in
maintaining a small modutils that could be compiled to support either
the in-kernel module load or a user level method (or both) so as to
avoid unnecessary differences between the user level and in-kernel
methods, given that the code that is specific to the kernel module
loader would be small.

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."

2002-11-25 23:37:27

by Rusty Russell

[permalink] [raw]
Subject: Re: Patch?: module-init-tools/modprobe.c - use modules.dep

In message <[email protected]> you write:
> Rusty Russell wrote:
> >Hmm, I like it. But I prefer to pull the depmod code into the source
> >too, to keep it all under one roof.
>
> I have been thinking about splitting depmod into two programs:
> the program as originally designed that generates modules.dep and one
> that generates hardware support files. The latter could be
> distributed in the Linux kernel tree and perhaps installed in
> /lib/modules/<version>/bin/ to make it easy to change support table
> formats as needed.

Makes sense, but the plan was to migrate to using module aliases
anyway (you took out module alias support in your patch, though) 8(.

The modules are postprocessed on built to convert the device tables
into a series of aliases, eg. "usb:v0506p4601dl*dh*dc*dsc*dp*ic*isc*ip*"
for drivers/usb/net/pegasus.o. Then /sbin/hotplug just goes "modprobe
usb:v0506p4601dl01dh01dc01dsc01dp01ic01isc01ip01" or whatever, and voila.

The alias system also allows a driver to alias to an older driver, eg:

/* We can be used in place of the older driver if it isn't present */
MODULE_ALIAS("foo2000");

I've put a FIXME:, under your scheme the alias information from the
modules themselves needs to be extracted by depmod.

> >The ELF dependence will go back in eventually, but that's trivial.
>
> I'm guessing this is for symbols. If it's for something other
> reason, I'd be curious to know it.

--name support. It's a hack, but it's 20 lines in total.

> >Hmm, Adam, do you want to reverse positions and become
> >module-init-tools maintainer? I'll send patches to you, instead of
> >vice versa. I'll release a 0.8 with the patches I have so far, then
> >hand it over if you want.
>
> >Thoughts?
> >Rusty.
>
> I'm honored by the offer, but I have not seen any convincing
> accounting of real benefits and costs that shows that it is a win to
> have the module loader in kernel memory.

Well, the linecount comes out as a wash (it's slightly bigger because
of the hoops I jump through to avoid a spinlock on module refcount
acquisition, but that's orthogonal). Some archs lose 100 lines, some
gain 400 lines. The win comes from the cleanliness of two syscalls:
one to add, one to remove. For userspace loading you have one to
allocate, one to insert, one to query so you know how to link. Turns
out to be more complicated than just doing the damn linking yourself.

Frankly, linking just *isn't* that hard, especially when you're doing
it on your own architecture (vs. 32-bit userspace handling both 32-bit
and 64-bit kernelspaces). With RTH's "make it a shared object" patch,
it becomes even more trivial.

But the flexibility! By having a real interface, insmod doesn't need
to know anything about the module (modprobe still does, but even that
is very limited). Shrinking insmod to 20 lines and putting it in
busybox is nice, but being able to change the way parameters are
parsed, being able to switch reference count schemes, alter
initialization or shutdown methods, rewrite module versioning to be
sane, and otherwise tweak the kernel internals without breaking
userspace is a huge win.

But let's ignore my ideas, and look at three things which have been
suggested to me by other people since this patch went in. Ted Ts'o's
digital signatures on modules. Obviously much simpler in kernelspace.
The second is Keith Owens' NUMA text replication. Now such a change
is entirely up to the architecture (no modutils upgrade, sure, it will
almost certainly break oprofile on them though for kernel hackers).
The third is David Woodhouse's "multiple init for modules". There are
some fundamental questions (each initfn must have a matching exitfn in
case a later one fails), but this change wouldn't break userspace
either.

Even if the code had added 500 lines to the kernel, I'd say it was a
win.

> I might be interested in
> maintaining a small modutils that could be compiled to support either
> the in-kernel module load or a user level method (or both) so as to
> avoid unnecessary differences between the user level and in-kernel
> methods, given that the code that is specific to the kernel module
> loader would be small.

Sure: it'd definitely be worth distributing them together rather than
the horrible install hack at the moment (there's a RPM which already
does this).

I'd prefer a static parser which turns modules.conf into modprobe.conf
rather than reimplementing modules.conf (config files which are so
complex they need a "hobbled mode" in case they are called from
untrusted context are in trouble already). My plans were:

1) Extend alias to be:

alias foo bar [and|or baz]...

Aliases would continue to insist that they resolve where defined (to
avoid loops).

2) Implement "options" of course, which would stack (in case you
attach modules to an alias), and

3) Implement "install" (to allow arbitrary stuff like pre and post,
weirdass conditional stuff, etc).

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

2002-11-26 01:26:16

by Rusty Russell

[permalink] [raw]
Subject: Re: Patch?: module-init-tools/modprobe.c - use modules.dep

In message <[email protected]> you write:
> Rusty Russell wrote:
> >(you took out module alias support in your patch, though) 8(.
>
> Did not. See the routine get_alias() in my version.

No, you took out the part that reads the aliases from the module
itself (.modalias section).

> However, my version does not support:
>
> wildcard aliases,
> an alias that expands to multiple targets, or
> an alias that expands to an alias.
>
> It should only take a few lines to fix the first two.

See my wishlist for the second one (or/and support): this is a request
I got from someone.

> As for last, I'm not sure how useful aliasing to an alias really is.

Don't know. Combinations of and/or aliases would be easiest with
this, though.

> It wouldn't be the Manhattan Project to add it, but I'd rather not
> add a feature if it has no real use.

Good man!

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

2002-11-26 18:57:03

by Adam J. Richter

[permalink] [raw]
Subject: Re: Patch?: module-init-tools/modprobe.c - use modules.dep

On Tue, 26 Nov 2002 10:34:30 +1100, Rusty Russell wrote:
>In message <[email protected]> you write:
[...]
>> >The ELF dependence will go back in eventually, but that's trivial.
>>
>> I'm guessing this is for symbols. If it's for something other
>> reason, I'd be curious to know it.
>
>--name support. It's a hack, but it's 20 lines in total.

Why not derive the default name from the file name?

By the way, I have sometimes wanted to be able to load
multiple copies of the same module with different names, usually
little debugging helpers with different command line options.


>> [...] I have not seen any convincing
>> accounting of real benefits and costs that shows that it is a win to
>> have the module loader in kernel memory.
>
>Well, the linecount comes out as a wash (it's slightly bigger because
>of the hoops I jump through to avoid a spinlock on module refcount
>acquisition, but that's orthogonal). Some archs lose 100 lines, some
>gain 400 lines. The win comes from the cleanliness of two syscalls:
>one to add, one to remove. For userspace loading you have one to
>allocate, one to insert, one to query so you know how to link. Turns
>out to be more complicated than just doing the damn linking yourself.

Sometimes a larger total code size to achieve smaller kernel
code size is a worthwhile trade-off because the userland code is not
always present, is swappable, is more customizable without rebooting
the system, etc. Whether that is the case with the module loader
is the question that I'm trying to analyze.


>Frankly, linking just *isn't* that hard, especially when you're doing
>it on your own architecture (vs. 32-bit userspace handling both 32-bit
>and 64-bit kernelspaces).

I think that Roman's approach of having a module-init.o
to initialize the struct module would reduce or eliminate this.

>With RTH's "make it a shared object" patch,
>it becomes even more trivial.

rth's patch prevents allocating sections separately, so you
won't be able to kmalloc them as much, using TLB entries unnecessarily,
and using memory less efficiently in the case where the non-init
sections would fit in a single area small enough to be kmalloc'ed.
It's a modest cost, but it's something for the score card.



>But the flexibility! By having a real interface, insmod doesn't need
>to know anything about the module (modprobe still does, but even that
>is very limited).

Roman's module-init.o reduces or eliminates this difference.

>Shrinking insmod to 20 lines and putting it in
>busybox is nice, but being able to change the way parameters are
>parsed,

I think the module parameters should be passed as an
argument to the init_module/insmod system call in either case.

>being able to switch reference count schemes, alter
>initialization or shutdown methods, rewrite module versioning to be
>sane, and otherwise tweak the kernel internals without breaking
>userspace is a huge win.

For some changes, yes, but for some many other changes, a
scheme that uses Roman's module-init.o might not need any insmod
changes. And, for an insmod as simple as Roman's mini-loader,
we could ship it with the kernel tree and install it in
/lib/modules/<version>/bin/.


>But let's ignore my ideas, and look at three things which have been
>suggested to me by other people since this patch went in. Ted Ts'o's
>digital signatures on modules. Obviously much simpler in kernelspace.

No. You're much more likely to have random crypto software in
user land. User level authentication code can allocate memory more
freely, lookup up supporting certificates in external files, do
more elaborate more elaborate error handling like popping up a
dialog to say "The certificate for this module has expired.
Install it anyway?", or attempt to log the problem in
detail to a security server.

>The second is Keith Owens' NUMA text replication. Now such a change
>is entirely up to the architecture (no modutils upgrade, sure, it will
>almost certainly break oprofile on them though for kernel hackers).

Yes. By the way, I assume that we're talking about the
read-only sections of a module being mapped to different physical
pages but having the same virtual addresses across all processors, by
the way. User level insmod would need to be changed to load the
read-only sections separately.


>The third is David Woodhouse's "multiple init for modules". There are
>some fundamental questions (each initfn must have a matching exitfn in
>case a later one fails), but this change wouldn't break userspace
>either.

With something like module-{init,end}.o, this should not
require a further change to insmod.

There are also a variety of changes which are easier with the
module loader in user land (saving ~100kB of unswappable space by
kicking the symbols out of the kernel, being able to load modules with
dependency loops). Also, it's a small incremental difference, but
lowering the minimum resource costs of CONFIG_MODULES means that a
standard binary Linux kernel link kit may be a slightly more appealing
option to gadget makers in comparison to Vxworks, Windows NTE, CE, in
terms of engineering risk (e.g., what if the one kernel person who is
working on someone's digital music player quits).

Anyhow, thank you very much for taking the time to explain
your case for kernel module loading. I'm still thinking about it.


Regarding module loading tools that could support both user level
and kernel module loading:

>Sure: it'd definitely be worth distributing them together rather than
>the horrible install hack at the moment (there's a RPM which already
>does this).

Good.

>I'd prefer a static parser which turns modules.conf into modprobe.conf
>rather than reimplementing modules.conf (config files which are so
>complex they need a "hobbled mode" in case they are called from
>untrusted context are in trouble already).

Could we just use modules.conf and not support certain
commands?

>My plans were:
>
>1) Extend alias to be:
>
> alias foo bar [and|or baz]...
>
> Aliases would continue to insist that they resolve where defined (to
> avoid loops).

As we discussed, I suspect that we can insist that the names on
the right hand size must be actual module names, not aliases, at least
until someone complains and identifies a real use for aliases to
aliases.

>2) Implement "options" of course, which would stack (in case you
> attach modules to an alias), and

There are usually a small number of "options" lines that
people want to add, and I think that external packages that install
kernel module might want to install and remove those lines, so it
might be better to use the filesystem as the database for this, by
having small files like /etc/modules/args/ipsec_tunnel.


>3) Implement "install" (to allow arbitrary stuff like pre and post,
> weirdass conditional stuff, etc).

Then why not just run the existing modutils version of
modprobe?

I've been thinking that perhaps we should eliminate
request_module() from the kernel and instead generate a "want"
hotplug event ("hotplug filesystem want ext3",
"hotplug devfs want /dev/discs/disc0/part3", "hotplug soundcore want oss")
and move some of this complexity from modprobe to hotplug. My
reason are:

1. While we usually want to load a module in response to
these events, we might not always want to, and but we still
might want the "pre and post, weirdass conditional stuff, etc."
For example "hotplug devfs want /dev/discs/disc0/part3" may
just invoke "partx -a /dev/discs/disc0/disc".

2. Walking a tree and loading a kernel module are probably best
done in C, while most of this other customization is
probably best done from shell scripts.

3. There is a known security issue that users might
be able to cause a "dangerous" module to be loaded by
doing things like "ifconfig name-of-a-dangerous-module".

4. Conceivably, we may want software packages that load a
set of hotplug functionality based on bus type, so it
would be helpful to use an interface that has the extra
argument for bus type as hotplug does.

Maybe enough as been broken already, and this idea could be
pursued incrementally later if it's worth it. Any comments would
be welcome.

Also, thanks again for explaining your case for kernel module
loading.


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."

2002-11-27 22:46:05

by Rusty Russell

[permalink] [raw]
Subject: Re: Patch?: module-init-tools/modprobe.c - use modules.dep

In message <[email protected]> you write:
> On Tue, 26 Nov 2002 10:34:30 +1100, Rusty Russell wrote:
> >In message <[email protected]> you write:
> [...]
> >> >The ELF dependence will go back in eventually, but that's trivial.
> >>
> >> I'm guessing this is for symbols. If it's for something other
> >> reason, I'd be curious to know it.
> >
> >--name support. It's a hack, but it's 20 lines in total.
>
> Why not derive the default name from the file name?

Yes, it's a choice. But inside the kernel, modules know their own
name for module_parm() support. Changing it outside the kernel is
counterintuitive, and hence generally inadvisable.

> By the way, I have sometimes wanted to be able to load
> multiple copies of the same module with different names, usually
> little debugging helpers with different command line options.

Yes, this is exactly why this hack exists (dummy.o and ethertap.o kind
of rely on it).

> >Frankly, linking just *isn't* that hard, especially when you're doing
> >it on your own architecture (vs. 32-bit userspace handling both 32-bit
> >and 64-bit kernelspaces).
>
> I think that Roman's approach of having a module-init.o
> to initialize the struct module would reduce or eliminate this.

Yes, and, we could ship libc with the kernel and avoid having to keep
stable system call interfaces too. But there's benifit in not doing
it: we keep the kernel fairly standalone and it is good discipline for
us.

> >With RTH's "make it a shared object" patch,
> >it becomes even more trivial.
>
> rth's patch prevents allocating sections separately, so you
> won't be able to kmalloc them as much, using TLB entries unnecessarily,
> and using memory less efficiently in the case where the non-init
> sections would fit in a single area small enough to be kmalloc'ed.
> It's a modest cost, but it's something for the score card.

Even in total, most modules are fairly small, so I don't think it's a
big issue really. Maybe.

> >But let's ignore my ideas, and look at three things which have been
> >suggested to me by other people since this patch went in. Ted Ts'o's
> >digital signatures on modules. Obviously much simpler in kernelspace.
>
> No. You're much more likely to have random crypto software in
> user land.

Yes, but I believe the problem is that the kernel has to authenticate
it somehow.

> There are also a variety of changes which are easier with the
> module loader in user land (saving ~100kB of unswappable space by
> kicking the symbols out of the kernel, being able to load modules with
> dependency loops).

The first one is definitely a valid point, which I hadn't considered
before. Of course, compression takes it down to 14k, which implies we
should at least be doing something with it 8(

> >I'd prefer a static parser which turns modules.conf into modprobe.conf
> >rather than reimplementing modules.conf (config files which are so
> >complex they need a "hobbled mode" in case they are called from
> >untrusted context are in trouble already).
>
> Could we just use modules.conf and not support certain
> commands?

Hmm, I'd prefer to convert.

>
> >My plans were:
> >
> >1) Extend alias to be:
> >
> > alias foo bar [and|or baz]...
> >
> > Aliases would continue to insist that they resolve where defined (to
> > avoid loops).
>
> As we discussed, I suspect that we can insist that the names on
> the right hand size must be actual module names, not aliases, at least
> until someone complains and identifies a real use for aliases to
> aliases.

Sure.

> >2) Implement "options" of course, which would stack (in case you
> > attach modules to an alias), and
>
> There are usually a small number of "options" lines that
> people want to add, and I think that external packages that install
> kernel module might want to install and remove those lines, so it
> might be better to use the filesystem as the database for this, by
> having small files like /etc/modules/args/ipsec_tunnel.

Hmm, Debian actually compiles up modules.conf at the moment as it is:
it's probably better to leave this to the distros anyway.

> >3) Implement "install" (to allow arbitrary stuff like pre and post,
> > weirdass conditional stuff, etc).
>
> Then why not just run the existing modutils version of
> modprobe?

Because three commands should do everything we need? Because it's
infinitely simpler? But mainly because I *want* people to customize
modprobe for their own uses (that's why /proc/sys/kernel/modprobe
exists), and simplicity is good.

> I've been thinking that perhaps we should eliminate
> request_module() from the kernel and instead generate a "want"

Hmm, I'll have to take time to think about it. One kernel change at a
time, I think 8)

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

2002-11-27 23:09:54

by Keith Owens

[permalink] [raw]
Subject: Re: Patch?: module-init-tools/modprobe.c - use modules.dep

On Wed, 27 Nov 2002 16:02:08 +1100,
Rusty Russell <[email protected]> wrote:
>In message <[email protected]> you write:
>> >I'd prefer a static parser which turns modules.conf into modprobe.conf
>> >rather than reimplementing modules.conf (config files which are so
>> >complex they need a "hobbled mode" in case they are called from
>> >untrusted context are in trouble already).
>>
>> Could we just use modules.conf and not support certain
>> commands?
>
>Hmm, I'd prefer to convert.

I hope you are going to check with everybody using complex modules.conf
files before you remove all the facilities. I know that there are
people who rely on being able to run commands in modules.conf to suit
their system.