2005-05-06 21:24:16

by Greg KH

[permalink] [raw]
Subject: [ANNOUNCE] hotplug-ng 002 release

After a very long delay, the 002 release of the hotplug-ng package
finally escaped from my box and can be found at:
kernel.org/pub/linux/utils/kernel/hotplug/hotplug-ng-002.tar.gz

The development tree has been converted over to a git archive, and can
be found at:
rsync://rsync.kernel.org/pub/scm/linux/hotplug/hotplug-ng.git
and can be browsed online at:
http://www.kernel.org/git/gitweb.cgi?p=linux%2Fhotplug%2Fhotplug-ng.git;a=log

Nothing major in this release, just lots of bugfixes from a bunch of
different people. Thanks to all of you who sent me fixes for the same
argc bug, I have been sufficiently chastised for such a stupid thing...

Full changelog can be found below, or browsed online at the link above.

So, where to next? After the 001 release announcement, the main thing
that has happened is that this project is pretty much obsolete
already...

Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
USB hotplug program can be written with a simple one line shell script:
modprobe $MODALIAS

So, with a few more kernel patches for the other subsystems (hint, I'll
gladly take them...) the other helper programs can too go away entirely.
That will leave us with only the main /sbin/hotplug multiplexor program
that any distro that uses udev is starting to abandon entirely.

Oh, and the upstream module-init-tools maintainer needs to accept that
patch one of these days...

If I've missed any patches for the code from anyone, my apologies, can
you please resend them against this latest tree?

thanks,

greg k-h

Summary of changes v001 to v002
============================================

Christian Borntraeger:
o fix segfault when no parameters are used for agents
o typo in make uninstall

Greg Kroah-Hartman:
o Add manpage to the install/uninstall rules
o delete klibc/klibc.spec, which is a generated file
o fix 'make release' to work properly with git
o update makefile to allow me to do a tarball interm release for someone
o fix up argc test in module_form.c
o make the release tarballs have writable files

Kay Sievers:
o rename LOG to USE_LOG
o cleanup logging.h and list.h
o libsysfs: new version
o remove old klibc_fixups
o klibc: version 1.0.3

Pozsar Balazs:
o Add ieee1394 support
o fix width of pci ids

Tobias Klauser:
o Manpage for hotplug-ng



2005-05-08 22:52:44

by Per Liden

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Fri, 6 May 2005, Greg KH wrote:

[...]
> Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> USB hotplug program can be written with a simple one line shell script:
> modprobe $MODALIAS

Nice, but why not just convert all this to a call to
request_module($MODALIAS)? Seems to me like the natural thing to do.

[...]
> Oh, and the upstream module-init-tools maintainer needs to accept that
> patch one of these days...

Where can this patch be found?

/Per

2005-05-09 03:58:10

by Rusty Russell

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Fri, 2005-05-06 at 14:22 -0700, Greg KH wrote:
> Oh, and the upstream module-init-tools maintainer needs to accept that
> patch one of these days...

??

Rusty.
--
A bad analogy is like a leaky screwdriver -- Richard Braakman

2005-05-09 21:14:04

by Per Svennerbrandt

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

* Per Liden ([email protected]) wrote:
> On Fri, 6 May 2005, Greg KH wrote:
>
> [...]
> > Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> > USB hotplug program can be written with a simple one line shell script:
> > modprobe $MODALIAS
>
> Nice, but why not just convert all this to a call to
> request_module($MODALIAS)? Seems to me like the natural thing to do.

I actually have a pretty hackish proof-of-consept patch that does
basicly that, and have been running it on my systems for the past five
months or so, if anybody's interested.

Along with it I also have a patch witch exports the module aliases for
PCI and USB devices through sysfs. With it the "coldplugging" of a
system (module wise) can be reduced to pretty much:

#!/bin/sh

