Skip to contentSkip to conversation

hotplug-ng 002 release

ANNOUNCE New Greg KH 15 participants 60 messages May 6, 2005 – May 18, 2005

Subsystems (5)
USB PCI Library code Driver core +1 more subsystems
lore
Share & tools
lore

Greg KH

author
#0 lore raw

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

-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4 opportunities to win big! Highest score wins.NEC IT Guy Games. Play to win an NEC 61 plasma display. Visit http://www.necitguy.com/?r
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#1 lore raw

On Fri, 6 May 2005, Greg KH wrote:

[...]

Quoted 3 lines

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.

[...]

Quoted 2 lines

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

Where can this patch be found?

/Per

-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4 opportunities to win big! Highest score wins.NEC IT Guy Games. Play to win an NEC 61 plasma display. Visit http://www.necitguy.com/?r
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#2 lore raw
Quoted 9 lines · * Per Liden ([email protected]) wrote:

On Fri, 6 May 2005, Greg KH wrote:

[...]

Quoted 3 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4 opportunities to win big! Highest score wins.NEC IT Guy Games. Play to win an NEC 61 plasma display. Visit http://www.necitguy.com/?r
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#3 lore raw
Quoted 8 lines · Per Svennerbrandt
Quoted 7 lines · * Per Liden ([email protected]) wrote:

On Fri, 6 May 2005, Greg KH wrote:

[...]

Quoted 3 lines

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

Quoted 6 lines
Quoted 2 lines

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.

Quoted 9 lines

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?

Quoted 10 lines

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

Quoted 7 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#4 lore raw
Quoted 3 lines · Per Liden

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#5 lore raw
Quoted 6 lines · Greg KH
Quoted 3 lines · Per Liden

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?

Quoted 2 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#6 lore raw
Quoted 10 lines · Per Liden
Quoted 5 lines · Greg KH
Quoted 1 line · Per Liden

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.

Quoted 8 lines

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

Quoted 1 line

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

author
#7 lore raw
In reply to Per Liden
Quoted 10 lines · Per Liden
Quoted 6 lines · Greg KH
Quoted 3 lines · Per Liden

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 :)

Quoted 2 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#8 lore raw
Quoted 8 lines · Greg KH
Quoted 7 lines · Per Liden
Quoted 5 lines · Greg KH
Quoted 3 lines · Per Liden

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.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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.

Quoted 5 lines
Quoted 2 lines

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

author
#9 lore raw
Quoted 10 lines · Per Liden
Quoted 8 lines · Greg KH
Quoted 7 lines · Per Liden
Quoted 5 lines · Greg KH
Quoted 3 lines · Per Liden

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.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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

#10 lore raw
In reply to Per Liden
Quoted 10 lines · * Per Liden ([email protected]) wrote:
Quoted 8 lines · Per Svennerbrandt
Quoted 7 lines · * Per Liden ([email protected]) wrote:

On Fri, 6 May 2005, Greg KH wrote:

[...]

Quoted 3 lines

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

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

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.

Quoted 6 lines
Quoted 6 lines

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

Quoted 7 lines
Quoted 3 lines

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.

Quoted 6 lines
Quoted 6 lines

(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))

Quoted 6 lines
Quoted 4 lines

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

Quoted 6 lines
Quoted 6 lines

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*?

Quoted 7 lines
Quoted 1 line

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_idt12&alloc_id344&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#11 lore raw

RFC PATCH 1/1 __request_module: fixed argument request_module with waitflag

In reply to Per Liden

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.

kernel/kmod.c renamed from kernel/kmod.c.orig +44 -36
@@ -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 */
include/linux/kmod.h renamed from include/linux/kmod.h.orig +2 -0
@@ -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
 

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_idt12&alloc_id344&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#12 lore raw

RFC PATCH 1/1 request_modalias: MODALIAS based module loading

In reply to Per Liden

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.

