From 65710cb6ea315b3ef76a8a3da7be99afcf58d2bb Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Sat, 4 Aug 2012 12:01:16 +0800 Subject: [PATCH] firmware loader: simplify pages ownership transfer This patch doesn't transfer ownership of pages' buffer to the instance of firmware until the firmware loading is completed, which will simplify firmware_loading_store a lot, so help to introduce the following cache_firmware and uncache_firmware mechanism during system suspend-resume cycle. In fact, this patch fixes one bug: if writing data into firmware loader device is bypassed between writting 1 and 0 to 'loading', OOPS will be triggered without the patch. Also handle the vmap failure case, and add some comments to make code more readable. Signed-off-by: Ming Lei Cc: Linus Torvalds Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_class.c | 62 +++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 803cfc1..1cbefcf 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -93,6 +93,8 @@ struct firmware_priv { struct completion completion; struct firmware *fw; unsigned long status; + void *data; + size_t size; struct page **pages; int nr_pages; int page_array_size; @@ -156,9 +158,11 @@ static void fw_dev_release(struct device *dev) struct firmware_priv *fw_priv = to_firmware_priv(dev); int i; + /* free untransfered pages buffer */ for (i = 0; i < fw_priv->nr_pages; i++) __free_page(fw_priv->pages[i]); kfree(fw_priv->pages); + kfree(fw_priv); module_put(THIS_MODULE); @@ -194,6 +198,7 @@ static ssize_t firmware_loading_show(struct device *dev, return sprintf(buf, "%d\n", loading); } +/* firmware holds the ownership of pages */ static void firmware_free_data(const struct firmware *fw) { int i; @@ -237,9 +242,7 @@ static ssize_t firmware_loading_store(struct device *dev, switch (loading) { case 1: - firmware_free_data(fw_priv->fw); - memset(fw_priv->fw, 0, sizeof(struct firmware)); - /* If the pages are not owned by 'struct firmware' */ + /* discarding any previous partial load */ for (i = 0; i < fw_priv->nr_pages; i++) __free_page(fw_priv->pages[i]); kfree(fw_priv->pages); @@ -250,20 +253,6 @@ static ssize_t firmware_loading_store(struct device *dev, break; case 0: if (test_bit(FW_STATUS_LOADING, &fw_priv->status)) { - vunmap(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 are now owned by 'struct firmware' */ - fw_priv->fw->pages = fw_priv->pages; - 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; @@ -273,7 +262,6 @@ 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; } @@ -299,12 +287,12 @@ static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj, ret_count = -ENODEV; goto out; } - if (offset > fw->size) { + if (offset > fw_priv->size) { ret_count = 0; goto out; } - if (count > fw->size - offset) - count = fw->size - offset; + if (count > fw_priv->size - offset) + count = fw_priv->size - offset; ret_count = count; @@ -396,6 +384,7 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj, retval = -ENODEV; goto out; } + retval = fw_realloc_buffer(fw_priv, offset + count); if (retval) goto out; @@ -418,7 +407,7 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj, count -= page_cnt; } - fw->size = max_t(size_t, offset, fw->size); + fw_priv->size = max_t(size_t, offset, fw_priv->size); out: mutex_unlock(&fw_lock); return retval; @@ -504,6 +493,29 @@ static void _request_firmware_cleanup(const struct firmware **firmware_p) *firmware_p = NULL; } +/* transfer the ownership of pages to firmware */ +static int fw_set_page_data(struct firmware_priv *fw_priv) +{ + struct firmware *fw = fw_priv->fw; + + fw_priv->data = vmap(fw_priv->pages, fw_priv->nr_pages, + 0, PAGE_KERNEL_RO); + if (!fw_priv->data) + return -ENOMEM; + + fw->data = fw_priv->data; + fw->pages = fw_priv->pages; + fw->size = fw_priv->size; + + WARN_ON(PFN_UP(fw->size) != fw_priv->nr_pages); + + fw_priv->nr_pages = 0; + fw_priv->pages = NULL; + fw_priv->data = NULL; + + return 0; +} + static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent, long timeout) { @@ -549,8 +561,12 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent, del_timer_sync(&fw_priv->timeout); mutex_lock(&fw_lock); - if (!fw_priv->fw->size || test_bit(FW_STATUS_ABORT, &fw_priv->status)) + if (!fw_priv->size || test_bit(FW_STATUS_ABORT, &fw_priv->status)) retval = -ENOENT; + + /* transfer pages ownership at the last minute */ + if (!retval) + retval = fw_set_page_data(fw_priv); fw_priv->fw = NULL; mutex_unlock(&fw_lock); -- 2.7.4