for DEV in /sys/bus/{pci,usb}/devices/*; do
modprobe `cat $DEV/modalias`
done

(And I actually run exactly that on my laptop, and it works surpricingly
well. (Largly due to the fact that the usb-controller is always attached
below the pci-bus of course, but it really wouldn't take that much work
to make it do the right thing even without relying on any specific
ordering/topology))

With the above in place my system does all the module-loading that I
care about automaticly, and most importantly does so without relying
on an /etc/hotplug/ dir with everything and it's grandma in it (or at
least thousands of lines of shellscripting).

But since the request_modalias() thing seemed as such an obvious thing
to do I have been reluctant to submit it fearing that I must have missed
some fundamental flaw in it or you guys would have implemented it that
way a long time ago? (at least since Rusty rewrote the module
loader). Was I wrong*?

Greg, Rusty, what do you think?

/ Per Svennerbrandt

* I also actually had my kernel add MODALIAS to the hotplug enviroment
long before Roman submited his patch doing this, even calling it MODALIAS
from the very beginning! :) So obviously I've been wrong before...

2005-05-09 23:21:36

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Mon, May 09, 2005 at 01:57:14PM +1000, Rusty Russell wrote:
> On Fri, 2005-05-06 at 14:22 -0700, Greg KH wrote:
> > Oh, and the upstream module-init-tools maintainer needs to accept that
> > patch one of these days...
>
> ??

I've attached the original message sent to you and me below.

thanks,

greg k-h


Attachments:
(No filename) (310.00 B)
modprobe_load_all_aliases.mbox (7.81 kB)
Download all attachments

2005-05-09 23:27:29

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Mon, May 09, 2005 at 12:52:02AM +0200, Per Liden wrote:
> On Fri, 6 May 2005, Greg KH wrote:
>
> [...]
> > Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> > USB hotplug program can be written with a simple one line shell script:
> > modprobe $MODALIAS
>
> Nice, but why not just convert all this to a call to
> request_module($MODALIAS)? Seems to me like the natural thing to do.

Because that's not the only thing that the hotplug event causes to
happen. It's easier to have userspace decide what to do with this
instead.

> [...]
> > Oh, and the upstream module-init-tools maintainer needs to accept that
> > patch one of these days...
>
> Where can this patch be found?

Just sent it to this same thread.

thanks,

greg k-h

2005-05-10 09:29:27

by Rusty Russell

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Mon, 2005-05-09 at 16:21 -0700, Greg KH wrote:
> On Mon, May 09, 2005 at 01:57:14PM +1000, Rusty Russell wrote:
> > On Fri, 2005-05-06 at 14:22 -0700, Greg KH wrote:
> > > Oh, and the upstream module-init-tools maintainer needs to accept that
> > > patch one of these days...
> >
> > ??
>
> I've attached the original message sent to you and me below.

Yes. It broke into small pieces against the testsuite. I know that
Linux kernel types generally don't consider that a problem 8)

So I rewrote it yesterday, so now it passes the testsuite. I also added
a test. It's in 3.2-pre4: if there are no more requests/bugs in the
next couple of days, I'll make that 3.3.

Thanks for the prodding!
Rusty.
--
A bad analogy is like a leaky screwdriver -- Richard Braakman

2005-05-10 09:59:25

by Marco d'Itri

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On May 10, Rusty Russell <[email protected]> wrote:

> So I rewrote it yesterday, so now it passes the testsuite. I also added
> a test. It's in 3.2-pre4: if there are no more requests/bugs in the
> next couple of days, I'll make that 3.3.
My major request is support for /etc/hotplug/blacklist.d/ in modprobe:
now that the hotplug subsystem does not know anymore the name of the
module to be loaded, it's up to modprobe to check the system blacklist.
IME, without this feature users will not accept hotplug-ng.

--
ciao,
Marco


Attachments:
(No filename) (535.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2005-05-10 12:58:34

by Alexander E. Patrakov

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

Marco d'Itri wrote:

>On May 10, Rusty Russell <[email protected]> wrote:
>
>
>
>>So I rewrote it yesterday, so now it passes the testsuite. I also added
>>a test. It's in 3.2-pre4: if there are no more requests/bugs in the
>>next couple of days, I'll make that 3.3.
>>
>>
>My major request is support for /etc/hotplug/blacklist.d/ in modprobe:
>now that the hotplug subsystem does not know anymore the name of the
>module to be loaded, it's up to modprobe to check the system blacklist.
>IME, without this feature users will not accept hotplug-ng.
>
>
Why not this or something similar (e.g. I want to blacklist the xxx and
yyy modules)? (note, untested)

in /etc/modprobe.d/hotplug_blacklist:

install xxx /sbin/blacklisted xxx
install yyy /sbin/blacklisted yyy

In /sbin/blacklisted:

#!/bin/sh
if [ -z "$SEQNUM" ] ; then
/sbin/modprobe -i "$@"
else
exit 1
fi

Any other variable that is certainly present in hotplug events will also
work instead of $SEQNUM.

--
Alexander E. Patrakov

2005-05-10 17:24:59

by Marco d'Itri

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On May 10, "Alexander E. Patrakov" <[email protected]> wrote:

> Why not this or something similar (e.g. I want to blacklist the xxx and
> yyy modules)? (note, untested)
Because it's impossible to predict how it will interact with other
install and alias commands.

A less fundamental but still major problem is that this would be a
different API, and both users and packages have been aware of
/etc/hotplug/blacklist* for a long time now.

--
ciao,
Marco

2005-05-10 20:14:02

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, May 10, 2005 at 07:24:47PM +0200, Marco d'Itri wrote:
> On May 10, "Alexander E. Patrakov" <[email protected]> wrote:
>
> > Why not this or something similar (e.g. I want to blacklist the xxx and
> > yyy modules)? (note, untested)

Nice, I like it.

> Because it's impossible to predict how it will interact with other
> install and alias commands.

Then we will just have to find out :)

> A less fundamental but still major problem is that this would be a
> different API, and both users and packages have been aware of
> /etc/hotplug/blacklist* for a long time now.

And as /etc/hotplug/* is going away for hotplug-ng, I don't think this
is going to be an issue. Also, the blacklisting stuff should not be
that prevelant anymore...

thanks,

greg k-h

2005-05-10 20:28:39

by Lee Revell

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, 2005-05-10 at 13:13 -0700, Greg KH wrote:
> Also, the blacklisting stuff should not be
> that prevelant anymore...

It's quite often used by ALSA users who need to prevent hotplug from
loading the OSS modules. Is there a better way to do this?

Lee

2005-05-10 20:36:27

by Marco d'Itri

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On May 10, Greg KH <[email protected]> wrote:

> > > Why not this or something similar (e.g. I want to blacklist the xxx and
> > > yyy modules)? (note, untested)
> Nice, I like it.
But it does not work.

> > Because it's impossible to predict how it will interact with other
> > install and alias commands.
> Then we will just have to find out :)
It should be clear that it will interact badly with another install
commands, with one of them being ignored. This is not acceptable.

> > A less fundamental but still major problem is that this would be a
> > different API, and both users and packages have been aware of
> > /etc/hotplug/blacklist* for a long time now.
> And as /etc/hotplug/* is going away for hotplug-ng, I don't think this
> is going to be an issue. Also, the blacklisting stuff should not be
> that prevelant anymore...
It's a feature which I know my users and other maintainers need
(for duplicated drivers, OSS drivers, watchdog drivers, usb{mouse,kbd}
and so on) so it's a prerequisite for the successful packaging of
hotplug-ng.

--
ciao,
Marco


Attachments:
(No filename) (1.04 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2005-05-10 20:53:40

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, May 10, 2005 at 10:31:56PM +0200, Marco d'Itri wrote:
> On May 10, Greg KH <[email protected]> wrote:
>
> > > > Why not this or something similar (e.g. I want to blacklist the xxx and
> > > > yyy modules)? (note, untested)
> > Nice, I like it.
> But it does not work.

Why not?

> > > Because it's impossible to predict how it will interact with other
> > > install and alias commands.
> > Then we will just have to find out :)
> It should be clear that it will interact badly with another install
> commands, with one of them being ignored. This is not acceptable.

Why? Will they not all just be checked in order?

> > > A less fundamental but still major problem is that this would be a
> > > different API, and both users and packages have been aware of
> > > /etc/hotplug/blacklist* for a long time now.
> > And as /etc/hotplug/* is going away for hotplug-ng, I don't think this
> > is going to be an issue. Also, the blacklisting stuff should not be
> > that prevelant anymore...
> It's a feature which I know my users and other maintainers need
> (for duplicated drivers, OSS drivers, watchdog drivers, usb{mouse,kbd}
> and so on) so it's a prerequisite for the successful packaging of
> hotplug-ng.

Ok, then, care to make a patch to module-init-tools to provide this
functionality?

thanks,

greg k-h

2005-05-10 21:06:18

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, May 10, 2005 at 04:28:24PM -0400, Lee Revell wrote:
> On Tue, 2005-05-10 at 13:13 -0700, Greg KH wrote:
> > Also, the blacklisting stuff should not be
> > that prevelant anymore...
>
> It's quite often used by ALSA users who need to prevent hotplug from
> loading the OSS modules. Is there a better way to do this?

Don't build the OSS modules at all? :)

greg k-h

2005-05-10 21:12:15

by Marco d'Itri

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On May 10, Greg KH <[email protected]> wrote:

> > It's quite often used by ALSA users who need to prevent hotplug from
> > loading the OSS modules. Is there a better way to do this?
> Don't build the OSS modules at all? :)
Not a solution, some users still need them.

--
ciao,
Marco


Attachments:
(No filename) (284.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2005-05-10 21:11:45

by Marco d'Itri

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On May 10, Greg KH <[email protected]> wrote:

> > > > Because it's impossible to predict how it will interact with other
> > > > install and alias commands.
> > > Then we will just have to find out :)
> > It should be clear that it will interact badly with another install
> > commands, with one of them being ignored. This is not acceptable.
> Why? Will they not all just be checked in order?
No:

$ cat /etc/modprobe.d/test
install test-module /tmp/test-program 1
install test-module /tmp/test-program 2
$ cat /tmp/test-program
#!/bin/sh -e
echo $* > /tmp/test-log
$ modprobe --verbose test-module
install /tmp/test-program 2
$ cat /tmp/test-log
2
$

And if both commands were to be run, it would break in a different way
(blacklisting would not work).

> > It's a feature which I know my users and other maintainers need
> > (for duplicated drivers, OSS drivers, watchdog drivers, usb{mouse,kbd}
> > and so on) so it's a prerequisite for the successful packaging of
> > hotplug-ng.
> Ok, then, care to make a patch to module-init-tools to provide this
> functionality?
Eventually yes if nobody else will beat me, but I cannot spend time on
this currently.

--
ciao,
Marco


Attachments:
(No filename) (1.15 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2005-05-10 21:25:42

by Erik van Konijnenburg

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, May 10, 2005 at 11:08:23PM +0200, Marco d'Itri wrote:
> On May 10, Greg KH <[email protected]> wrote:
>
> > > It's a feature which I know my users and other maintainers need
> > > (for duplicated drivers, OSS drivers, watchdog drivers, usb{mouse,kbd}
> > > and so on) so it's a prerequisite for the successful packaging of
> > > hotplug-ng.
> > Ok, then, care to make a patch to module-init-tools to provide this
> > functionality?
> Eventually yes if nobody else will beat me, but I cannot spend time on
> this currently.

This patch would also be very useful for my pet project, building
smarter initial ram images; I'll have a go at it.

Regards,
Erik

2005-05-10 21:26:02

by Giuseppe Bilotta

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, 10 May 2005 13:13:55 -0700, Greg KH wrote:

> Also, the blacklisting stuff should not be
> that prevelant anymore...

Is there a way to control the order in which modules get loaded? For
example, I usually blacklist the parport module and only load it when
I need it, thus freeing an IRQ (for audio, IIRC). If parport loads
automatically, it grabs the IRQ; if it loads after the IRQ is grabbed
already, it'll resort to polled mode. Can these things be controlled
without the blacklist?

--
Giuseppe "Oblomov" Bilotta

"I'm never quite so stupid
as when I'm being smart" --Linus van Pelt

2005-05-10 21:36:30

by Bill Nottingham

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

Greg KH ([email protected]) said:
> > > > Because it's impossible to predict how it will interact with other
> > > > install and alias commands.
> > > Then we will just have to find out :)
> > It should be clear that it will interact badly with another install
> > commands, with one of them being ignored. This is not acceptable.
>
> Why? Will they not all just be checked in order?

You can only have one install command per name.

Bill

2005-05-10 21:52:15

by Per Liden

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Mon, 9 May 2005, Greg KH wrote:

> On Mon, May 09, 2005 at 12:52:02AM +0200, Per Liden wrote:
> > On Fri, 6 May 2005, Greg KH wrote:
> >
> > [...]
> > > Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> > > USB hotplug program can be written with a simple one line shell script:
> > > modprobe $MODALIAS
> >
> > Nice, but why not just convert all this to a call to
> > request_module($MODALIAS)? Seems to me like the natural thing to do.
>
> Because that's not the only thing that the hotplug event causes to
> happen. It's easier to have userspace decide what to do with this
> instead.

Sure, the hotplug event could still be issued so that userspace could do
magic things when it wants to (load firmware or whatever), but since the
kernel already has all the infrastructure in place to load modules on
demand, and it's used all over the place, it doesn't make sense to use a
completely different approach here.

Also, since most people never need to do anything except modprobe, they
can still have a working system without any scripts what so ever... again
just like normal on demand module loading.

/Per

2005-05-10 22:17:58

by Per Liden

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Mon, 9 May 2005, Per Svennerbrandt wrote:

> * Per Liden ([email protected]) wrote:
> > On Fri, 6 May 2005, Greg KH wrote:
> >
> > [...]
> > > Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> > > USB hotplug program can be written with a simple one line shell script:
> > > modprobe $MODALIAS
> >
> > Nice, but why not just convert all this to a call to
> > request_module($MODALIAS)? Seems to me like the natural thing to do.
>
> I actually have a pretty hackish proof-of-consept patch that does
> basicly that, and have been running it on my systems for the past five
> months or so, if anybody's interested.

Ah! Please post the patches.

> Along with it I also have a patch witch exports the module aliases for
> PCI and USB devices through sysfs. With it the "coldplugging" of a
> system (module wise) can be reduced to pretty much:
>
> #!/bin/sh
>
> for DEV in /sys/bus/{pci,usb}/devices/*; do
> modprobe `cat $DEV/modalias`
> done

Nice! This is really what coldplugging _should_ look like. Hmm, maybe
even coldplug the system by request_module()'ing those as well at some
stage?

> (And I actually run exactly that on my laptop, and it works surpricingly
> well. (Largly due to the fact that the usb-controller is always attached
> below the pci-bus of course, but it really wouldn't take that much work
> to make it do the right thing even without relying on any specific
> ordering/topology))
>
> With the above in place my system does all the module-loading that I
> care about automaticly, and most importantly does so without relying
> on an /etc/hotplug/ dir with everything and it's grandma in it (or at
> least thousands of lines of shellscripting).

This is exactly what I'm looking for as well.

> But since the request_modalias() thing seemed as such an obvious thing
> to do I have been reluctant to submit it fearing that I must have missed
> some fundamental flaw in it or you guys would have implemented it that
> way a long time ago? (at least since Rusty rewrote the module
> loader). Was I wrong*?
>
> Greg, Rusty, what do you think?

I'd like to get a better understanding of that as well. Why invent a
second on demand module loader when we have kmod? The current approach
feels like a step back to something very similar to the old kerneld.

/Per L

2005-05-10 22:42:08

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, May 11, 2005 at 12:17:12AM +0200, Per Liden wrote:
> I'd like to get a better understanding of that as well. Why invent a
> second on demand module loader when we have kmod? The current approach
> feels like a step back to something very similar to the old kerneld.

kmod is not used at all if you are running udev on your system. It's
also better to allow userspace to make the decision as to if it should
load a specific module or not, not the kernel.

And it allows us to get rid of the kmod code entirely :)

thanks,

greg k-h

2005-05-10 22:42:13

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Mon, May 09, 2005 at 11:13:24PM +0200, Per Svennerbrandt wrote:
>
> Along with it I also have a patch witch exports the module aliases for
> PCI and USB devices through sysfs. With it the "coldplugging" of a
> system (module wise) can be reduced to pretty much:

I'd like to see this, that should be acceptable to add to the kernel.

> #!/bin/sh
>
> for DEV in /sys/bus/{pci,usb}/devices/*; do
> modprobe `cat $DEV/modalias`
> done

Yes, very nice :)

thaks,

greg k-h

2005-05-10 23:56:17

by Per Liden

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, 10 May 2005, Greg KH wrote:

> On Wed, May 11, 2005 at 12:17:12AM +0200, Per Liden wrote:
> > I'd like to get a better understanding of that as well. Why invent a
> > second on demand module loader when we have kmod? The current approach
> > feels like a step back to something very similar to the old kerneld.
>
> kmod is not used at all if you are running udev on your system.

Since when does udev load modules for you? And how would it know when to
load "device less" modules such as filesystems?

> It's also better to allow userspace to make the decision as to if it
> should load a specific module or not, not the kernel.

If you don't want a specific module to be loaded, then don't build it.
You just said that yourself in the blacklisting dicsussion remember? ;)
(hint: "Don't build the OSS modules at all?").

/Per

2005-05-10 23:56:04

by Erik van Konijnenburg

[permalink] [raw]
Subject: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

Hi,

Patch against module-init-tools-3.2-pre4 to ignore modules
listed in /etc/hotplug/blacklist or blacklist.d (recursively).

* blacklist is only effective during adding,
not during remove or match with -a,
and not for modules required to resolve a dependency.
* tested only with -n, -v and --showdeps, not in live use.
* in particular, no testing for interaction with /etc/init.d scripts
* Roman, I'm not sure how this meshes with your patch to pass $MODNAME
from the driver to hotplug, discussed in:
http://marc.theaimsgroup.com/?l=linux-hotplug-devel&m=110994425816837
Perhaps you could have a look at it?

Regards,
Erik

Signed-off-by: Erik van Konijnenburg <[email protected]>

diff -urN module-init-tools-3.2-pre4/modprobe.c module-init-tools-3.2-pre4-new/modprobe.c
--- module-init-tools-3.2-pre4/modprobe.c 2005-05-08 09:38:52.000000000 +0200
+++ module-init-tools-3.2-pre4-new/modprobe.c 2005-05-11 01:14:16.000000000 +0200
@@ -1291,6 +1291,123 @@
return 0;
}

+
+struct blacklist
+{
+ struct blacklist *next;
+ char *module;
+};
+
+/* Link in a blacklist line */
+static struct blacklist *
+add_blacklist (const char *modname, struct blacklist *blacklist)
+{
+ struct blacklist *new;
+
+ new = NOFAIL(malloc(sizeof(*new)));
+ new->module = NOFAIL(strdup(modname));
+ new->next = blacklist;
+ return new;
+}
+
+/* add stuff from file to list, return false on error */
+static int
+read_blacklist_file (const char *filename, struct blacklist **blacklist)
+{
+ char *line;
+ unsigned int linenum = 0;
+ FILE *cfile;
+
+ cfile = fopen(filename, "r");
+ if (!cfile)
+ return 0;
+
+ while ((line = getline_wrapped(cfile, &linenum)) != NULL) {
+ char *ptr = line;
+ char *modname;
+
+ modname = strsep_skipspace(&ptr, "\t ");
+ if (modname == NULL || modname[0] == '#' || modname[0] == '\0')
+ continue;
+
+ *blacklist = add_blacklist(modname, *blacklist);
+ free(line);
+ }
+ fclose(cfile);
+ return 1;
+}
+
+/* add stuff from file or dir to list, return false on error */
+static int
+read_blacklist (const char *filename, struct blacklist **blacklist)
+{
+ DIR *dir;
+
+ /* If it's a directory, recurse. */
+ dir = opendir(filename);
+ if (dir) {
+ struct dirent *i;
+
+ /* FIXME: don't we want .rpmnew protection? */
+ while ((i = readdir(dir)) != NULL) {
+ if (!streq(i->d_name,".") && !streq(i->d_name,"..")) {
+ char sub[strlen(filename) + 1
+ + strlen(i->d_name) + 1];
+
+ sprintf(sub, "%s/%s", filename, i->d_name);
+ if (!read_blacklist(sub, blacklist))
+ warn("Failed to open"
+ " blacklist file %s: %s\n",
+ sub, strerror(errno));
+ }
+ }
+ closedir(dir);
+ return 1;
+ }
+
+ return read_blacklist_file(filename, blacklist);
+}
+
+static const char *default_blacklists[] =
+{
+ "/etc/hotplug/blacklist",
+ "/etc/hotplug/blacklist.d",
+};
+
+static void
+read_toplevel_blacklist(const char *filename, struct blacklist **blacklist)
+{
+ unsigned int i;
+
+ if (filename) {
+ if (!read_blacklist(filename, blacklist))
+ fatal("Failed to open blacklist file %s: %s\n",
+ filename, strerror(errno));
+ return;
+ }
+
+ /* Try defaults. */
+ for (i = 0; i < ARRAY_SIZE(default_blacklists); i++) {
+ if (!read_blacklist(default_blacklists[i], blacklist))
+ warn("Failed to open blacklist file %s: %s\n",
+ filename, strerror(errno));
+ }
+}
+
+static int is_blacklisted (struct blacklist *blacklist, char *modulename)
+{
+ int result = 0;
+
+ while (blacklist) {
+ if (strcmp (blacklist->module, modulename) == 0) {
+ result = 1;
+ break;
+ }
+ blacklist = blacklist->next;
+ }
+ return result;
+}
+
int main(int argc, char *argv[])
{
struct utsname buf;
@@ -1315,6 +1432,7 @@
char *newname = NULL;
char *aliasfilename, *symfilename;
errfn_t error = fatal;
+ struct blacklist *blacklist = NULL;

/* Prepend options from environment. */
argv = merge_args(getenv("MODPROBE_OPTIONS"), argv, &argc);
@@ -1475,6 +1593,9 @@
optstring = gather_options(argv+optind+1);
}