lib/kobject_uevent.c renamed from lib/kobject_uevent.c.orig +25 -0
@@ -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:

init/Kconfig renamed from init/Kconfig.orig +9 -0
@@ -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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_idt12&alloc_id344&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#13 lore raw

Re: [PATCH][RFC] request_modalias: MODALIAS based module loading

Ah, one more thing.

Quoted 10 lines
lib/kobject_uevent.c renamed from lib/kobject_uevent.c.orig +10 -0
+#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=");
+	

lib/kobject_uevent.c line 198

Quoted 10 lines
lib/kobject_uevent.c renamed from lib/kobject_uevent.c.orig +10 -0
+	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 */

So, why didn't I simply call the non __-ish request_module in there (as i originally intended)?

Well, as it turns out, most subsystem regularly call kobj->hotplug() with all kinds of locks held and thus modprobe needs to be called asynchronously; something which wasn't possible with the old version.

/Per S

(An alternative explanation could be that *that* just would've made the path simply *too* non-intrusive and that it thus was forbidden by the mighty kernel deamons... ;)

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_idt12&alloc_id344&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#14 lore raw
In reply to Per Svennerbrandt
Quoted 4 lines · Per Svennerbrandt

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.

Quoted 5 lines

#!/bin/sh

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

Yes, very nice :)

thaks,

greg k-h

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#15 lore raw
In reply to Per Svennerbrandt
Quoted 19 lines · Per Svennerbrandt
Quoted 9 lines · * Per Liden ([email protected]) wrote:

On Fri, 6 May 2005, Greg KH wrote:

[...]

Quoted 3 lines

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:

Quoted 5 lines

#!/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

---
 drivers/pci/hotplug.c |   10 ++++++++++
 1 files changed, 10 insertions(+)
drivers/pci/hotplug.c +10 -0
@@ -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

---
 drivers/pci/pci-sysfs.c |   12 ++++++++++++
 1 files changed, 12 insertions(+)
drivers/pci/pci-sysfs.c +12 -0
@@ -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

---
 drivers/usb/core/sysfs.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+)
drivers/usb/core/sysfs.c +34 -0
@@ -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 = {

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#16 lore raw

Greg KH wrote:
[]

drivers/pci/hotplug.c line 57

Quoted 10 lines

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

drivers/pci/hotplug.c +3 -0
@@ -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",

[]

drivers/pci/pci-sysfs.c line 83

Quoted 7 lines

Subject: PCI: add modalias sysfs file for pci devices

drivers/pci/pci-sysfs.c +4 -0
+	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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#17 lore raw
Quoted 13 lines · Michael Tokarev

Greg KH wrote:
[]

drivers/pci/hotplug.c line 57

Quoted 10 lines

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

drivers/pci/hotplug.c +3 -0
@@ -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",

[]

Quoted 11 lines

drivers/pci/pci-sysfs.c line 83

Quoted 7 lines

Subject: PCI: add modalias sysfs file for pci devices

drivers/pci/pci-sysfs.c +4 -0
+	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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#18 lore raw
In reply to Greg KH
Quoted 20 lines · * On Fri, 13 May 2005, Greg KH ([email protected]) wrote:
Quoted 19 lines · Per Svennerbrandt
Quoted 9 lines · * Per Liden ([email protected]) wrote:

On Fri, 6 May 2005, Greg KH wrote:

[...]

Quoted 3 lines

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:

Quoted 7 lines
Quoted 5 lines

#!/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.

Quoted 3 lines

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.

Quoted 17 lines

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

---
 drivers/pci/hotplug.c |   10 ++++++++++
 1 files changed, 10 insertions(+)
drivers/pci/hotplug.c +3 -0
@@ -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",

drivers/pci/hotplug.c line 67

Quoted 10 lines
drivers/pci/hotplug.c +7 -0
+			    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().

Quoted 29 lines

Subject: PCI: add modalias sysfs file for pci devices

---
 drivers/pci/pci-sysfs.c |   12 ++++++++++++
 1 files changed, 12 insertions(+)
drivers/pci/pci-sysfs.c +11 -0
@@ -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),

