firmware_class: skip timeout in request_firmware during boot

Javier Pello javier.pello at urjc.es
Wed Jul 25 11:59:50 CDT 2007


> > The patches apply cleanly against 2.6.22. Any suggestions or
> > feedback will be welcome.
> 
> How is this different from request_firmware_nowait()?

request_firmware_nowait ends up calling _request_firmware anyway.
I see a couple of points here:

1. A driver (or whatever) calls request_firmware as a general rule,
and calls request_firmware_nowait in a context where it cannot sleep
(that is what I guess from the documentation), so it is a driver
decision: If it can wait, it calls request_firmware; otherwise, it
calls request_firmware_nowait. But the driver's decision that it
*can* wait does not mean that it *has to*, if waiting will get
nowhere (nothing listening in userspace).

2. Changing a bunch of request_firmware into request_firmware_nowait
wouldn't be the right thing, either. It would mean that the whole
kernel would not be stopped for a minute, but we would still have
an auxiliary thread doing the wait. The symptoms are gone, but the
problem is still there, with a thread in a useless delay when it
can be determined right from the start that it is pointless.

> Kernel coding style is no braces on single-command blocks...
> (here and above).

Oops, sorry about that. I seemed to skip that part from
Documentation/CodingStyle. I've attached a new, corrected version.

Javier


==========
From: Javier Pello <javier.pello at urjc.es>

kobject_uevent: return an error if event was not delivered to userspace

Make kobject_uevent_env return an error to the caller if the event was
not delivered to userspace either via netlink or via uevent_helper.

Signed-off-by: Javier Pello <javier.pello at urjc.es>

---
diff -u linux-2.6.22/lib/kobject_uevent.c linux-2.6.22-p/lib/kobject_uevent.c
--- linux-2.6.22/lib/kobject_uevent.c	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22-p/lib/kobject_uevent.c	2007-07-18 20:17:10.000000000 +0200
@@ -196,7 +196,8 @@
 			}
 
 			NETLINK_CB(skb).dst_group = 1;
-			netlink_broadcast(uevent_sock, skb, 0, 1, GFP_KERNEL);
+			retval = netlink_broadcast(uevent_sock, skb, 0, 1,
+						GFP_KERNEL);
 		}
 	}
 #endif
@@ -208,7 +209,14 @@
 		argv [0] = uevent_helper;
 		argv [1] = (char *)subsystem;
 		argv [2] = NULL;
-		call_usermodehelper (argv[0], argv, envp, 0);
+#if defined(CONFIG_NET)
+		if (retval)
+			retval = call_usermodehelper (argv[0], argv, envp, 0);
+		else
+			call_usermodehelper (argv[0], argv, envp, 0);
+#else
+		retval = call_usermodehelper (argv[0], argv, envp, 0);
+#endif
 	}
 
 exit:

==========
From: Javier Pello <javier.pello at urjc.es>

request_firmware: skip timeout if userspace was not notified

Stop _request_firmware from setting up a timer and waiting for
the firmware image to appear if kobject_uevent returns an error
(meaning userspace was not notified at all).

This prevents useless delays if there is no userspace tool to
provide the firmware image (eg during boot).

Signed-off-by: Javier Pello <javier.pello at urjc.es>

---
diff -u linux-2.6.22/drivers/base/firmware_class.c linux-2.6.22-p/drivers/base/firmware_class.c
--- linux-2.6.22/drivers/base/firmware_class.c	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22-p/drivers/base/firmware_class.c	2007-07-18 20:06:53.000000000 +0200
@@ -420,8 +420,11 @@
 			add_timer(&fw_priv->timeout);
 		}
 
-		kobject_uevent(&f_dev->kobj, KOBJ_ADD);
-		wait_for_completion(&fw_priv->completion);
+		retval = kobject_uevent(&f_dev->kobj, KOBJ_ADD);
+		if (retval)
+			fw_load_abort(fw_priv);
+		else
+			wait_for_completion(&fw_priv->completion);
 		set_bit(FW_STATUS_DONE, &fw_priv->status);
 		del_timer_sync(&fw_priv->timeout);
 	} else
==========




More information about the Kernel-mentors mailing list