+ /* FIXME: extra option for alternate blacklist file? */
+ read_toplevel_blacklist (NULL, &blacklist);
+
/* num_modules is always 1 except for -r or -a. */
for (i = 0; i < num_modules; i++) {
struct module_command *commands = NULL;
@@ -1486,6 +1607,11 @@
/* Convert name we are looking for */
underscores(modulearg);

+ /* FIXME: do we blacklist on -a? */
+ if (is_blacklisted (blacklist, modulearg) && !remove) {
+ continue;
+ }
+
/* Returns the resolved alias, options */
read_toplevel_config(config, modulearg, 0,
remove, &modoptions, &commands, &aliases);

2005-05-11 00:03:45

by Rusty Russell

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, 2005-05-10 at 13:13 -0700, Greg KH wrote:
> On Tue, May 10, 2005 at 07:24:47PM +0200, Marco d'Itri wrote:
> > On May 10, "Alexander E. Patrakov" <[email protected]> wrote:
> >
> > > Why not this or something similar (e.g. I want to blacklist the xxx and
> > > yyy modules)? (note, untested)
>
> Nice, I like it.
>
> > Because it's impossible to predict how it will interact with other
> > install and alias commands.
>
> Then we will just have to find out :)

The interaction of aliases and install commands on the same thing is
deliberately undefined (eg. "alias foo bar // install foo ..." - which
takes priority?), but aliases that lead to install commands like this is
OK. If not, it's a bug.

The other possible solution is for /etc/hotplug.d/blacklist to contain
"install xxx /bin/false // install yyy /bin/true //
include /etc/modprobe.d" and have hotplug invoke modprobe with
--config=/etc/hotplug.d/blacklist. Substitute names to fit.

Now, should install commands in included files override install commands
in earlier files? Again, undefined, but currently they do: in the above
model maybe they shouldn't? We can nail it down either way...

Thanks,
Rusty.
--
A bad analogy is like a leaky screwdriver -- Richard Braakman

2005-05-11 00:07:27

by Marco d'Itri

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On May 11, Erik van Konijnenburg <[email protected]> wrote:

Thank you for your patch! I have two comments:
- the blacklist should be used only if modprobe is run by the kernel
(check $SEQNUM?)
- I think it could just look at the directory, the old file can be
trivially moved or simlinked in it on upgrades.

--
ciao,
Marco


Attachments:
(No filename) (330.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2005-05-11 00:11:45

by Marco d'Itri

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On May 11, Rusty Russell <[email protected]> wrote:

> The other possible solution is for /etc/hotplug.d/blacklist to contain
> "install xxx /bin/false // install yyy /bin/true //
> include /etc/modprobe.d" and have hotplug invoke modprobe with
> --config=/etc/hotplug.d/blacklist. Substitute names to fit.
I understand that this modprobe would look for an alias or install
directive for $MODALIAS, while it's the actual module name which users
need to blacklist (but I know a few situations in which it would be
useful to be able to match $MODALIAS on the blacklist too...).

--
ciao,
Marco


Attachments:
(No filename) (598.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2005-05-11 00:12:36

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, 2005-05-11 at 01:55 +0200, Erik van Konijnenburg wrote:
> Hi,
>
> Patch against module-init-tools-3.2-pre4 to ignore modules
> listed in /etc/hotplug/blacklist or blacklist.d (recursively).

That's a lot of code to make a module impossible to install, and I think
it kind of misses the point. Surely a non-hotplug "modprobe
blacklisted-module" should work?

If we define the first install to override included install commands,
and hotplug uses --config=<blacklistfile>, I think we have a better
solution.

> * tested only with -n, -v and --showdeps, not in live use.

I'm going to start bundling the testsuite with the source, to encourage
people to actually use it: it's not that hard...

Rusty.
--
A bad analogy is like a leaky screwdriver -- Richard Braakman

2005-05-11 01:09:44

by Rusty Russell

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, 2005-05-11 at 02:10 +0200, Marco d'Itri wrote:
> On May 11, Rusty Russell <[email protected]> wrote:
>
> > The other possible solution is for /etc/hotplug.d/blacklist to contain
> > "install xxx /bin/false // install yyy /bin/true //
> > include /etc/modprobe.d" and have hotplug invoke modprobe with
> > --config=/etc/hotplug.d/blacklist. Substitute names to fit.

> I understand that this modprobe would look for an alias or install
> directive for $MODALIAS, while it's the actual module name which users
> need to blacklist (but I know a few situations in which it would be
> useful to be able to match $MODALIAS on the blacklist too...).

Yes, I'm assuming that the config file/dir used by hotplug would simply
look like:

# Install commands so hotplug doesn't load some modules.
# Use /bin/true so modprobe doesn't complain.

# evbug is a debug tool and should be loaded explicitly
install evbug /bin/true

To blacklist by aliases, you could use install commands (which, again
undefined, are actually implemented to override alias commands):

# hotplug thinks that the XYZ driver is great for this card.
# But that modprobe causes uncomfortable nasal daemons.
install pci:v1d1dc0sd0bc10sc10i1 /bin/hotplug-warning

If that becomes important, I can document that behaviour and add a test
case for it.

Thanks!
Rusty.
--
A bad analogy is like a leaky screwdriver -- Richard Braakman

2005-05-11 01:11:59

by Erik van Konijnenburg

[permalink] [raw]
Subject: Re: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, May 11, 2005 at 10:08:25AM +1000, Rusty Russell wrote:
> On Wed, 2005-05-11 at 01:55 +0200, Erik van Konijnenburg wrote:
> > Hi,
> >
> > Patch against module-init-tools-3.2-pre4 to ignore modules
> > listed in /etc/hotplug/blacklist or blacklist.d (recursively).
>
> That's a lot of code to make a module impossible to install, and I think
> it kind of misses the point. Surely a non-hotplug "modprobe
> blacklisted-module" should work?
>
> If we define the first install to override included install commands,
> and hotplug uses --config=<blacklistfile>, I think we have a better
> solution.

Lets see:
- You want less code (for good reason, it really is a lot more
than the equivalent in perl)
- Marco wants the blacklist only to work when invoked via kernel,
and needs only /etc/hotplug/blacklist.d.
- Marco wants the blacklist to apply after alias expansion
rather than before.
- I want to avoid install commands whenever possible.
The reason is that for building an initramfs, I need to interpret
modprobe output and decide what goes on the image. For 'insmod'
that's easy; but I don't know of a way to interpret install
command lines and deciding *reliably* what executables and libraries
need to go on the image. (incidentally, this is a use case
where blacklist interpretation is desirable, even though modprobe
is not invoked by the kernel)

Here's an alternative approach that should cover these interests:
- add a keyword 'blacklist' to the configuration language,
that will be interpreted after alias expansion, but before
searching modules.dep.

Advantages:
- it needs a lot less code
- distributions can decide whether blacklists work always,
never, or only for the kernel simply by playing with which
configuration file is used
- my initramfs builder does not have to be special cased
to know that some install directives really are blacklist
directives.

If this approach seems workable to you, I plan to build something
like this tomorrow.

> > * tested only with -n, -v and --showdeps, not in live use.
>
> I'm going to start bundling the testsuite with the source, to encourage
> people to actually use it: it's not that hard...

That's a hint I guess ... OK, next patch will go through the suite.

Regards,
Erik

2005-05-11 01:22:29

by Brian Gerst

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

Per Liden wrote:
> On Tue, 10 May 2005, Greg KH wrote:
>
>
>>On Wed, May 11, 2005 at 12:17:12AM +0200, Per Liden wrote:
>>
>>>I'd like to get a better understanding of that as well. Why invent a
>>>second on demand module loader when we have kmod? The current approach
>>>feels like a step back to something very similar to the old kerneld.
>>
>>kmod is not used at all if you are running udev on your system.
>
>
> Since when does udev load modules for you? And how would it know when to
> load "device less" modules such as filesystems?
>
>
>>It's also better to allow userspace to make the decision as to if it
>>should load a specific module or not, not the kernel.
>
>
> If you don't want a specific module to be loaded, then don't build it.
> You just said that yourself in the blacklisting dicsussion remember? ;)
> (hint: "Don't build the OSS modules at all?").

Think about distibution kernels that build everything possible.

--
Brian Gerst

2005-05-11 02:04:29

by Bodo Eggert

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

Giuseppe Bilotta <[email protected]> wrote:

> Is there a way to control the order in which modules get loaded? For
> example, I usually blacklist the parport module and only load it when
> I need it, thus freeing an IRQ (for audio, IIRC). If parport loads
> automatically, it grabs the IRQ; if it loads after the IRQ is grabbed
> already, it'll resort to polled mode. Can these things be controlled
> without the blacklist?

Documentation/parport.txt

The rest should be configurable in /etc/mod{ules,probe}.conf
--
Top 100 things you don't want the sysadmin to say:
69. ``Why is my "rm *.o" taking so long?''

2005-05-11 03:39:46

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, 2005-05-11 at 03:11 +0200, Erik van Konijnenburg wrote:
> Here's an alternative approach that should cover these interests:
> - add a keyword 'blacklist' to the configuration language,
> that will be interpreted after alias expansion, but before
> searching modules.dep.

This makes "blacklist X" equivalent to "install X /bin/true" right? i.e
"ignore it".

> Advantages:
> - it needs a lot less code
> - distributions can decide whether blacklists work always,
> never, or only for the kernel simply by playing with which
> configuration file is used
> - my initramfs builder does not have to be special cased
> to know that some install directives really are blacklist
> directives.

Well, a module mentioned in hotplug's blacklist file would be a pretty
good candidate for exclusion from your initramfs builder. Existing
install commands are already trouble for initramfs building, since they
can do arbitrary things...

How about I allow "--config=-" and hotplug can use the existing
blacklists and 'sed'?

Rusty.
--
A bad analogy is like a leaky screwdriver -- Richard Braakman

2005-05-11 05:33:36

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, May 11, 2005 at 01:56:01AM +0200, Per Liden wrote:
> On Tue, 10 May 2005, Greg KH wrote:
>
> > On Wed, May 11, 2005 at 12:17:12AM +0200, Per Liden wrote:
> > > I'd like to get a better understanding of that as well. Why invent a
> > > second on demand module loader when we have kmod? The current approach
> > > feels like a step back to something very similar to the old kerneld.
> >
> > kmod is not used at all if you are running udev on your system.
>
> Since when does udev load modules for you?

It does not. That was my point :)

> And how would it know when to load "device less" modules such as
> filesystems?

Ah damm, I forgot about those pesky things. Oh well, my plot to rid the
kernel of kmod will take a few more years...

greg k-h

2005-05-11 05:36:55

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, May 10, 2005 at 11:51:04PM +0200, Per Liden wrote:
> On Mon, 9 May 2005, Greg KH wrote:
>
> > On Mon, May 09, 2005 at 12:52:02AM +0200, Per Liden wrote:
> > > On Fri, 6 May 2005, Greg KH wrote:
> > >
> > > [...]
> > > > Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> > > > USB hotplug program can be written with a simple one line shell script:
> > > > modprobe $MODALIAS
> > >
> > > Nice, but why not just convert all this to a call to
> > > request_module($MODALIAS)? Seems to me like the natural thing to do.
> >
> > Because that's not the only thing that the hotplug event causes to
> > happen. It's easier to have userspace decide what to do with this
> > instead.
>
> Sure, the hotplug event could still be issued so that userspace could do
> magic things when it wants to (load firmware or whatever), but since the
> kernel already has all the infrastructure in place to load modules on
> demand, and it's used all over the place, it doesn't make sense to use a
> completely different approach here.

Well, do you really want your kernel to be creating 2 userspace programs
for every device in sysfs that is created? Remember, we can't just not
emit that hotplug event, too many programs need it....

> Also, since most people never need to do anything except modprobe, they
> can still have a working system without any scripts what so ever... again
> just like normal on demand module loading.