drivers/pci/pci-sysfs.c line 98

Quoted 6 lines
drivers/pci/pci-sysfs.c +1 -0
@@ -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. :)

Quoted 25 lines

Subject: USB: add modalias sysfs file for usb devices

---
 drivers/usb/core/sysfs.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+)
drivers/usb/core/sysfs.c +10 -0
@@ -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;
+

drivers/usb/core/sysfs.c line 308

Quoted 10 lines
drivers/usb/core/sysfs.c +10 -0
+		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.

Quoted 17 lines
drivers/usb/core/sysfs.c +13 -0
+ 	} 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,

drivers/usb/core/sysfs.c line 332

Quoted 7 lines
drivers/usb/core/sysfs.c +1 -0
@@ -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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#19 lore raw
Quoted 10 lines · Per Svennerbrandt
Quoted 1 line · * 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 :)

Quoted 2 lines

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.

Quoted 14 lines
Quoted 14 lines
drivers/usb/core/sysfs.c +14 -0
+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),
Quoted 8 lines

drivers/usb/core/sysfs.c line 308

Quoted 6 lines
drivers/usb/core/sysfs.c +6 -0
+			       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 :)

Quoted 3 lines

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?)

Quoted 4 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#20 lore raw
Quoted 9 lines · * Greg KH ([email protected]) wrote:
Quoted 8 lines · Per Svennerbrandt
Quoted 1 line · * 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... ;) ;)

Quoted 4 lines
Quoted 2 lines

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--
Quoted 25 lines
Quoted 14 lines
Quoted 14 lines
(quoted diff) +14 -0
+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),
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+			       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 :)

Quoted 7 lines
Quoted 3 lines

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:

drivers/usb/core/sysfs.c renamed from drivers/usb/core/sysfs.c.orig +17 -0
@@ -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*
                                       ^^^^^^^^^^
Quoted 10 lines

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?)

Quoted 4 lines

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

#21 lore raw
In reply to Per Svennerbrandt

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

[snip]

Quoted 5 lines

#!/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

Signature · 14 lines
 /) 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           (/


-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_idt12&alloc_id344&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
author
#22 lore raw
In reply to Per Liden
Quoted 9 lines · Per Liden

On Fri, 6 May 2005, Greg KH wrote:

[...]

Quoted 3 lines

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.

Quoted 5 lines

[...]

Quoted 2 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4 opportunities to win big! Highest score wins.NEC IT Guy Games. Play to win an NEC 61 plasma display. Visit http://www.necitguy.com/?r
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#23 lore raw
Quoted 8 lines · Greg KH
Quoted 7 lines · Per Liden

On Fri, 6 May 2005, Greg KH wrote:

[...]

Quoted 3 lines

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

Quoted 6 lines
Quoted 2 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#24 lore raw
Quoted 17 lines · Per Liden
Quoted 8 lines · Greg KH
Quoted 7 lines · Per Liden

On Fri, 6 May 2005, Greg KH wrote:

[...]

Quoted 3 lines

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

Quoted 6 lines
Quoted 2 lines

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.

Quoted 5 lines

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

Quoted 3 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#25 lore raw
In reply to Greg KH
Quoted 2 lines · Greg KH

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

??

Rusty.

Signature · 13 lines
A bad analogy is like a leaky screwdriver -- Richard Braakman



-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4
opportunities to win big! Highest score wins.NEC IT Guy Games. Play to
win an NEC 61 plasma display. Visit http://www.necitguy.com/?r 
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
author
#26 lore raw
Quoted 5 lines · Rusty Russell
Quoted 2 lines · Greg KH

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

From [email protected] Fri Mar 4 13:16:17 2005
Return-Path: <[email protected]>
Delivered-To: unknown
Received: from kroah.com ([216.218.225.136]) by kroah.com for <[email protected]>; Fri, 4 Mar 2005 13:09:32 -0800
Received: from linta.de ([213.239.214.66]) by kroah.com for <[email protected]>; Fri, 4 Mar 2005 13:09:28 -0800
Received: (qmail 26925 invoked by uid 1000); 4 Mar 2005 21:09:27 -0000
Date: Fri, 4 Mar 2005 22:09:27 +0100
From: Dominik Brodowski <[email protected]>
To: [email protected], [email protected]
Subject: [PATCH] module-init-tools - modprobe: load _all_ aliases instead of only one
Message-ID: <[email protected]>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.5.6+20040907i
X-SpamProbe: GOOD 0.0000021 44092103930514f517776d6bcb20e468
Status: RO
Content-Length: 7103
Lines: 241

This patch compiles, it seems to work for the purposes I use modprobe for locally, but please review it thoroughly.

Thanks,
Dominik

hotplug-ng as well as pcmcia's hotplug agents require that "modprobe" called with an alias string load _all_ modules instead of just one match. The attached patch tries to implement it.

modprobe.c renamed from modprobe.c.original +80 -51
@@ -950,6 +950,21 @@
 	return strsep(string, delim);
 }
 
