Re: Firmware class breaks udev

[Hannes Reinecke <hare@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

Kay Sievers wrote:
> On Tue, 2005-03-15 at 14:09 +0100, Hannes Reinecke wrote:
[ .. ]
>>This is not what happens.
>>modprobe will return _after_ device initialisation finished, ie after
>>the firmware is loaded (or not, as the case might be).
>>The firmware event is triggered when modprobe is still running, so udev
>>thinks the device is still initialising and waits until modprobe returns
>>before executing that event.
> 
> Ick, I expected modprobe to return. This is what I saw the last time
> with the ipw2100 driver. 
> 
Well, yes. After the timeout :-(.

>>>>For now I'll be putting in a quick exit for firmware events (ie not wait
>>>>for the devices initialisation to finish) but this is nevertheless _nasty_.
>>>The time I stumbled across a similar problem with the firmware class, I
>>>thought about adding a TIMEOUT= to the hotplug environment which udev
>>>can use to prioritize such events. But we should better replace
>>>class_firmware.
>>>
>>That's what i thought also. (That's why I named the variable 'timeout').
> 
> You have put that into the kernel hotplug env? Or udev itself?
> 
udev. This time I've attached the patch :-).
It might be not a bad idea to have a 'TIMEOUT' variable within the
kernel, then we can set it to a proper value.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux AG				S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

[-- Attachment #2: udev-053-udevd-override-timeout-events.patch --]
[-- Type: text/plain, Size: 1550 bytes --]

diff -pur udev-glibc/udevd.c udev-053/udevd.c
--- udev-glibc/udevd.c	2005-03-15 09:34:08.000000000 +0100
+++ udev-053/udevd.c	2005-03-15 09:37:13.000000000 +0100
@@ -113,6 +113,15 @@ static void msg_queue_insert(struct hotp
 		return;
 	}
 
+	/* Always execute messages with timeout directly */
+	if (msg->timeout > 0) {
+		info("move seq %llu with timeout %u to exec queue",
+		     msg->seqnum, msg->timeout);
+		list_add(&msg->node, &exec_list);
+		run_exec_q = 1;
+		return;
+	}
+
 	/* sort message by sequence number into list */
 	list_for_each_entry_reverse(loop_msg, &msg_list, node) {
 		if (loop_msg->seqnum < msg->seqnum)
@@ -293,6 +303,10 @@ static struct hotplug_msg *running_with_
 	if (msg->devpath == NULL)
 		return NULL;
 
+	/* Skip any events with a timeout set */
+	if (msg->timeout > 0)
+		return NULL;
+
 	list_for_each_entry(loop_msg, &running_list, node) {
 		if (loop_msg->devpath == NULL)
 			continue;
@@ -490,6 +503,9 @@ static struct hotplug_msg *get_udevsend_
 
 		if (strncmp(key, "PHYSDEVPATH=", 12) == 0)
 			msg->physdevpath = &key[12];
+
+		if (strncmp(key, "FIRMWARE=", 9) == 0)
+			msg->timeout = 10;
 	}
 	msg->envp[i++] = "UDEVD_EVENT=1";
 	msg->envp[i] = NULL;
diff -pur udev-glibc/udevd.h udev-053/udevd.h
--- udev-glibc/udevd.h	2005-02-11 20:55:12.000000000 +0100
+++ udev-053/udevd.h	2005-03-15 09:34:50.000000000 +0100
@@ -60,6 +60,7 @@ struct hotplug_msg {
 	char *subsystem;
 	unsigned long long seqnum;
 	char *physdevpath;
+	unsigned int timeout;
 	char *envp[HOTPLUG_NUM_ENVP+1];
 	char envbuf[];
 };