Lots of programs do lots of stuff other than modprobe on hotplug events.
Look at HAL for just one example.

thanks,

greg k-h

2005-05-11 05:40:19

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, May 11, 2005 at 02:05:19AM +0200, Marco d'Itri wrote:
> On May 11, Erik van Konijnenburg <[email protected]> wrote:
>
> Thank you for your patch! I have two comments:
> - the blacklist should be used only if modprobe is run by the kernel
> (check $SEQNUM?)

No, it should be used only if you are running modprobe on an alias.

thanks,

greg k-h

2005-05-11 10:00:32

by Erik van Konijnenburg

[permalink] [raw]
Subject: Re: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, May 11, 2005 at 01:39:13PM +1000, Rusty Russell wrote:
> On Wed, 2005-05-11 at 03:11 +0200, Erik van Konijnenburg wrote:
> > Here's an alternative approach that should cover these interests:
> > - add a keyword 'blacklist' to the configuration language,
> > that will be interpreted after alias expansion, but before
> > searching modules.dep.
>
> This makes "blacklist X" equivalent to "install X /bin/true" right? i.e
> "ignore it".

Except for output on --show-depends and total number of forks, yes.

Based on comments from Greg and Christian, it would be better to apply
blacklisting only to the result of alias expanding for kernel generated
module maps. So:

cat >> /etc/hotplug/blacklist.d/test <<//
blacklist snd_rme96
//

would apply to

modprobe 'pci:v000010EEd00003FC0sv*sd*bc*sc*i*'

but not to

modprobe snd_rme96

> > Advantages:
> > - it needs a lot less code
> > - distributions can decide whether blacklists work always,
> > never, or only for the kernel simply by playing with which
> > configuration file is used
> > - my initramfs builder does not have to be special cased
> > to know that some install directives really are blacklist
> > directives.
>
> Well, a module mentioned in hotplug's blacklist file would be a pretty
> good candidate for exclusion from your initramfs builder. Existing
> install commands are already trouble for initramfs building, since they
> can do arbitrary things...

Yep. At the moment my initramfs builder aborts on such install directives,
because there's no correct way of handling them. Since install isn't used
all that much and certainly not in boot-critical stuff, thats acceptable,
but if we're going to use install for every blacklisted module,
that's a problem.

> How about I allow "--config=-" and hotplug can use the existing
> blacklists and 'sed'?

That would keep existing blacklist, which is good for Marco and
for initramfs builders.

However, it would be bad for hotplug-ng: one of the strong points
is that it speeds booting by dropping loads of shell code, and
adding sed for blacklist processing does not fit well with that.

I'll take a stab at a patch that introduces the 'blacklist' keyword
to see if that reduces the code enough to make it acceptable.

Regards,
Erik


2005-05-11 10:52:25

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, 2005-05-11 at 11:59 +0200, Erik van Konijnenburg wrote:
> Based on comments from Greg and Christian, it would be better to apply
> blacklisting only to the result of alias expanding for kernel generated
> module maps.

Then perhaps depmod should be the one to read a blacklist file? It
produces the modules.alias file where these things live.

Rusty.
--
A bad analogy is like a leaky screwdriver -- Richard Braakman

2005-05-11 10:58:37

by Marco d'Itri

[permalink] [raw]
Subject: Re: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

On May 11, Rusty Russell <[email protected]> wrote:

> Then perhaps depmod should be the one to read a blacklist file? It
> produces the modules.alias file where these things live.
I think this would be confusing for users.

--
ciao,
Marco


Attachments:
(No filename) (246.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2005-05-11 13:06:44

by Erik van Konijnenburg

[permalink] [raw]
Subject: Re: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, May 11, 2005 at 12:58:18PM +0200, Marco d'Itri wrote:
> On May 11, Rusty Russell <[email protected]> wrote:
>
> > Then perhaps depmod should be the one to read a blacklist file? It
> > produces the modules.alias file where these things live.
> I think this would be confusing for users.

Second that. Modprobe remains smaller, but we would need some wrapper
around depmod to do the blacklisting, plus the user or packager that
adds an entry to the blacklist has to arrange for depmod to be redone
at the right time. It's so much more convenient if depmod only has
to run at kernel install time.

Here's a second version of the patch, now based on 'blacklist' keyword.

The patch has shrunken from 126 insertions(+)
to 81 insertions(+), 17 deletions(-)

Testing: with blacklist via82cxxx_audio,
./modprobe -n -v 'pci:v00001106d00003059sv*sd*bc*sc*i*
gives only alsa, otherwise alsa plus oss.
./modprobe -n -v via82cxxx_audio
gives oss, regardless of blacklisting.

The test suite complains here for 02proc.sh, both with and without the patch,
in tests/test-modprobe/02proc.sh. Let me know if it's worthwhile to
dive into that.

Regards,
Erik


Signed-off-by: <[email protected]> Erik van Konijnenburg

--- module-init-tools-3.2-pre4/modprobe.c 2005-05-08 09:38:52.000000000 +0200
+++ module-init-tools-3.2-pre4-new/modprobe.c 2005-05-11 14:22:15.000000000 +0200
@@ -611,6 +611,12 @@
char *module;
};

+struct module_blacklist
+{
+ struct module_blacklist *next;
+ char *modulename;
+};
+
/* Link in a new option line from the config file. */
static struct module_options *
add_options(const char *modname,
@@ -657,6 +663,45 @@
return new;
}

+/* Link in a new blacklist line from the config file. */
+static struct module_blacklist *
+add_blacklist(const char *modname, struct module_blacklist *blacklist)
+{
+ struct module_blacklist *new;
+
+ new = NOFAIL(malloc(sizeof(*new)));
+ new->modulename = NOFAIL(strdup(modname));
+ new->next = blacklist;
+ return new;
+}
+
+/* Find blacklist commands if any. */
+static int
+find_blacklist(const char *modname, const struct module_blacklist *blacklist)
+{
+ while (blacklist) {
+ if (strcmp(blacklist->modulename, modname) == 0)
+ return 1;
+ blacklist = blacklist->next;
+ }
+ return 0;
+}
+
+/* return a new alias list, with backlisted elems filtered out */
+static struct module_alias *
+apply_blacklist(const struct module_alias *aliases,
+ const struct module_blacklist *blacklist)
+{
+ struct module_alias *result = NULL;
+ while (aliases) {
+ char *modname = aliases->module;
+ if (! find_blacklist (modname, blacklist))
+ result = add_alias (modname, result);
+ aliases = aliases->next;
+ }
+ return result;
+}
+
/* Find install commands if any. */
static const char *find_command(const char *modname,
const struct module_command *commands)
@@ -977,7 +1022,8 @@
int removing,
struct module_options **options,
struct module_command **commands,
- struct module_alias **alias);
+ struct module_alias **alias,
+ struct module_blacklist **blacklist);

/* FIXME: Maybe should be extended to "alias a b [and|or c]...". --RR */
static int read_config_file(const char *filename,
@@ -986,7 +1032,8 @@
int removing,
struct module_options **options,
struct module_command **commands,
- struct module_alias **aliases)
+ struct module_alias **aliases,
+ struct module_blacklist **blacklist)
{
char *line;
unsigned int linenum = 0;
@@ -1027,7 +1074,8 @@
else {
if (!read_config(newfilename, name,
dump_only, removing,
- options, commands, &newalias))
+ options, commands, &newalias,
+ blacklist))
warn("Failed to open included"
" config file %s: %s\n",
newfilename, strerror(errno));
@@ -1055,6 +1103,14 @@
*commands = add_command(underscores(modname),
ptr, *commands);
}
+ } else if (strcmp(cmd, "blacklist") == 0) {
+ modname = strsep_skipspace(&ptr, "\t ");
+ if (!modname)
+ grammar(cmd, filename, linenum);
+ else if (!removing) {
+ *blacklist = add_blacklist(underscores(modname),
+ *blacklist);
+ }
} else if (strcmp(cmd, "remove") == 0) {
modname = strsep_skipspace(&ptr, "\t ");
if (!modname || !ptr)
@@ -1081,7 +1137,8 @@
int removing,
struct module_options **options,
struct module_command **commands,
- struct module_alias **aliases)
+ struct module_alias **aliases,
+ struct module_blacklist **blacklist)
{
DIR *dir;

@@ -1097,8 +1154,8 @@

sprintf(sub, "%s/%s", filename, i->d_name);
if (!read_config(sub, name,
- dump_only, removing,
- options, commands, aliases))
+ dump_only, removing, options,
+ commands, aliases, blacklist))
warn("Failed to open"
" config file %s: %s\n",
sub, strerror(errno));
@@ -1109,7 +1166,7 @@
}

return read_config_file(filename, name, dump_only, removing,
- options, commands, aliases);
+ options, commands, aliases, blacklist);
}

static const char *default_configs[] =
@@ -1124,13 +1181,14 @@
int removing,
struct module_options **options,
struct module_command **commands,
- struct module_alias **aliases)
+ struct module_alias **aliases,
+ struct module_blacklist **blacklist)
{
unsigned int i;

if (filename) {
if (!read_config(filename, name, dump_only, removing,
- options, commands, aliases))
+ options, commands, aliases, blacklist))
fatal("Failed to open config file %s: %s\n",
filename, strerror(errno));
return;
@@ -1138,8 +1196,8 @@

/* Try defaults. */
for (i = 0; i < ARRAY_SIZE(default_configs); i++) {
- if (read_config(default_configs[i], name, dump_only,
- removing, options, commands, aliases))
+ if (read_config(default_configs[i], name, dump_only, removing,
+ options, commands, aliases, blacklist))
return;
}
}
@@ -1457,13 +1515,14 @@
struct module_command *commands = NULL;
struct module_options *modoptions = NULL;
struct module_alias *aliases = NULL;
+ struct module_blacklist *blacklist = NULL;

read_toplevel_config(config, "", 1, 0,
- &modoptions, &commands, &aliases);
+ &modoptions, &commands, &aliases, &blacklist);
read_config(aliasfilename, "", 1, 0,&modoptions, &commands,
- &aliases);
+ &aliases, &blacklist);
read_config(symfilename, "", 1, 0, &modoptions, &commands,
- &aliases);
+ &aliases, &blacklist);
exit(0);
}

@@ -1480,6 +1539,7 @@
struct module_command *commands = NULL;
struct module_options *modoptions = NULL;
struct module_alias *aliases = NULL;
+ struct module_blacklist *blacklist = NULL;
LIST_HEAD(list);
char *modulearg = argv[optind + i];

@@ -1488,13 +1548,14 @@

/* Returns the resolved alias, options */
read_toplevel_config(config, modulearg, 0,
- remove, &modoptions, &commands, &aliases);
+ remove, &modoptions, &commands, &aliases, &blacklist);

/* No luck? Try symbol names, if starts with symbol:. */
if (!aliases
&& strncmp(modulearg, "symbol:", strlen("symbol:")) == 0)
read_config(symfilename, modulearg, 0,
- remove, &modoptions, &commands, &aliases);
+ remove, &modoptions, &commands,
+ &aliases, &blacklist);

if (!aliases) {
/* We only use canned aliases as last resort. */
@@ -1502,9 +1563,12 @@

if (list_empty(&list)
&& !find_command(modulearg, commands))
+ {
read_config(aliasfilename, modulearg, 0,
remove, &modoptions, &commands,
- &aliases);
+ &aliases, &blacklist);
+ aliases = apply_blacklist(aliases, blacklist);
+ }
}

if (aliases) {

2005-05-11 17:39:15

by Per Liden

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Tue, 10 May 2005, Greg KH wrote:

> On Wed, May 11, 2005 at 01:56:01AM +0200, Per Liden wrote:
> > On Tue, 10 May 2005, Greg KH wrote:
> >
> > > On Wed, May 11, 2005 at 12:17:12AM +0200, Per Liden wrote:
> > > > I'd like to get a better understanding of that as well. Why invent a
> > > > second on demand module loader when we have kmod? The current approach
> > > > feels like a step back to something very similar to the old kerneld.
> > >
> > > kmod is not used at all if you are running udev on your system.
> >
> > Since when does udev load modules for you?
>
> It does not. That was my point :)

Sorry, but I still don't get your point. Whether kmod is used or not is
unrelated to the use of udev.

> > And how would it know when to load "device less" modules such as
> > filesystems?
>
> Ah damm, I forgot about those pesky things. Oh well, my plot to rid the
> kernel of kmod will take a few more years...

I'm relieved ;)

/Per