+struct alias_module {
+	struct alias_module *next;
+	char * name;
+};
+
+static int add_alias_module(char *name, struct alias_module **alias_module)
+{
+	struct alias_module *new = NOFAIL(malloc(sizeof(struct alias_module)));
+	new->next = *alias_module;
+	new->name = NOFAIL(strdup(name));
+	*alias_module = new;
+
+	return 0;
+}
+
 /* Recursion */
 static int read_config(const char *filename,
 		       const char *name,
@@ -957,7 +972,8 @@
 		       int removing,
 		       struct module_options **options,
 		       struct module_command **commands,
-		       char **alias);
+		       struct alias_module **alias);
+
 
 /* FIXME: Maybe should be extended to "alias a b [and|or c]...". --RR */
 static int read_config_file(const char *filename,
@@ -966,7 +982,7 @@
 			    int removing,
 			    struct module_options **options,
 			    struct module_command **commands,
-			    char **alias)
+			    struct alias_module **alias)
 {
 	char *line;
 	unsigned int linenum = 0;
@@ -996,9 +1012,9 @@
 			if (!wildcard || !realname)
 				grammar(cmd, filename, linenum);
 			else if (fnmatch(wildcard,name,0) == 0)
-				*alias = NOFAIL(strdup(realname));
+				add_alias_module(realname, alias);
 		} else if (strcmp(cmd, "include") == 0) {
-			char *newalias = NULL, *newfilename;
+			char *newfilename;
 
 			newfilename = strsep_skipspace(&ptr, "\t ");
 			if (!newfilename)
@@ -1006,15 +1022,10 @@
 			else {
 				if (!read_config(newfilename, name,
 						 dump_only, removing,
-						 options, commands, &newalias))
+						 options, commands, alias))
 					warn("Failed to open included"
 					      " config file %s: %s\n",
 					      newfilename, strerror(errno));
-
-				/* Files included override aliases,
-				   etc that was already set ... */
-				if (newalias)
-					*alias = newalias;
 			}
 		} else if (strcmp(cmd, "options") == 0) {
 			modname = strsep_skipspace(&ptr, "\t ");
@@ -1060,7 +1071,7 @@
 		       int removing,
 		       struct module_options **options,
 		       struct module_command **commands,
-		       char **alias)
+		       struct alias_module **alias)
 {
 	DIR *dir;
 
@@ -1103,7 +1114,7 @@
 				 int removing,
 				 struct module_options **options,
 				 struct module_command **commands,
-				 char **alias)
+				 struct alias_module **alias)
 {
 	unsigned int i;
 
@@ -1226,6 +1237,48 @@
 	return 0;
 }
 
