From 6e03a201bbe8137487f340d26aa662110e324b20 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Thu, 9 Apr 2009 22:04:07 -0700 Subject: firmware: speed up request_firmware(), v3 Rather than calling vmalloc() repeatedly to grow the firmware image as we receive data from userspace, just allocate and fill individual pages. Then vmap() the whole lot in one go when we're done. A quick test with a 337KiB iwlagn firmware shows the time taken for request_firmware() going from ~32ms to ~5ms after I apply this patch. [v2: define PAGE_KERNEL_RO as PAGE_KERNEL where necessary, use min_t()] [v3: kunmap() takes the struct page *, not the virtual address] Signed-off-by: David Woodhouse Tested-by: Sachin Sant --- drivers/base/firmware_class.c | 129 +++++++++++++++++++++++++++++++++--------- 1 file changed, 103 insertions(+), 26 deletions(-) (limited to 'drivers/base/firmware_class.c') diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index d3a59c688fe..8a267c42762 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -17,7 +17,7 @@ #include #include #include - +#include #include #include "base.h" @@ -45,7 +45,10 @@ struct firmware_priv { struct bin_attribute attr_data; struct firmware *fw; unsigned long status; - int alloc_size; + struct page **pages; + int nr_pages; + int page_array_size; + const char *vdata; struct timer_list timeout; }; @@ -122,6 +125,10 @@ static ssize_t firmware_loading_show(struct device *dev, return sprintf(buf, "%d\n", loading); } +/* Some architectures don't have PAGE_KERNEL_RO */ +#ifndef PAGE_KERNEL_RO +#define PAGE_KERNEL_RO PAGE_KERNEL +#endif /** * firmware_loading_store - set value in the 'loading' control file * @dev: device pointer @@ -141,6 +148,7 @@ static ssize_t firmware_loading_store(struct device *dev, { struct firmware_priv *fw_priv = dev_get_drvdata(dev); int loading = simple_strtol(buf, NULL, 10); + int i; switch (loading) { case 1: @@ -151,13 +159,30 @@ static ssize_t firmware_loading_store(struct device *dev, } vfree(fw_priv->fw->data); fw_priv->fw->data = NULL; + for (i = 0; i < fw_priv->nr_pages; i++) + __free_page(fw_priv->pages[i]); + kfree(fw_priv->pages); + fw_priv->pages = NULL; + fw_priv->page_array_size = 0; + fw_priv->nr_pages = 0; fw_priv->fw->size = 0; - fw_priv->alloc_size = 0; set_bit(FW_STATUS_LOADING, &fw_priv->status); mutex_unlock(&fw_lock); break; case 0: if (test_bit(FW_STATUS_LOADING, &fw_priv->status)) { + vfree(fw_priv->fw->data); + fw_priv->fw->data = vmap(fw_priv->pages, + fw_priv->nr_pages, + 0, PAGE_KERNEL_RO); + if (!fw_priv->fw->data) { + dev_err(dev, "%s: vmap() failed\n", __func__); + goto err; + } + /* Pages will be freed by vfree() */ + fw_priv->pages = NULL; + fw_priv->page_array_size = 0; + fw_priv->nr_pages = 0; complete(&fw_priv->completion); clear_bit(FW_STATUS_LOADING, &fw_priv->status); break; @@ -167,6 +192,7 @@ static ssize_t firmware_loading_store(struct device *dev, dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading); /* fallthrough */ case -1: + err: fw_load_abort(fw_priv); break; } @@ -191,8 +217,28 @@ firmware_data_read(struct kobject *kobj, struct bin_attribute *bin_attr, ret_count = -ENODEV; goto out; } - ret_count = memory_read_from_buffer(buffer, count, &offset, - fw->data, fw->size); + if (offset > fw->size) + return 0; + if (count > fw->size - offset) + count = fw->size - offset; + + ret_count = count; + + while (count) { + void *page_data; + int page_nr = offset >> PAGE_SHIFT; + int page_ofs = offset & (PAGE_SIZE-1); + int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count); + + page_data = kmap(fw_priv->pages[page_nr]); + + memcpy(buffer, page_data + page_ofs, page_cnt); + + kunmap(fw_priv->pages[page_nr]); + buffer += page_cnt; + offset += page_cnt; + count -= page_cnt; + } out: mutex_unlock(&fw_lock); return ret_count; @@ -201,27 +247,39 @@ out: static int fw_realloc_buffer(struct firmware_priv *fw_priv, int min_size) { - u8 *new_data; - int new_size = fw_priv->alloc_size; + int pages_needed = ALIGN(min_size, PAGE_SIZE) >> PAGE_SHIFT; + + /* If the array of pages is too small, grow it... */ + if (fw_priv->page_array_size < pages_needed) { + int new_array_size = max(pages_needed, + fw_priv->page_array_size * 2); + struct page **new_pages; + + new_pages = kmalloc(new_array_size * sizeof(void *), + GFP_KERNEL); + if (!new_pages) { + fw_load_abort(fw_priv); + return -ENOMEM; + } + memcpy(new_pages, fw_priv->pages, + fw_priv->page_array_size * sizeof(void *)); + memset(&new_pages[fw_priv->page_array_size], 0, sizeof(void *) * + (new_array_size - fw_priv->page_array_size)); + kfree(fw_priv->pages); + fw_priv->pages = new_pages; + fw_priv->page_array_size = new_array_size; + } - if (min_size <= fw_priv->alloc_size) - return 0; + while (fw_priv->nr_pages < pages_needed) { + fw_priv->pages[fw_priv->nr_pages] = + alloc_page(GFP_KERNEL | __GFP_HIGHMEM); - new_size = ALIGN(min_size, PAGE_SIZE); - new_data = vmalloc(new_size); - if (!new_data) { - printk(KERN_ERR "%s: unable to alloc buffer\n", __func__); - /* Make sure that we don't keep incomplete data */ - fw_load_abort(fw_priv); - return -ENOMEM; - } - fw_priv->alloc_size = new_size; - if (fw_priv->fw->data) { - memcpy(new_data, fw_priv->fw->data, fw_priv->fw->size); - vfree(fw_priv->fw->data); + if (!fw_priv->pages[fw_priv->nr_pages]) { + fw_load_abort(fw_priv); + return -ENOMEM; + } + fw_priv->nr_pages++; } - fw_priv->fw->data = new_data; - BUG_ON(min_size > fw_priv->alloc_size); return 0; } @@ -258,10 +316,25 @@ firmware_data_write(struct kobject *kobj, struct bin_attribute *bin_attr, if (retval) goto out; - memcpy((u8 *)fw->data + offset, buffer, count); - - fw->size = max_t(size_t, offset + count, fw->size); retval = count; + + while (count) { + void *page_data; + int page_nr = offset >> PAGE_SHIFT; + int page_ofs = offset & (PAGE_SIZE - 1); + int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count); + + page_data = kmap(fw_priv->pages[page_nr]); + + memcpy(page_data + page_ofs, buffer, page_cnt); + + kunmap(fw_priv->pages[page_nr]); + buffer += page_cnt; + offset += page_cnt; + count -= page_cnt; + } + + fw->size = max_t(size_t, offset, fw->size); out: mutex_unlock(&fw_lock); return retval; @@ -277,7 +350,11 @@ static struct bin_attribute firmware_attr_data_tmpl = { static void fw_dev_release(struct device *dev) { struct firmware_priv *fw_priv = dev_get_drvdata(dev); + int i; + for (i = 0; i < fw_priv->nr_pages; i++) + __free_page(fw_priv->pages[i]); + kfree(fw_priv->pages); kfree(fw_priv); kfree(dev); -- cgit v1.2.3-70-g09d2 From 6acf70f078ca42a63397b8b84bf6383b01303009 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 23 Apr 2009 22:31:52 +0800 Subject: driver core: firmware_class: replace kfree(dev) with put_device(dev) against v2.6.30-rc3-next tree. Signed-off-by: Ming Lei Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_class.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/base/firmware_class.c') diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 8a267c42762..2d296b7f703 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -356,7 +356,7 @@ static void fw_dev_release(struct device *dev) __free_page(fw_priv->pages[i]); kfree(fw_priv->pages); kfree(fw_priv); - kfree(dev); + put_device(dev); module_put(THIS_MODULE); } @@ -400,14 +400,16 @@ static int fw_register_device(struct device **dev_p, const char *fw_name, retval = device_register(f_dev); if (retval) { dev_err(device, "%s: device_register failed\n", __func__); - goto error_kfree; + put_device(f_dev); + goto error_kfree1; } *dev_p = f_dev; return 0; error_kfree: - kfree(fw_priv); kfree(f_dev); +error_kfree1: + kfree(fw_priv); return retval; } -- cgit v1.2.3-70-g09d2 From 7fcab099795812a8a08eb3b8c8ddb35c3685045f Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Fri, 29 May 2009 11:33:19 +0800 Subject: driver core: fix documentation of request_firmware_nowait request_firmware_nowait declares it can be called in non-sleep contexts, but kthead_run called by request_firmware_nowait may sleep. So fix its documentation and comment to make callers clear about it. Signed-off-by: Ming Lei Signed-off-by: Greg Kroah-Hartman --- Documentation/firmware_class/README | 3 ++- drivers/base/firmware_class.c | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/base/firmware_class.c') diff --git a/Documentation/firmware_class/README b/Documentation/firmware_class/README index c3480aa66ba..7eceaff63f5 100644 --- a/Documentation/firmware_class/README +++ b/Documentation/firmware_class/README @@ -77,7 +77,8 @@ seconds for the whole load operation. - request_firmware_nowait() is also provided for convenience in - non-user contexts. + user contexts to request firmware asynchronously, but can't be called + in atomic contexts. about in-kernel persistence: diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 2d296b7f703..24277025058 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -617,8 +617,9 @@ request_firmware_work_func(void *arg) * @cont: function will be called asynchronously when the firmware * request is over. * - * Asynchronous variant of request_firmware() for contexts where - * it is not possible to sleep. + * Asynchronous variant of request_firmware() for user contexts where + * it is not possible to sleep for long time. It can't be called + * in atomic contexts. **/ int request_firmware_nowait( -- cgit v1.2.3-70-g09d2 From 976821d756c53b421d7c58a9ed60125016d330aa Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Wed, 27 May 2009 00:49:31 +0200 Subject: firmware: allocate firmware id dynamically The firmware loader has a statically allocated 30 bytes long string for the firmware id (a.k.a. the firmware file name). There is no reason why we couldnt allocate it dynamically, and avoid having restrictions on the firmware names lengths. Signed-off-by: Samuel Ortiz Cc: Marcel Holtmann Cc: Zhu Yi , Cc: John Linville Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_class.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'drivers/base/firmware_class.c') diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 24277025058..3591ba7ff6f 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -40,7 +40,7 @@ static int loading_timeout = 60; /* In seconds */ static DEFINE_MUTEX(fw_lock); struct firmware_priv { - char fw_id[FIRMWARE_NAME_MAX]; + char *fw_id; struct completion completion; struct bin_attribute attr_data; struct firmware *fw; @@ -355,6 +355,7 @@ static void fw_dev_release(struct device *dev) for (i = 0; i < fw_priv->nr_pages; i++) __free_page(fw_priv->pages[i]); kfree(fw_priv->pages); + kfree(fw_priv->fw_id); kfree(fw_priv); put_device(dev); @@ -386,7 +387,13 @@ static int fw_register_device(struct device **dev_p, const char *fw_name, init_completion(&fw_priv->completion); fw_priv->attr_data = firmware_attr_data_tmpl; - strlcpy(fw_priv->fw_id, fw_name, FIRMWARE_NAME_MAX); + fw_priv->fw_id = kstrdup(fw_name, GFP_KERNEL); + if (!fw_priv->fw_id) { + dev_err(device, "%s: Firmware name allocation failed\n", + __func__); + retval = -ENOMEM; + goto error_kfree; + } fw_priv->timeout.function = firmware_class_timeout; fw_priv->timeout.data = (u_long) fw_priv; @@ -401,14 +408,15 @@ static int fw_register_device(struct device **dev_p, const char *fw_name, if (retval) { dev_err(device, "%s: device_register failed\n", __func__); put_device(f_dev); - goto error_kfree1; + goto error_kfree_fw_id; } *dev_p = f_dev; return 0; +error_kfree_fw_id: + kfree(fw_priv->fw_id); error_kfree: kfree(f_dev); -error_kfree1: kfree(fw_priv); return retval; } -- cgit v1.2.3-70-g09d2 From acc0e90fbccbc6e4d48184cba0983ea044e131af Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 2 Jun 2009 15:39:55 -0700 Subject: driver core: fix gcc 4.3.3 warnings about string literals This removes the warning: format not a string literal and no format arguments warnings in the driver core that gcc 4.3.3 complains about. Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 4 ++-- drivers/base/firmware_class.c | 2 +- drivers/base/platform.c | 2 +- drivers/base/sys.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/base/firmware_class.c') diff --git a/drivers/base/core.c b/drivers/base/core.c index 7e8576d7822..4d59975c24a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -875,7 +875,7 @@ int device_add(struct device *dev) * the name, and force the use of dev_name() */ if (dev->init_name) { - dev_set_name(dev, dev->init_name); + dev_set_name(dev, "%s", dev->init_name); dev->init_name = NULL; } @@ -1272,7 +1272,7 @@ struct device *__root_device_register(const char *name, struct module *owner) if (!root) return ERR_PTR(err); - err = dev_set_name(&root->dev, name); + err = dev_set_name(&root->dev, "%s", name); if (err) { kfree(root); return ERR_PTR(err); diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 3591ba7ff6f..ddeb819c8f8 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -399,7 +399,7 @@ static int fw_register_device(struct device **dev_p, const char *fw_name, fw_priv->timeout.data = (u_long) fw_priv; init_timer(&fw_priv->timeout); - dev_set_name(f_dev, dev_name(device)); + dev_set_name(f_dev, "%s", dev_name(device)); f_dev->parent = device; f_dev->class = &firmware_class; dev_set_drvdata(f_dev, fw_priv); diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 59df6290951..81cb01bfc35 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -245,7 +245,7 @@ int platform_device_add(struct platform_device *pdev) if (pdev->id != -1) dev_set_name(&pdev->dev, "%s.%d", pdev->name, pdev->id); else - dev_set_name(&pdev->dev, pdev->name); + dev_set_name(&pdev->dev, "%s", pdev->name); for (i = 0; i < pdev->num_resources; i++) { struct resource *p, *r = &pdev->resource[i]; diff --git a/drivers/base/sys.c b/drivers/base/sys.c index 162355cd745..79a9ae5238a 100644 --- a/drivers/base/sys.c +++ b/drivers/base/sys.c @@ -141,7 +141,7 @@ int sysdev_class_register(struct sysdev_class *cls) cls->kset.kobj.ktype = &ktype_sysdev_class; cls->kset.kobj.kset = system_kset; - retval = kobject_set_name(&cls->kset.kobj, cls->name); + retval = kobject_set_name(&cls->kset.kobj, "%s", cls->name); if (retval) return retval; -- cgit v1.2.3-70-g09d2