2005-05-11 17:43:09

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, May 11, 2005 at 07:36:54PM +0200, Per Liden wrote:
> On Tue, 10 May 2005, Greg KH wrote:
>
> > On Wed, May 11, 2005 at 01:56:01AM +0200, Per Liden wrote:
> > > On Tue, 10 May 2005, Greg KH wrote:
> > >
> > > > On Wed, May 11, 2005 at 12:17:12AM +0200, Per Liden wrote:
> > > > > I'd like to get a better understanding of that as well. Why invent a
> > > > > second on demand module loader when we have kmod? The current approach
> > > > > feels like a step back to something very similar to the old kerneld.
> > > >
> > > > kmod is not used at all if you are running udev on your system.
> > >
> > > Since when does udev load modules for you?
> >
> > It does not. That was my point :)
>
> Sorry, but I still don't get your point. Whether kmod is used or not is
> unrelated to the use of udev.

I was trying to point out that if you use udev, kmod is not used at all
to autoload modules when you try to open a device file for a driver that
is not present (as if the module isn't loaded, then the device file
would not be present to try to open.)

But I forgot about the filesystem stuff...

Hope this clears things up.

greg k-h

2005-05-12 04:40:24

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, 2005-05-11 at 15:06 +0200, Erik van Konijnenburg wrote:
> On Wed, May 11, 2005 at 12:58:18PM +0200, Marco d'Itri wrote:
> > On May 11, Rusty Russell <[email protected]> wrote:
> >
> > > Then perhaps depmod should be the one to read a blacklist file? It
> > > produces the modules.alias file where these things live.
> > I think this would be confusing for users.
>
> Second that. Modprobe remains smaller, but we would need some wrapper
> around depmod to do the blacklisting, plus the user or packager that
> adds an entry to the blacklist has to arrange for depmod to be redone
> at the right time. It's so much more convenient if depmod only has
> to run at kernel install time.

Applied, with testsuite and documentation, released as pre5. Diff below
for your convenience. Erik, if you could use "./tests/runtests -vv
02proc.sh" and tell me what's failing for you, that'd help (an unwitting
distro dependency?)

Cheers,
Rusty.

diff --exclude tests -urN module-init-tools-3.2-pre4/doc/modprobe.conf.sgml module-init-tools-3.2-pre5/doc/modprobe.conf.sgml
--- module-init-tools-3.2-pre4/doc/modprobe.conf.sgml 2004-11-08 11:15:47.000000000 +1100
+++ module-init-tools-3.2-pre5/doc/modprobe.conf.sgml 2005-05-12 12:08:33.000000000 +1000
@@ -162,6 +162,22 @@
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>blacklist <replaceable>modulename</replaceable>
+ </term>
+ <listitem>
+ <para>
+ Modules can contain their own aliases: usually these are
+ aliases describing the devices they support, such as
+ "pci:123...". These "internal" aliases can be overridden
+ by normal "alias" keywords, but there are cases where two
+ or more modules both support the same devices, or a module
+ invalidly claims to support a device: the
+ <command>blacklist</command> keyword indicates that all of
+ a particular module's internal aliases are to be ignored.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</refsect1>
<refsect1>
diff --exclude tests -urN module-init-tools-3.2-pre4/modprobe.c module-init-tools-3.2-pre5/modprobe.c
--- module-init-tools-3.2-pre4/modprobe.c 2005-05-08 17:38:52.000000000 +1000
+++ module-init-tools-3.2-pre5/modprobe.c 2005-05-12 11:58:22.000000000 +1000
@@ -611,6 +611,12 @@
char *module;
};

+struct module_blacklist
+{
+ struct module_blacklist *next;
+ char *modulename;
+};
+
/* Link in a new option line from the config file. */
static struct module_options *
add_options(const char *modname,
@@ -657,6 +663,45 @@
return new;
}

+/* Link in a new blacklist line from the config file. */
+static struct module_blacklist *
+add_blacklist(const char *modname, struct module_blacklist *blacklist)
+{
+ struct module_blacklist *new;
+
+ new = NOFAIL(malloc(sizeof(*new)));
+ new->modulename = NOFAIL(strdup(modname));
+ new->next = blacklist;
+ return new;
+}
+
+/* Find blacklist commands if any. */
+static int
+find_blacklist(const char *modname, const struct module_blacklist *blacklist)
+{
+ while (blacklist) {
+ if (strcmp(blacklist->modulename, modname) == 0)
+ return 1;
+ blacklist = blacklist->next;
+ }
+ return 0;
+}
+
+/* return a new alias list, with backlisted elems filtered out */
+static struct module_alias *
+apply_blacklist(const struct module_alias *aliases,
+ const struct module_blacklist *blacklist)
+{
+ struct module_alias *result = NULL;
+ while (aliases) {
+ char *modname = aliases->module;
+ if (!find_blacklist(modname, blacklist))
+ result = add_alias(modname, result);
+ aliases = aliases->next;
+ }
+ return result;
+}
+
/* Find install commands if any. */
static const char *find_command(const char *modname,
const struct module_command *commands)
@@ -977,7 +1022,8 @@
int removing,
struct module_options **options,
struct module_command **commands,
- struct module_alias **alias);
+ struct module_alias **alias,
+ struct module_blacklist **blacklist);

/* FIXME: Maybe should be extended to "alias a b [and|or c]...". --RR */
static int read_config_file(const char *filename,
@@ -986,7 +1032,8 @@
int removing,
struct module_options **options,
struct module_command **commands,
- struct module_alias **aliases)
+ struct module_alias **aliases,
+ struct module_blacklist **blacklist)
{
char *line;
unsigned int linenum = 0;
@@ -1027,7 +1074,8 @@
else {
if (!read_config(newfilename, name,
dump_only, removing,
- options, commands, &newalias))
+ options, commands, &newalias,
+ blacklist))
warn("Failed to open included"
" config file %s: %s\n",
newfilename, strerror(errno));
@@ -1055,6 +1103,14 @@
*commands = add_command(underscores(modname),
ptr, *commands);
}
+ } else if (strcmp(cmd, "blacklist") == 0) {
+ modname = strsep_skipspace(&ptr, "\t ");
+ if (!modname)
+ grammar(cmd, filename, linenum);
+ else if (!removing) {
+ *blacklist = add_blacklist(underscores(modname),
+ *blacklist);
+ }
} else if (strcmp(cmd, "remove") == 0) {
modname = strsep_skipspace(&ptr, "\t ");
if (!modname || !ptr)
@@ -1081,7 +1137,8 @@
int removing,
struct module_options **options,
struct module_command **commands,
- struct module_alias **aliases)
+ struct module_alias **aliases,
+ struct module_blacklist **blacklist)
{
DIR *dir;

@@ -1097,8 +1154,8 @@

sprintf(sub, "%s/%s", filename, i->d_name);
if (!read_config(sub, name,
- dump_only, removing,
- options, commands, aliases))
+ dump_only, removing, options,
+ commands, aliases, blacklist))
warn("Failed to open"
" config file %s: %s\n",
sub, strerror(errno));
@@ -1109,7 +1166,7 @@
}

return read_config_file(filename, name, dump_only, removing,
- options, commands, aliases);
+ options, commands, aliases, blacklist);
}

static const char *default_configs[] =
@@ -1124,13 +1181,14 @@
int removing,
struct module_options **options,
struct module_command **commands,
- struct module_alias **aliases)
+ struct module_alias **aliases,
+ struct module_blacklist **blacklist)
{
unsigned int i;

if (filename) {
if (!read_config(filename, name, dump_only, removing,
- options, commands, aliases))
+ options, commands, aliases, blacklist))
fatal("Failed to open config file %s: %s\n",
filename, strerror(errno));
return;
@@ -1138,8 +1196,8 @@

/* Try defaults. */
for (i = 0; i < ARRAY_SIZE(default_configs); i++) {
- if (read_config(default_configs[i], name, dump_only,
- removing, options, commands, aliases))
+ if (read_config(default_configs[i], name, dump_only, removing,
+ options, commands, aliases, blacklist))
return;
}
}
@@ -1457,13 +1515,14 @@
struct module_command *commands = NULL;
struct module_options *modoptions = NULL;
struct module_alias *aliases = NULL;
+ struct module_blacklist *blacklist = NULL;

read_toplevel_config(config, "", 1, 0,
- &modoptions, &commands, &aliases);
+ &modoptions, &commands, &aliases, &blacklist);
read_config(aliasfilename, "", 1, 0,&modoptions, &commands,
- &aliases);
+ &aliases, &blacklist);
read_config(symfilename, "", 1, 0, &modoptions, &commands,
- &aliases);
+ &aliases, &blacklist);
exit(0);
}

@@ -1480,6 +1539,7 @@
struct module_command *commands = NULL;
struct module_options *modoptions = NULL;
struct module_alias *aliases = NULL;
+ struct module_blacklist *blacklist = NULL;
LIST_HEAD(list);
char *modulearg = argv[optind + i];

@@ -1488,13 +1548,14 @@

/* Returns the resolved alias, options */
read_toplevel_config(config, modulearg, 0,
- remove, &modoptions, &commands, &aliases);
+ remove, &modoptions, &commands, &aliases, &blacklist);

/* No luck? Try symbol names, if starts with symbol:. */
if (!aliases
&& strncmp(modulearg, "symbol:", strlen("symbol:")) == 0)
read_config(symfilename, modulearg, 0,
- remove, &modoptions, &commands, &aliases);
+ remove, &modoptions, &commands,
+ &aliases, &blacklist);

if (!aliases) {
/* We only use canned aliases as last resort. */
@@ -1502,9 +1563,12 @@

if (list_empty(&list)
&& !find_command(modulearg, commands))
+ {
read_config(aliasfilename, modulearg, 0,
remove, &modoptions, &commands,
- &aliases);
+ &aliases, &blacklist);
+ aliases = apply_blacklist(aliases, blacklist);
+ }
}

if (aliases) {

--
A bad analogy is like a leaky screwdriver -- Richard Braakman

2005-05-12 07:47:57

by Erik van Konijnenburg

[permalink] [raw]
Subject: Re: [PATCH] Re: [ANNOUNCE] hotplug-ng 002 release

On Thu, May 12, 2005 at 02:39:31PM +1000, Rusty Russell wrote:
> Applied, with testsuite and documentation, released as pre5. Diff below
> for your convenience. Erik, if you could use "./tests/runtests -vv
> 02proc.sh" and tell me what's failing for you, that'd help (an unwitting
> distro dependency?)

False alarm. The test suites for pre4 and pre5 now both pass here,
so the earlier errors were probably due to some sloppyness on my side.

Regards,
Erik