+#define HANDLE_ONE_MODULE()						\
+	if (list_empty(&list)) {					\
+		/* The dependencies have to be real modules, but	\
+		   handle case where the first is completely bogus. */	\
+		command = find_command(modname, commands);		\
+		if (command && !ignore_commands) {			\
+			do_command(modname, command, verbose, dry_run,	\
+				   fatal, remove ? "remove":"install");	\
+			continue;					\
+		}							\
+		if (unknown_silent)					\
+			exit(1);					\
+		error("Module %s not found.\n", modname);		\
+		continue;						\
+	}								\
+									\
+	if (remove)							\
+		rmmod(&list, newname, first_time, error, dry_run,	\
+		      verbose, commands, ignore_commands, 0);		\
+	else								\
+		insmod(&list, NOFAIL(strdup(optstring)), newname,	\
+		       first_time, error, dry_run, verbose, modoptions,	\
+		       commands, ignore_commands, ignore_proc,		\
+		       strip_vermagic, strip_modversion);		\
+	free(modname);
+
+#define HANDLE_ALL_ALIASES()						\
+	while (alias_module) {						\
+		struct alias_module *next = alias_module->next;		\
+		char *modname = alias_module->name;			\
+									\
+		optstring = add_extra_options(modulearg, optstring,	\
+					      modoptions);		\
+		read_depends(dirname, modname, &list);			\
+									\
+		HANDLE_ONE_MODULE();					\
+									\
+		free(alias_module);					\
+		alias_module = next;					\
+	}
+
+
 int main(int argc, char *argv[])
 {
 	struct utsname buf;
@@ -1391,7 +1444,7 @@
 	if (dump_only) {
 		struct module_command *commands = NULL;
 		struct module_options *modoptions = NULL;
-		char *a = NULL;
+		struct alias_module *a = NULL;
 
 		read_toplevel_config(config, "", 1, 0,
 				     &modoptions, &commands, &a);
@@ -1415,27 +1468,26 @@
 		LIST_HEAD(list);
 		char *modulearg = argv[optind + i];
 		char *modname = NULL;
+		struct alias_module *alias_module = NULL;
 
 		/* Convert name we are looking for */
 		underscores(modulearg);
 
 		/* Returns the resolved alias, options */
 		read_toplevel_config(config, modulearg, 0,
-				     remove, &modoptions, &commands, &modname);
+				     remove, &modoptions, &commands, &alias_module);
 
 		/* No luck?  Try symbol names, if starts with symbol:. */
-		if (!modname
+		if (!alias_module
 		    && strncmp(modulearg, "symbol:", strlen("symbol:")) == 0)
 			read_config(symfilename, modulearg, 0,
-				    remove, &modoptions, &commands, &modname);
+				    remove, &modoptions, &commands, &alias_module);
 
-		/* If we have an alias, gather any options associated with it
+
+		/* If we have aliases, gather any options associated with it
 		   (needs to happen after parsing complete). */
-		if (modname) {
-		got_modname:
-			optstring = add_extra_options(modulearg, optstring,
-						      modoptions);
-			read_depends(dirname, modname, &list);
+		if (alias_module) {
+			HANDLE_ALL_ALIASES();
 		} else {
 			read_depends(dirname, modulearg, &list);
 			/* We don't allow canned aliases to override
@@ -1446,37 +1498,14 @@
 			    && read_config(aliasfilename,
 					   modulearg, 0,
 					   remove, &modoptions,
-					   &commands, &modname)
-			    && modname)
-				goto got_modname;
-
-			modname = strdup(modulearg);
-		}
-
-		if (list_empty(&list)) {
-			/* The dependencies have to be real modules, but
-			   handle case where the first is completely bogus. */
-			command = find_command(modname, commands);
-			if (command && !ignore_commands) {
-				do_command(modname, command, verbose, dry_run,
-					   fatal, remove ? "remove":"install");
-				continue;
+					   &commands, &alias_module)
+			    && alias_module) {
+				HANDLE_ALL_ALIASES();
+			} else {
+				modname = strdup(modulearg);
+				HANDLE_ONE_MODULE();
 			}
-			if (unknown_silent)
-				exit(1);
-			error("Module %s not found.\n", modname);
-			continue;
 		}
-
-		if (remove)
-			rmmod(&list, newname, first_time, error, dry_run,
-			      verbose, commands, ignore_commands, 0);
-		else
-			insmod(&list, NOFAIL(strdup(optstring)), newname,
-			       first_time, error, dry_run, verbose, modoptions,
-			       commands, ignore_commands, ignore_proc,
-			       strip_vermagic, strip_modversion);
-		free(modname);
 	}
 
 	free(dirname);
