Re: [PATCH] firmware: speed up request_firmware()

[Clemens Ladisch]

Ira Snyder wrote:
> I didn't want to change an existing kernel interface, so I just made
> the easiest change that worked for me.

Well, userspace does know the actual size of the image, so I see no
reason why it shouldn't be able to tell the kernel about it beforehand.

> I'd be happy to test patches anyone comes up with.

=====

This adds a data_size attribute to the firmware loading device so that
userspace can tell us about the firmware image size.  This allows us to
preallocate the buffer with the final size, thus avoiding reallocating
the buffer for every page of data as it comes in.

Signed-off-by: Clemens Ladisch <clemens@xxxxxxxxxx>

--- linux-2.6.orig/Documentation/firmware_class/README
+++ linux-2.6/Documentation/firmware_class/README
@@ -21,7 +21,7 @@
  kernel(driver): calls request_firmware(&fw_entry, $FIRMWARE, device)
 
  userspace:
- 	- /sys/class/firmware/xxx/{loading,data} appear.
+ 	- /sys/class/firmware/xxx/{loading,data,data_size} appear.
 	- hotplug gets called with a firmware identifier in $FIRMWARE
 	  and the usual hotplug environment.
 		- hotplug: echo 1 > /sys/class/firmware/xxx/loading
@@ -29,11 +29,13 @@
  kernel: Discard any previous partial load.
 
  userspace:
+		- hotplug: echo ... > /sys/class/firmware/xxx/data_size
 		- hotplug: cat appropriate_firmware_image > \
 					/sys/class/firmware/xxx/data
 
- kernel: grows a buffer in PAGE_SIZE increments to hold the image as it
-	 comes in.
+ kernel: Copies the firmware image into a buffer of the specified size.
+	 If the image is larger, the buffer automatically grows in
+	 PAGE_SIZE increments.
 
  userspace:
 		- hotplug: echo 0 > /sys/class/firmware/xxx/loading
@@ -60,6 +62,9 @@
 
 	HOTPLUG_FW_DIR=/usr/lib/hotplug/firmware/
 
+	if [ -e /sys/$DEVPATH/data_size ]; then
+		stat -c %s $HOTPLUG_FW_DIR/$FIRMWARE > /sys/$DEVPATH/data_size
+	fi
 	echo 1 > /sys/$DEVPATH/loading
 	cat $HOTPLUG_FW_DIR/$FIRMWARE > /sysfs/$DEVPATH/data
 	echo 0 > /sys/$DEVPATH/loading
@@ -73,6 +78,9 @@
  - firmware_data_read() and firmware_loading_show() are just provided
    for testing and completeness, they are not called in normal use.
 
+ - /sys/class/firmware/xxx/data_size is optional for compatibility with
+   older kernels.
+
  - There is also /sys/class/firmware/timeout which holds a timeout in
    seconds for the whole load operation.
 
--- linux-2.6.orig/Documentation/firmware_class/hotplug-script
+++ linux-2.6/Documentation/firmware_class/hotplug-script
@@ -6,6 +6,9 @@
 
 HOTPLUG_FW_DIR=/usr/lib/hotplug/firmware/
 
+if [ -e /sys/$DEVPATH/data_size ]; then
+	stat -c %s $HOTPLUG_FW_DIR/$FIRMWARE > /sys/$DEVPATH/data_size
+fi
 echo 1 > /sys/$DEVPATH/loading
 cat $HOTPLUG_FW_DIR/$FIRMWARE > /sys/$DEVPATH/data
 echo 0 > /sys/$DEVPATH/loading
--- linux-2.6.orig/drivers/base/firmware_class.c
+++ linux-2.6/drivers/base/firmware_class.c
@@ -46,6 +46,7 @@ struct firmware_priv {
 	struct firmware *fw;
 	unsigned long status;
 	int alloc_size;
+	int size_hint;
 	struct timer_list timeout;
 };
 
@@ -114,6 +115,32 @@ static struct class firmware_class = {
 	.dev_release	= fw_dev_release,
 };
 
+static ssize_t firmware_data_size_show(struct device *dev,
+				       struct device_attribute *attr, char *buf)
+{
+	struct firmware_priv *fw_priv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", fw_priv->size_hint);
+}
+
+static ssize_t firmware_data_size_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct firmware_priv *fw_priv = dev_get_drvdata(dev);
+	long value;
+	int err;
+
+	err = strict_strtol(buf, 10, &value);
+	if (err)
+		return err;
+	fw_priv->size_hint = value;
+	return count;
+}
+
+static DEVICE_ATTR(data_size, 0644,
+		   firmware_data_size_show, firmware_data_size_store);
+
 static ssize_t firmware_loading_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
@@ -207,6 +234,7 @@ fw_realloc_buffer(struct firmware_priv *
 	if (min_size <= fw_priv->alloc_size)
 		return 0;
 
+	min_size = max(min_size, fw_priv->size_hint);
 	new_size = ALIGN(min_size, PAGE_SIZE);
 	new_data = vmalloc(new_size);
 	if (!new_data) {
@@ -359,6 +387,12 @@ static int fw_setup_device(struct firmwa
 		goto error_unreg;
 	}
 
+	retval = device_create_file(f_dev, &dev_attr_data_size);
+	if (retval) {
+		dev_err(device, "%s: device_create_file failed\n", __func__);
+		goto error_unreg;
+	}
+
 	retval = device_create_file(f_dev, &dev_attr_loading);
 	if (retval) {
 		dev_err(device, "%s: device_create_file failed\n", __func__);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/