2005-05-12 21:45:00

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Mon, May 09, 2005 at 11:13:24PM +0200, Per Svennerbrandt wrote:
> * Per Liden ([email protected]) wrote:
> > On Fri, 6 May 2005, Greg KH wrote:
> >
> > [...]
> > > Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> > > USB hotplug program can be written with a simple one line shell script:
> > > modprobe $MODALIAS
> >
> > Nice, but why not just convert all this to a call to
> > request_module($MODALIAS)? Seems to me like the natural thing to do.
>
> I actually have a pretty hackish proof-of-consept patch that does
> basicly that, and have been running it on my systems for the past five
> months or so, if anybody's interested.
>
> Along with it I also have a patch witch exports the module aliases for
> PCI and USB devices through sysfs. With it the "coldplugging" of a
> system (module wise) can be reduced to pretty much:
>
> #!/bin/sh
>
> for DEV in /sys/bus/{pci,usb}/devices/*; do
> modprobe `cat $DEV/modalias`
> done

Ok, as you never posted your patch, I had to do it myself :)

Here's 3 patches that I just added to my trees, and will show up in the
next -mm release. They create the modalias file for usb and pci
devices, and add the MODALIAS env variable for the pci hotplug event.

thanks,

greg k-h


Subject: PCI: add MODALIAS to hotplug event for pci devices

Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/pci/hotplug.c | 10 ++++++++++
1 files changed, 10 insertions(+)

--- gregkh-2.6.orig/drivers/pci/hotplug.c 2005-05-12 14:28:39.000000000 -0700
+++ gregkh-2.6/drivers/pci/hotplug.c 2005-05-12 14:28:47.000000000 -0700
@@ -52,6 +52,16 @@
if ((buffer_size - length <= 0) || (i >= num_envp))
return -ENOMEM;

+ envp[i++] = scratch;
+ length += scnprintf (scratch, buffer_size - length,
+ "MODALIAS=pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
+ pdev->vendor, pdev->device,
+ pdev->subsystem_vendor, pdev->subsystem_device,
+ (u8)(pdev->class >> 16), (u8)(pdev->class >> 8),
+ (u8)(pdev->class));
+ if ((buffer_size - length <= 0) || (i >= num_envp))
+ return -ENOMEM;
+
envp[i] = NULL;

return 0;


Subject: PCI: add modalias sysfs file for pci devices

Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/pci/pci-sysfs.c | 12 ++++++++++++
1 files changed, 12 insertions(+)

--- gregkh-2.6.orig/drivers/pci/pci-sysfs.c 2005-05-12 14:28:25.000000000 -0700
+++ gregkh-2.6/drivers/pci/pci-sysfs.c 2005-05-12 14:28:40.000000000 -0700
@@ -73,6 +73,17 @@
return (str - buf);
}

+static ssize_t modalias_show(struct device *dev, char *buf)
+{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
+
+ return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
+ pci_dev->vendor, pci_dev->device,
+ pci_dev->subsystem_vendor, pci_dev->subsystem_device,
+ (u8)(pci_dev->class >> 16), (u8)(pci_dev->class >> 8),
+ (u8)(pci_dev->class));
+}
+
struct device_attribute pci_dev_attrs[] = {
__ATTR_RO(resource),
__ATTR_RO(vendor),
@@ -82,6 +93,7 @@
__ATTR_RO(class),
__ATTR_RO(irq),
__ATTR_RO(local_cpus),
+ __ATTR_RO(modalias),
__ATTR_NULL,
};



Subject: USB: add modalias sysfs file for usb devices

Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/usb/core/sysfs.c | 34 ++++++++++++++++++++++++++++++++++
1 files changed, 34 insertions(+)

--- gregkh-2.6.orig/drivers/usb/core/sysfs.c 2005-05-12 14:28:59.000000000 -0700
+++ gregkh-2.6/drivers/usb/core/sysfs.c 2005-05-12 14:29:05.000000000 -0700
@@ -286,6 +286,39 @@
}
static DEVICE_ATTR(interface, S_IRUGO, show_interface_string, NULL);

+static ssize_t show_modalias(struct device *dev, char *buf)
+{
+ struct usb_interface *intf;
+ struct usb_device *udev;
+
+ intf = to_usb_interface(dev);
+ udev = interface_to_usbdev(intf);
+ if (udev->descriptor.bDeviceClass == 0) {
+ struct usb_host_interface *alt = intf->cur_altsetting;
+
+ return sprintf(buf, "usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02Xic%02Xisc%02Xip%02X\n",
+ le16_to_cpu(udev->descriptor.idVendor),
+ le16_to_cpu(udev->descriptor.idProduct),
+ le16_to_cpu(udev->descriptor.bcdDevice),
+ udev->descriptor.bDeviceClass,
+ udev->descriptor.bDeviceSubClass,
+ udev->descriptor.bDeviceProtocol,
+ alt->desc.bInterfaceClass,
+ alt->desc.bInterfaceSubClass,
+ alt->desc.bInterfaceProtocol);
+ } else {
+ return sprintf(buf, "usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02Xic*isc*ip*\n",
+ le16_to_cpu(udev->descriptor.idVendor),
+ le16_to_cpu(udev->descriptor.idProduct),
+ le16_to_cpu(udev->descriptor.bcdDevice),
+ udev->descriptor.bDeviceClass,
+ udev->descriptor.bDeviceSubClass,
+ udev->descriptor.bDeviceProtocol);
+ }
+
+}
+static DEVICE_ATTR(modalias, S_IRUGO, show_modalias, NULL);
+
static struct attribute *intf_attrs[] = {
&dev_attr_bInterfaceNumber.attr,
&dev_attr_bAlternateSetting.attr,
@@ -293,6 +326,7 @@
&dev_attr_bInterfaceClass.attr,
&dev_attr_bInterfaceSubClass.attr,
&dev_attr_bInterfaceProtocol.attr,
+ &dev_attr_modalias.attr,
NULL,
};
static struct attribute_group intf_attr_grp = {

2005-05-13 08:20:05

by Michael Tokarev

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

Greg KH wrote:
[]
> Subject: PCI: add MODALIAS to hotplug event for pci devices
> --- gregkh-2.6.orig/drivers/pci/hotplug.c 2005-05-12 14:28:39.000000000 -0700
> +++ gregkh-2.6/drivers/pci/hotplug.c 2005-05-12 14:28:47.000000000 -0700
> @@ -52,6 +52,16 @@
> if ((buffer_size - length <= 0) || (i >= num_envp))
> return -ENOMEM;
>
> + envp[i++] = scratch;
> + length += scnprintf (scratch, buffer_size - length,
> + "MODALIAS=pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
[]
> Subject: PCI: add modalias sysfs file for pci devices
> --- gregkh-2.6.orig/drivers/pci/pci-sysfs.c 2005-05-12 14:28:25.000000000 -0700
> +++ gregkh-2.6/drivers/pci/pci-sysfs.c 2005-05-12 14:28:40.000000000 -0700
> + return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
> + pci_dev->vendor, pci_dev->device,
> + pci_dev->subsystem_vendor, pci_dev->subsystem_device,
> + (u8)(pci_dev->class >> 16), (u8)(pci_dev->class >> 8),

Just a small note/suggestion... Looks like it's worth to create a common
routine for the two cases. Just to be sure the value in $MODALIAS and in
devices/xx/modalias are the same. I think.

/mjt

2005-05-13 16:05:43

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Fri, May 13, 2005 at 12:19:28PM +0400, Michael Tokarev wrote:
> Greg KH wrote:
> []
> > Subject: PCI: add MODALIAS to hotplug event for pci devices
> > --- gregkh-2.6.orig/drivers/pci/hotplug.c 2005-05-12 14:28:39.000000000 -0700
> > +++ gregkh-2.6/drivers/pci/hotplug.c 2005-05-12 14:28:47.000000000 -0700
> > @@ -52,6 +52,16 @@
> > if ((buffer_size - length <= 0) || (i >= num_envp))
> > return -ENOMEM;
> >
> > + envp[i++] = scratch;
> > + length += scnprintf (scratch, buffer_size - length,
> > + "MODALIAS=pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
> []
> > Subject: PCI: add modalias sysfs file for pci devices
> > --- gregkh-2.6.orig/drivers/pci/pci-sysfs.c 2005-05-12 14:28:25.000000000 -0700
> > +++ gregkh-2.6/drivers/pci/pci-sysfs.c 2005-05-12 14:28:40.000000000 -0700
> > + return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
> > + pci_dev->vendor, pci_dev->device,
> > + pci_dev->subsystem_vendor, pci_dev->subsystem_device,
> > + (u8)(pci_dev->class >> 16), (u8)(pci_dev->class >> 8),
>
> Just a small note/suggestion... Looks like it's worth to create a common
> routine for the two cases. Just to be sure the value in $MODALIAS and in
> devices/xx/modalias are the same. I think.

Sure, if you want to do so go ahead :)

But be careful of the CONFIG_HOTPLUG issue...

It's easier just to leave it as is for now.

thanks,

greg k-h

2005-05-13 23:24:43

by Per Svennerbrandt

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

* On Fri, 13 May 2005, Greg KH ([email protected]) wrote:
> On Mon, May 09, 2005 at 11:13:24PM +0200, Per Svennerbrandt wrote:
> > * Per Liden ([email protected]) wrote:
> > > On Fri, 6 May 2005, Greg KH wrote:
> > >
> > > [...]
> > > > Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> > > > USB hotplug program can be written with a simple one line shell script:
> > > > modprobe $MODALIAS
> > >
> > > Nice, but why not just convert all this to a call to
> > > request_module($MODALIAS)? Seems to me like the natural thing to do.
> >
> > I actually have a pretty hackish proof-of-consept patch that does
> > basicly that, and have been running it on my systems for the past five
> > months or so, if anybody's interested.
> >
> > Along with it I also have a patch witch exports the module aliases for
> > PCI and USB devices through sysfs. With it the "coldplugging" of a
> > system (module wise) can be reduced to pretty much:
> >
> > #!/bin/sh
> >
> > for DEV in /sys/bus/{pci,usb}/devices/*; do
> > modprobe `cat $DEV/modalias`
> > done
>
> Ok, as you never posted your patch, I had to do it myself :)

Oh, crap! Seems like I'm forever doomed to be sitting on my patches for
six months thinking they arn't good enough, only to then repeatedly
getting beaten by couple of hours when finally deciding on submitting
them then... ;) ;)

I guess I'l just have to dedicate more time if I'm ever going to get any
code into the kernel.

> Here's 3 patches that I just added to my trees, and will show up in the
> next -mm release. They create the modalias file for usb and pci
> devices, and add the MODALIAS env variable for the pci hotplug event.

Nice to see this finally getting some attention. Comments below.

> Subject: PCI: add MODALIAS to hotplug event for pci devices
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ---
> drivers/pci/hotplug.c | 10 ++++++++++
> 1 files changed, 10 insertions(+)
>
> --- gregkh-2.6.orig/drivers/pci/hotplug.c 2005-05-12 14:28:39.000000000 -0700
> +++ gregkh-2.6/drivers/pci/hotplug.c 2005-05-12 14:28:47.000000000 -0700
> @@ -52,6 +52,16 @@
> if ((buffer_size - length <= 0) || (i >= num_envp))
> return -ENOMEM;
>
> + envp[i++] = scratch;
> + length += scnprintf (scratch, buffer_size - length,
> + "MODALIAS=pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
> + pdev->vendor, pdev->device,
> + pdev->subsystem_vendor, pdev->subsystem_device,
> + (u8)(pdev->class >> 16), (u8)(pdev->class >> 8),
> + (u8)(pdev->class));
> + if ((buffer_size - length <= 0) || (i >= num_envp))
> + return -ENOMEM;
> +
> envp[i] = NULL;
>
> return 0;

This is pretty much identical to my patch except mine also converts PCI
into using add_hotplug_env_var().

> Subject: PCI: add modalias sysfs file for pci devices
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ---
> drivers/pci/pci-sysfs.c | 12 ++++++++++++
> 1 files changed, 12 insertions(+)
>
> --- gregkh-2.6.orig/drivers/pci/pci-sysfs.c 2005-05-12 14:28:25.000000000 -0700
> +++ gregkh-2.6/drivers/pci/pci-sysfs.c 2005-05-12 14:28:40.000000000 -0700
> @@ -73,6 +73,17 @@
> return (str - buf);
> }
>
> +static ssize_t modalias_show(struct device *dev, char *buf)
> +{
> + struct pci_dev *pci_dev = to_pci_dev(dev);
> +
> + return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
> + pci_dev->vendor, pci_dev->device,
> + pci_dev->subsystem_vendor, pci_dev->subsystem_device,
> + (u8)(pci_dev->class >> 16), (u8)(pci_dev->class >> 8),
> + (u8)(pci_dev->class));
> +}
> +
> struct device_attribute pci_dev_attrs[] = {
> __ATTR_RO(resource),
> __ATTR_RO(vendor),
> @@ -82,6 +93,7 @@
> __ATTR_RO(class),
> __ATTR_RO(irq),
> __ATTR_RO(local_cpus),
> + __ATTR_RO(modalias),
> __ATTR_NULL,
> };

This is *exactly* identical to my patch. :)

>
> Subject: USB: add modalias sysfs file for usb devices
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ---
> drivers/usb/core/sysfs.c | 34 ++++++++++++++++++++++++++++++++++
> 1 files changed, 34 insertions(+)
>
> --- gregkh-2.6.orig/drivers/usb/core/sysfs.c 2005-05-12 14:28:59.000000000 -0700
> +++ gregkh-2.6/drivers/usb/core/sysfs.c 2005-05-12 14:29:05.000000000 -0700
> @@ -286,6 +286,39 @@
> }
> static DEVICE_ATTR(interface, S_IRUGO, show_interface_string, NULL);
>
> +static ssize_t show_modalias(struct device *dev, char *buf)
> +{
> + struct usb_interface *intf;
> + struct usb_device *udev;
> +
> + intf = to_usb_interface(dev);
> + udev = interface_to_usbdev(intf);
> + if (udev->descriptor.bDeviceClass == 0) {
> + struct usb_host_interface *alt = intf->cur_altsetting;
> +
> + return sprintf(buf, "usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02Xic%02Xisc%02Xip%02X\n",
> + le16_to_cpu(udev->descriptor.idVendor),
> + le16_to_cpu(udev->descriptor.idProduct),
> + le16_to_cpu(udev->descriptor.bcdDevice),
> + udev->descriptor.bDeviceClass,
> + udev->descriptor.bDeviceSubClass,
> + udev->descriptor.bDeviceProtocol,
> + alt->desc.bInterfaceClass,
> + alt->desc.bInterfaceSubClass,
> + alt->desc.bInterfaceProtocol);

Are you sure this is correct?

I had problems with alt (intf->cur_altsetting) beeing null and actually
ended up ignoring the interface bits altogether. I'm bretty sure the
above will crash repeatedly on at least some of my machines.

> + } else {
> + return sprintf(buf, "usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02Xic*isc*ip*\n",
> + le16_to_cpu(udev->descriptor.idVendor),
> + le16_to_cpu(udev->descriptor.idProduct),
> + le16_to_cpu(udev->descriptor.bcdDevice),
> + udev->descriptor.bDeviceClass,
> + udev->descriptor.bDeviceSubClass,
> + udev->descriptor.bDeviceProtocol);
> + }
> +
> +}
> +static DEVICE_ATTR(modalias, S_IRUGO, show_modalias, NULL);
> +
> static struct attribute *intf_attrs[] = {
> &dev_attr_bInterfaceNumber.attr,
> &dev_attr_bAlternateSetting.attr,
> @@ -293,6 +326,7 @@
> &dev_attr_bInterfaceClass.attr,
> &dev_attr_bInterfaceSubClass.attr,
> &dev_attr_bInterfaceProtocol.attr,
> + &dev_attr_modalias.attr,
> NULL,
> };
> static struct attribute_group intf_attr_grp = {

So now that I'm not able to submit it toghether with a mixture of other,
at least slightly, related things that I actually *do believe* have a
small possibility of beeing accepted: How do I get my request_modalias
patch in? ;) ;)

Have a nice day!

/Per Svennerbrandt

2005-05-14 05:59:26

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Sat, May 14, 2005 at 01:21:33AM +0200, Per Svennerbrandt wrote:
> * On Fri, 13 May 2005, Greg KH ([email protected]) wrote:
> > Ok, as you never posted your patch, I had to do it myself :)
>
> Oh, crap! Seems like I'm forever doomed to be sitting on my patches for
> six months thinking they arn't good enough, only to then repeatedly
> getting beaten by couple of hours when finally deciding on submitting
> them then... ;) ;)
>
> I guess I'l just have to dedicate more time if I'm ever going to get any
> code into the kernel.

Or just send those patches earlier :)

> This is pretty much identical to my patch except mine also converts PCI
> into using add_hotplug_env_var().

That would be nice to do, but it would have been doing 2 things in one
patch, not good.

> > +static ssize_t show_modalias(struct device *dev, char *buf)
> > +{
> > + struct usb_interface *intf;
> > + struct usb_device *udev;
> > +
> > + intf = to_usb_interface(dev);
> > + udev = interface_to_usbdev(intf);
> > + if (udev->descriptor.bDeviceClass == 0) {
> > + struct usb_host_interface *alt = intf->cur_altsetting;
> > +
> > + return sprintf(buf, "usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02Xic%02Xisc%02Xip%02X\n",
> > + le16_to_cpu(udev->descriptor.idVendor),
> > + le16_to_cpu(udev->descriptor.idProduct),
> > + le16_to_cpu(udev->descriptor.bcdDevice),
> > + udev->descriptor.bDeviceClass,
> > + udev->descriptor.bDeviceSubClass,
> > + udev->descriptor.bDeviceProtocol,
> > + alt->desc.bInterfaceClass,
> > + alt->desc.bInterfaceSubClass,
> > + alt->desc.bInterfaceProtocol);
>
> Are you sure this is correct?

Works for me :)

> I had problems with alt (intf->cur_altsetting) beeing null and actually
> ended up ignoring the interface bits altogether. I'm bretty sure the
> above will crash repeatedly on at least some of my machines.

Please let me know if it does. Did you put the modalias on the
usb_device or the interface? It belongs on the interface, as this patch
does.

cur_altsetting could be NULL pretty early in the initialization phase of
a USB device, but by the time these files are created, it should be fine
(otherwise this same check in the hotplug call would also fail, right?)

> So now that I'm not able to submit it toghether with a mixture of other,
> at least slightly, related things that I actually *do believe* have a
> small possibility of beeing accepted: How do I get my request_modalias
> patch in? ;) ;)

Send them on, let's see what you have, and we can take it from there.

thanks,

greg k-h

2005-05-14 23:02:56

by Michael Tokarev

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

Greg KH wrote:
[]
> Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> USB hotplug program can be written with a simple one line shell script:
> modprobe $MODALIAS

Speaking of which.. may I suggest this:

if [ ! -L /sys/$DEVPATH/driver ]; then
modprobe $MODALIAS
fi

instead? Hopefully, thanks to previous patches in this area, the sysfs
directory has already been populated properly at a time of the hotplug
event, right?

/mjt

2005-05-15 22:41:56

by Per Svennerbrandt

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

* Greg KH ([email protected]) wrote:
> On Sat, May 14, 2005 at 01:21:33AM +0200, Per Svennerbrandt wrote:
> > * On Fri, 13 May 2005, Greg KH ([email protected]) wrote:
> > > Ok, as you never posted your patch, I had to do it myself :)
> >
> > Oh, crap! Seems like I'm forever doomed to be sitting on my patches for
> > six months thinking they arn't good enough, only to then repeatedly
> > getting beaten by couple of hours when finally deciding on submitting
> > them then... ;) ;)
> >
> > I guess I'l just have to dedicate more time if I'm ever going to get any
> > code into the kernel.
>
> Or just send those patches earlier :)

Well, it's not sending the patches that's the hard part. :) It's getting to
the point where you're feeling a 100% confident that the code is a 100%
correct and without side effects that is. And since I'm so inexperienced in
kernel programming getting to that point simply involves reading just
lots and lots of code, and hence ends up taking a lot of time...
But enough about that. :)

Personal TODO list:
# prt-get install aspell
$ code++
$ talk--

> > > +static ssize_t show_modalias(struct device *dev, char *buf)
> > > +{
> > > + struct usb_interface *intf;
> > > + struct usb_device *udev;
> > > +
> > > + intf = to_usb_interface(dev);
> > > + udev = interface_to_usbdev(intf);
> > > + if (udev->descriptor.bDeviceClass == 0) {
> > > + struct usb_host_interface *alt = intf->cur_altsetting;
> > > +
> > > + return sprintf(buf, "usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02Xic%02Xisc%02Xip%02X\n",
> > > + le16_to_cpu(udev->descriptor.idVendor),
> > > + le16_to_cpu(udev->descriptor.idProduct),
> > > + le16_to_cpu(udev->descriptor.bcdDevice),
> > > + udev->descriptor.bDeviceClass,
> > > + udev->descriptor.bDeviceSubClass,
> > > + udev->descriptor.bDeviceProtocol,
> > > + alt->desc.bInterfaceClass,
> > > + alt->desc.bInterfaceSubClass,
> > > + alt->desc.bInterfaceProtocol);
> >
> > Are you sure this is correct?
>
> Works for me :)
>
> > I had problems with alt (intf->cur_altsetting) beeing null and actually
> > ended up ignoring the interface bits altogether. I'm bretty sure the
> > above will crash repeatedly on at least some of my machines.
>
> Please let me know if it does. Did you put the modalias on the
> usb_device or the interface? It belongs on the interface, as this patch
> does.

Here's what I did:

--- linux-2.6.12-rc2/drivers/usb/core/sysfs.c.orig 2005-04-12 14:25:44.000000000 +0200
+++ linux-2.6.12-rc2/drivers/usb/core/sysfs.c 2005-04-12 15:14:24.000000000 +0200
@@ -164,6 +164,22 @@ show_maxchild (struct device *dev, char
}
static DEVICE_ATTR(maxchild, S_IRUGO, show_maxchild, NULL);

+static ssize_t
+show_modalias (struct device *dev, char *buf)
+{
+ struct usb_device *udev = to_usb_device (dev);
+ /* FIXME: Add proper interface support */
+ return sprintf (buf, "usb:v%04Xp%04Xdl%04Xdh%04Xdc%02Xdsc%02Xdp%02Xic*isc*ip*\n",
+ le16_to_cpu(udev->descriptor.idVendor),
+ le16_to_cpu(udev->descriptor.idProduct),
+ le16_to_cpu(udev->descriptor.bcdDevice),
+ udev->descriptor.bcdDevice,
+ udev->descriptor.bDeviceClass,
+ udev->descriptor.bDeviceSubClass,
+ udev->descriptor.bDeviceProtocol);
+}
+static DEVICE_ATTR(modalias, S_IRUGO, show_modalias, NULL);
+
/* Descriptor fields */
#define usb_descriptor_attr_le16(field, format_string) \
static ssize_t \
@@ -211,6 +245,7 @@ static struct attribute *dev_attrs[] = {
&dev_attr_bDeviceSubClass.attr,
&dev_attr_bDeviceProtocol.attr,
&dev_attr_bNumConfigurations.attr,
+ &dev_attr_modalias.attr,
&dev_attr_speed.attr,
&dev_attr_devnum.attr,
&dev_attr_version.attr,

So, yeah, it was on the device and your version is obviously better. It
does, however, have one little "problem" in common with mine:

Plug in GPS.

$ grep MODALIAS /tmp/hotplug.log
MODALIAS=usb:v067Bp2303dl0202dh0202dc00dsc00dp00icFFisc00ip00
^^^^^^^^^^^^^
$ cat /sys/devices/pci0000\:00/0000\:00\:07.2/usb1/1-1/modalias
usb:v067Bp2303dl0202dh0202dc00dsc00dp00ic*isc*ip*
^^^^^^^^^^
> cur_altsetting could be NULL pretty early in the initialization phase of
> a USB device, but by the time these files are created, it should be fine
> (otherwise this same check in the hotplug call would also fail, right?)

> > So now that I'm not able to submit it toghether with a mixture of other,
> > at least slightly, related things that I actually *do believe* have a
> > small possibility of beeing accepted: How do I get my request_modalias
> > patch in? ;) ;)
>
> Send them on, let's see what you have, and we can take it from there.