#27 lore raw
Quoted 8 lines · Greg KH
Quoted 5 lines · Rusty Russell
Quoted 2 lines · Greg KH

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.

Signature · 13 lines
A bad analogy is like a leaky screwdriver -- Richard Braakman



-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#28 lore raw
Quoted 3 lines · Rusty Russell

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.

Signature · 2 lines
ciao,
Marco
#29 lore raw
Quoted 9 lines · Marco d'Itri
Quoted 5 lines · Rusty Russell

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.

Quoted 6 lines

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.

Signature · 13 lines
Alexander E. Patrakov



-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#30 lore raw
Quoted 2 lines · Alexander E. Patrakov

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.

Signature · 13 lines
ciao,
Marco


-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
author
#31 lore raw
Quoted 4 lines · Marco d'Itri
Quoted 2 lines · Alexander E. Patrakov

Why not this or something similar (e.g. I want to blacklist the xxx and yyy modules)? (note, untested)

Nice, I like it.

Quoted 2 lines

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

Then we will just have to find out :)

Quoted 3 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#32 lore raw
Quoted 2 lines · Greg KH

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#33 lore raw
Quoted 6 lines · Lee Revell
Quoted 2 lines · Greg KH

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#34 lore raw
Quoted 3 lines · Greg KH
Quoted 2 lines

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.

Signature · 2 lines
ciao,
Marco
#35 lore raw
In reply to Greg KH
Quoted 3 lines · Greg KH
Quoted 2 lines
Quoted 2 lines

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.

Quoted 3 lines
Quoted 2 lines

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.

Quoted 6 lines
Quoted 3 lines

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.

Signature · 2 lines
ciao,
Marco
author
#36 lore raw
Quoted 6 lines · Marco d'Itri
Quoted 3 lines · Greg KH
Quoted 2 lines
Quoted 2 lines

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?

Quoted 5 lines
Quoted 3 lines
Quoted 2 lines

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?

Quoted 10 lines
Quoted 6 lines
Quoted 3 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#37 lore raw
Quoted 7 lines · Greg KH
Quoted 5 lines
Quoted 3 lines
Quoted 2 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#38 lore raw
In reply to Greg KH
Quoted 6 lines · Greg KH
Quoted 5 lines
Quoted 3 lines
Quoted 2 lines

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

Quoted 6 lines
Quoted 4 lines

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.

Signature · 2 lines
ciao,
Marco
#39 lore raw
Quoted 10 lines · Marco d'Itri
Quoted 6 lines · Greg KH
Quoted 4 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#40 lore raw

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&m0994425816837
Perhaps you could have a look at it?

Regards,
Erik

modprobe.c +126 -0
diff -urN module-init-tools-3.2-pre4/modprobe.c module-init-tools-3.2-pre4-new/modprobe.c
@@ -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);

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#41 lore raw

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.

Signature · 2 lines
ciao,
Marco
author
#42 lore raw
Quoted 5 lines · Marco d'Itri

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#43 lore raw
In reply to Erik van Konijnenburg
Quoted 4 lines · Erik van Konijnenburg

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.

Quoted 1 line

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

Signature · 13 lines
A bad analogy is like a leaky screwdriver -- Richard Braakman