I'll do that.

Pelle

2005-05-16 19:17:10

by Greg KH

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Sun, May 15, 2005 at 03:02:50AM +0400, Michael Tokarev wrote:
> Greg KH wrote:
> []
> >Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> >USB hotplug program can be written with a simple one line shell script:
> > modprobe $MODALIAS
>
> Speaking of which.. may I suggest this:
>
> if [ ! -L /sys/$DEVPATH/driver ]; then
> modprobe $MODALIAS
> fi

Sure, go ahead and make my script triple in lines :)

But yes, that would work just fine also. I was just trying to show the
simplicity that is now available to us.

thanks,

greg k-h

2005-05-18 07:32:14

by Giuseppe Bilotta

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Wed, 11 May 2005 04:04:11 +0200, Bodo Eggert
<[email protected]> wrote:

> Giuseppe Bilotta <[email protected]> wrote:
>
>> Is there a way to control the order in which modules get loaded? For
>> example, I usually blacklist the parport module and only load it when
>> I need it, thus freeing an IRQ (for audio, IIRC). If parport loads
>> automatically, it grabs the IRQ; if it loads after the IRQ is grabbed
>> already, it'll resort to polled mode. Can these things be controlled
>> without the blacklist?
>
> Documentation/parport.txt

Thanks for saying RTFD nicely :)

> The rest should be configurable in /etc/mod{ules,probe}.conf

Thanks again.

--
Giuseppe "Oblomov" Bilotta

"Da grande lotter? per la pace"
"A me me la compra il mio babbo"
(Altan)
("When I grow up, I will fight for peace"
"I'll have my daddy buy it for me")

2005-05-18 09:27:39

by David Weinehall

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

On Mon, May 09, 2005 at 11:13:24PM +0200, Per Svennerbrandt wrote:

[snip]

> #!/bin/sh
>
> for DEV in /sys/bus/{pci,usb}/devices/*; do
> modprobe `cat $DEV/modalias`
> done

{}-globbing isn't valid for POSIX /bin/sh, it's an extension.
Won't work with sh in busybox, posh, ash, and various other shells.

[snip]

Regards: David Weinehall
--
/) David Weinehall <[email protected]> /) Northern lights wander (\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\) http://www.acc.umu.se/~tao/ (/ Full colour fire (/

2005-05-18 23:01:36

by Per Svennerbrandt

[permalink] [raw]
Subject: Re: [ANNOUNCE] hotplug-ng 002 release

* Per Liden ([email protected]) wrote:
> On Mon, 9 May 2005, Per Svennerbrandt wrote:
>
> > * Per Liden ([email protected]) wrote:
> > > On Fri, 6 May 2005, Greg KH wrote:
> > >
> > > [...]
> > > > Now, with the 2.6.12-rc3 kernel, and a patch for module-init-tools, the
> > > > USB hotplug program can be written with a simple one line shell script:
> > > > modprobe $MODALIAS
> > >
> > > Nice, but why not just convert all this to a call to
> > > request_module($MODALIAS)? Seems to me like the natural thing to do.
> >
> > I actually have a pretty hackish proof-of-consept patch that does
> > basicly that, and have been running it on my systems for the past five
> > months or so, if anybody's interested.
>
> Ah! Please post the patches.

Ok, so I finally decided that it's probably silly of me to hold of a
working patch just because I wanted my first submission to the kernel
to somehow be a marvellous example of ellegancy and not just some
quick hack that I threw toghether as a personal proof-of-consept.

> > Along with it I also have a patch witch exports the module aliases for
> > PCI and USB devices through sysfs. With it the "coldplugging" of a
> > system (module wise) can be reduced to pretty much:
> >
> > #!/bin/sh
> >
> > for DEV in /sys/bus/{pci,usb}/devices/*; do
> > modprobe `cat $DEV/modalias`
> > done
>
> Nice! This is really what coldplugging _should_ look like. Hmm, maybe
> even coldplug the system by request_module()'ing those as well at some
> stage?

Well, the code already generates all the requests necessary. :) The only
problem now of course beeing that it generates those at a "stage" ;) when
userspace usually isn't ready to fullfill those.

I'm currently thinking about maybe making all the requests sleep on a
waitqueue untill the root filesystem becomes availible, shoudn't be too
hard if I remember the code correctly... Could potentialy end up using
a lot of resourses though.

And, yeah, I know this could all be done quite easily with scripts in a
initrd or similar, but that is in fact *exactly* what I'm trying to
avoid here, to reduce complexity and keep thinks as simple as they
possibly can be. I'm not proposing this as a generic solution for
everyone, rather the opposite in fact. I do however beleve there to be
enough demand out there for this particular kind of "special case" to
warrant (optional) support for it in the mainline kernel.

> > (And I actually run exactly that on my laptop, and it works surpricingly
> > well. (Largly due to the fact that the usb-controller is always attached
> > below the pci-bus of course, but it really wouldn't take that much work
> > to make it do the right thing even without relying on any specific
> > ordering/topology))
> >
> > With the above in place my system does all the module-loading that I
> > care about automaticly, and most importantly does so without relying
> > on an /etc/hotplug/ dir with everything and it's grandma in it (or at
> > least thousands of lines of shellscripting).
>
> This is exactly what I'm looking for as well.

For example, I see absolutely no need for a fullblown hotplug system
on my minimalst-userspace router/firewall. I just want the kernel to
be able to load the right modules, on demand, as it boots (and perhaps
when I plug in a usb flashdrive, every third, or so, year).

> > But since the request_modalias() thing seemed as such an obvious thing
> > to do I have been reluctant to submit it fearing that I must have missed
> > some fundamental flaw in it or you guys would have implemented it that
> > way a long time ago? (at least since Rusty rewrote the module
> > loader). Was I wrong*?
> >
> > Greg, Rusty, what do you think?
>
> I'd like to get a better understanding of that as well. Why invent a
> second on demand module loader when we have kmod? The current approach
> feels like a step back to something very similar to the old kerneld.
>
> /Per L

Sorry for the delay, patches to follow.

/ Per Svennerbrandt

2005-05-18 23:01:52

by Per Svennerbrandt

[permalink] [raw]
Subject: [PATCH][RFC] __request_module: fixed argument request_module with waitflag

The following extracts the code in request_module which is responsible
for executing modprobe into a new helper function called __request_module.
This new function takes the wait flag which gets passed down to
call_usermodeheper as an argument, allowing async execution of modprobe.

During the writing of this I've had a bit of a mental struggle about
whether or not maybe call_modprobe is a better name for this and thus
I'm fine with either one if anyone else has a preference.

Signed-off-by: Per Svennerbrandt <[email protected]>

--- linux-2.6.12-rc2/kernel/kmod.c.orig 2005-04-16 19:08:22.000000000 +0200
+++ linux-2.6.12-rc2/kernel/kmod.c 2005-05-12 23:50:00.000000000 +0200
@@ -49,6 +49,49 @@
*/
char modprobe_path[KMOD_PATH_LEN] = "/sbin/modprobe";

+int __request_module(char *name, int wait)
+{
+ int ret;
+ unsigned int max_modprobes;
+ static atomic_t kmod_concurrent = ATOMIC_INIT(0);
+#define MAX_KMOD_CONCURRENT 50 /* Completely arbitrary value - KAO */
+ static int kmod_loop_msg;
+
+ char *argv[] = { modprobe_path, "-q", "--", name, NULL };
+ static char *envp[] = { "HOME=/",
+ "TERM=linux",
+ "PATH=/sbin:/usr/sbin:/bin:/usr/bin",
+ NULL };
+
+ /* If modprobe needs a service that is in a module, we get a recursive
+ * loop. Limit the number of running kmod threads to max_threads/2 or
+ * MAX_KMOD_CONCURRENT, whichever is the smaller. A cleaner method
+ * would be to run the parents of this process, counting how many times
+ * kmod was invoked. That would mean accessing the internals of the
+ * process tables to get the command line, proc_pid_cmdline is static
+ * and it is not worth changing the proc code just to handle this case.
+ * KAO.
+ *
+ * "trace the ppid" is simple, but will fail if someone's
+ * parent exits. I think this is as good as it gets. --RR
+ */
+ max_modprobes = min(max_threads/2, MAX_KMOD_CONCURRENT);
+ atomic_inc(&kmod_concurrent);
+ if (atomic_read(&kmod_concurrent) > max_modprobes) {
+ /* We may be blaming an innocent here, but unlikely */
+ if (kmod_loop_msg++ < 5)
+ printk(KERN_ERR
+ "request_module: runaway loop modprobe %s\n",
+ name);
+ atomic_dec(&kmod_concurrent);
+ return -ENOMEM;
+ }
+ ret = call_usermodehelper(modprobe_path, argv, envp, wait);
+ atomic_dec(&kmod_concurrent);
+ return ret;
+}
+EXPORT_SYMBOL(__request_module);
+
/**
* request_module - try to load a kernel module
* @fmt: printf style format string for the name of the module
@@ -67,50 +110,15 @@ int request_module(const char *fmt, ...)
{
va_list args;
char module_name[MODULE_NAME_LEN];
- unsigned int max_modprobes;
int ret;
- char *argv[] = { modprobe_path, "-q", "--", module_name, NULL };
- static char *envp[] = { "HOME=/",
- "TERM=linux",
- "PATH=/sbin:/usr/sbin:/bin:/usr/bin",
- NULL };
- static atomic_t kmod_concurrent = ATOMIC_INIT(0);
-#define MAX_KMOD_CONCURRENT 50 /* Completely arbitrary value - KAO */
- static int kmod_loop_msg;

va_start(args, fmt);
ret = vsnprintf(module_name, MODULE_NAME_LEN, fmt, args);
va_end(args);
if (ret >= MODULE_NAME_LEN)
return -ENAMETOOLONG;

- /* If modprobe needs a service that is in a module, we get a recursive
- * loop. Limit the number of running kmod threads to max_threads/2 or
- * MAX_KMOD_CONCURRENT, whichever is the smaller. A cleaner method
- * would be to run the parents of this process, counting how many times
- * kmod was invoked. That would mean accessing the internals of the
- * process tables to get the command line, proc_pid_cmdline is static
- * and it is not worth changing the proc code just to handle this case.
- * KAO.
- *
- * "trace the ppid" is simple, but will fail if someone's
- * parent exits. I think this is as good as it gets. --RR
- */
- max_modprobes = min(max_threads/2, MAX_KMOD_CONCURRENT);
- atomic_inc(&kmod_concurrent);
- if (atomic_read(&kmod_concurrent) > max_modprobes) {
- /* We may be blaming an innocent here, but unlikely */
- if (kmod_loop_msg++ < 5)
- printk(KERN_ERR
- "request_module: runaway loop modprobe %s\n",
- module_name);
- atomic_dec(&kmod_concurrent);
- return -ENOMEM;
- }
-
- ret = call_usermodehelper(modprobe_path, argv, envp, 1);
- atomic_dec(&kmod_concurrent);
- return ret;
+ return __request_module(module_name, 1);
}
EXPORT_SYMBOL(request_module);
#endif /* CONFIG_KMOD */
--- linux-2.6.12-rc2/include/linux/kmod.h.orig 2005-03-02 08:37:49.000000000 +0100
+++ linux-2.6.12-rc2/include/linux/kmod.h 2005-05-12 23:53:22.000000000 +0200
@@ -28,8 +28,10 @@
#ifdef CONFIG_KMOD
/* modprobe exit status on success, -ve on error. Return value
* usually useless though. */
+extern int __request_module(char *name, int wait);
extern int request_module(const char * name, ...) __attribute__ ((format (printf, 1, 2)));
#else
+static inline int __request_module(char *name, int wait) { return -ENOSYS; }
static inline int request_module(const char * name, ...) { return -ENOSYS; }
#endif

2005-05-18 23:03:58

by Per Svennerbrandt

[permalink] [raw]
Subject: [PATCH][RFC] request_modalias: MODALIAS based module loading

Ok, so here finally goes:

The following adds a new function called request_modalias to
lib/kobject_uevent.c and makes the kobject hotplug function call it
whenever it gets called in response of an addition.

Warning: The following also contains perhaps the worst abuse of strcmp
ever to be recorded in the history of public mailinglists, so beware!!!

This is the kind of stuff that has the potential for making pretty much
anyone with only even the slightest hint of technical good taste,
myself included, quickly rush for their b-p-b with a horrid expression
on their faces and a thought on their minds about what's becomming of
the world.

On the other hand it also made the patch so nonintrusive that, for a
proof-of-consept kind of thing, I just couldn't resist it. It also has
the added benefit of making the various subsystems virtually "plug and
play": As soon as they start exporting MODALIAS to the hotplug
enviroment request_module will also, if activated, start requesting
modules for them.

Let's hope I'm not prematurely ending the lives of too many brave brown
paper bags out there.

Signed-off-by: Per Svennerbrandt <[email protected]>

--- linux-2.6.12-rc2/lib/kobject_uevent.c.orig 2005-03-02 08:38:09.000000000 +0100
+++ linux-2.6.12-rc2/lib/kobject_uevent.c 2005-05-13 00:13:39.000000000 +0200
@@ -19,6 +19,7 @@
#include <linux/skbuff.h>
#include <linux/netlink.h>
#include <linux/string.h>
+#include <linux/kmod.h>
#include <linux/kobject_uevent.h>
#include <linux/kobject.h>
#include <net/sock.h>
@@ -175,6 +176,26 @@

#endif /* CONFIG_KOBJECT_UEVENT */

+#ifdef CONFIG_REQUEST_MODALIAS
+/**
+ * request_modalias - try to load any modules specified
+ * by MODALIAS in the enviroment
+ * @envp: pointer to an enviroment possibly containing a MODALIAS
+ */
+static void request_modalias(char **envp)
+{
+ int i, len = strlen("MODALIAS=");
+
+ if (envp == NULL) return;
+
+ for (i = 0; envp[i]; i++)
+ if (!strncmp(envp[i], "MODALIAS=", len))
+ __request_module(envp[i] + len, 0);
+}
+
+#else
+static void request_modalias(char **envp) { }
+#endif /* CONFIG_REQUEST_MODALIAS */

#ifdef CONFIG_HOTPLUG
char hotplug_path[HOTPLUG_PATH_LEN] = "/sbin/hotplug";
@@ -297,6 +318,10 @@ void kobject_hotplug(struct kobject *kob
__FUNCTION__, argv[0], argv[1], (unsigned long long)seq,
envp[0], envp[1], envp[2], envp[3], envp[4]);

+ if (action == KOBJ_ADD)
+ /* since we didn't specify MODALIAS here, it must be beyond i */
+ request_modalias(envp + i);
+
send_uevent(action_string, kobj_path, envp, GFP_KERNEL);

if (!hotplug_path[0])

Here's also a patch to the relevant Kconfig to enable it:

--- linux-2.6.12-rc2/init/Kconfig.orig 2005-05-06 23:42:20.000000000 +0200
+++ linux-2.6.12-rc2/init/Kconfig 2005-05-07 12:16:28.000000000 +0200
@@ -454,6 +454,15 @@
runs modprobe with the appropriate arguments, thereby
loading the module if it is available. If unsure, say Y.

+config REQUEST_MODALIAS
+ bool "MODALIAS based module loading"
+ depends on KMOD && HOTPLUG
+ help
+ If you say Y here, the resulting kernel will also be able to
+ load the modules for most types of hardware attached to buses
+ such as PCI, USB and PCMCIA automatically, based on
+ information contained in their "module aliases".
+
config STOP_MACHINE
bool
default y