-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#44 lore raw
Quoted 6 lines · Rusty Russell
Quoted 4 lines · Erik van Konijnenburg

Hi,

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

Quoted 7 lines

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.

Quoted 4 lines
Quoted 1 line

* 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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#45 lore raw
Quoted 4 lines · Erik van Konijnenburg

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

Quoted 8 lines

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.

Signature · 13 lines
A bad analogy is like a leaky screwdriver -- Richard Braakman



-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#46 lore raw
Quoted 8 lines · Rusty Russell
Quoted 4 lines · Erik van Konijnenburg

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

Quoted 9 lines
Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 2 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#47 lore raw
Quoted 3 lines · Erik van Konijnenburg

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.

Signature · 13 lines
A bad analogy is like a leaky screwdriver -- Richard Braakman



-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#48 lore raw
Quoted 2 lines · Rusty Russell

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.

Signature · 2 lines
ciao,
Marco
#49 lore raw
Quoted 5 lines · Marco d'Itri
Quoted 2 lines · Rusty Russell

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

modprobe.c +81 -17
@@ -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) {

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#50 lore raw
Quoted 5 lines · Erik van Konijnenburg
Quoted 4 lines · Marco d'Itri
Quoted 2 lines · Rusty Russell

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

Quoted 7 lines
Quoted 1 line

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.

doc/modprobe.conf.sgml +16 -0
diff --exclude tests -urN module-init-tools-3.2-pre4/doc/modprobe.conf.sgml module-init-tools-3.2-pre5/doc/modprobe.conf.sgml
@@ -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>
modprobe.c +81 -17

204 lines not on this page · raw message

Signature · 13 lines
A bad analogy is like a leaky screwdriver -- Richard Braakman



-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#51 lore raw
Quoted 4 lines · Rusty Russell

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#52 lore raw
In reply to Greg KH
Quoted 2 lines · Greg KH

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?

Signature · 4 lines
Giuseppe "Oblomov" Bilotta

"I'm never quite so stupid
 as when I'm being smart" --Linus van Pelt
#53 lore raw
In reply to Greg KH
Quoted 6 lines · Greg KH
Quoted 4 lines · Marco d'Itri
Quoted 2 lines · Alexander E. Patrakov

Why not this or something similar (e.g. I want to blacklist the xxx and yyy modules)? (note, untested)

Quoted 6 lines

Nice, I like it.

Quoted 2 lines

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.

Signature · 13 lines
A bad analogy is like a leaky screwdriver -- Richard Braakman



-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#54 lore raw
Quoted 4 lines · Rusty Russell

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

Signature · 2 lines
ciao,
Marco
#55 lore raw
Quoted 11 lines · Marco d'Itri
Quoted 4 lines · Rusty Russell

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.

Signature · 13 lines
A bad analogy is like a leaky screwdriver -- Richard Braakman



-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#56 lore raw
In reply to Greg KH
Quoted 6 lines · Erik van Konijnenburg

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.

Quoted 8 lines · Greg KH
Quoted 5 lines · Marco d'Itri

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.

And i second this. It does not matter if modprobe was invoked via kernel or not. The blacklist has to be used if there is a chance that a wrong module could be loaded. And that happens just if you are running modprobe on an alias.
=> use blacklist if the module name was found in the kernels alias
table.

Signature · 14 lines
ciao, christian

�眠�足���仕事�敵�。


-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&opÌk
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#57 lore raw
In reply to Greg KH

Greg KH wrote:
[]

Quoted 3 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_ids93&alloc_id281&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

author
#58 lore raw
Quoted 11 lines · Michael Tokarev

Greg KH wrote:
[]

Quoted 3 lines

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

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_idt12&alloc_id344&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#59 lore raw
In reply to Greg KH
Quoted 3 lines
Quoted 3 lines

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

Do not forget about /etc/hotplug.d/*/

Signature · 2 lines
ciao,
Marco