From: Daniel Wagner <[email protected]> The firmware user helper code tracks the current state of the loading process via an member of struct firmware_buf and a completion. Let's encapsulate this simple state machine into struct fw_status.
We don't need the fw_lock to protect the state machine anymore. First of all we don't do any RWM operations instead we do either a write or read to the status variable. Reading the status variable via READ_ONCE ensures we see the new state whenever we get woken. We are not allowed to use the complete_all() more than once so we need to filter out all state transition but the last. That is okay since only the final state DONE|ABORTED is of interest. Besides that we also safe a few bytes: cris etrax-100lx_defconfig $ bloat-o-meter firmware_class.o.old firmware_class.o.new add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-234 (-234) function old new delta _request_firmware 1228 994 -234 Total: Before=2179, After=1945, chg -10.74% x86_64 FW_LOADER !FW_LOADER_USER_HELPER_FALLBACK $ bloat-o-meter firmware_class.o.old firmware_class.o.new add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-194 (-194) function old new delta _request_firmware 2118 1924 -194 Total: Before=6811, After=6617, chg -2.85% x86_64 FW_LOADER FW_LOADER_USER_HELPER_FALLBACK $ bloat-o-meter firmware_class.o.old firmware_class.o.new add/remove: 0/0 grow/shrink: 2/5 up/down: 4/-193 (-189) function old new delta fw_shutdown_notify 81 83 +2 firmware_data_read 162 164 +2 fw_pm_notify 396 394 -2 firmware_loading_store 458 441 -17 firmware_data_write 504 475 -29 firmware_loading_show 92 61 -31 _request_firmware 2931 2817 -114 Total: Before=10328, After=10139, chg -1.83% Note the variable loading_timeout and the inline function firmware_loadint_timeout() are used also when no user helper is enabled. So they need to be moved out side of the ifdef section. Signed-off-by: Daniel Wagner <[email protected]> Cc: Ming Lei <[email protected]> Cc: Luis R. Rodriguez <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> --- drivers/base/firmware_class.c | 158 +++++++++++++++++++++++++----------------- 1 file changed, 93 insertions(+), 65 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 22d1760..d3dcf87 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -91,19 +91,88 @@ static inline bool fw_is_builtin_firmware(const struct firmware *fw) } #endif +static int loading_timeout = 60; /* In seconds */ + +static inline long firmware_loading_timeout(void) +{ + return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET; +} + +#ifdef CONFIG_FW_LOADER_USER_HELPER + enum { + FW_STATUS_UNKNOWN, FW_STATUS_LOADING, FW_STATUS_DONE, - FW_STATUS_ABORT, + FW_STATUS_ABORTED, }; -static int loading_timeout = 60; /* In seconds */ +struct fw_status { + unsigned long status; + struct completion completion; +}; -static inline long firmware_loading_timeout(void) +static void fw_status_init(struct fw_status *fw_st) { - return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET; + fw_st->status = FW_STATUS_UNKNOWN; + init_completion(&fw_st->completion); +} + +static unsigned long __fw_status_get(struct fw_status *fw_st) +{ + return READ_ONCE(fw_st->status); +} + +static int fw_status_wait_timeout(struct fw_status *fw_st, long timeout) +{ + unsigned long status; + int err; + + err = wait_for_completion_interruptible_timeout(&fw_st->completion, + timeout); + status = READ_ONCE(fw_st->status); + if (err == 0 && status == FW_STATUS_ABORTED) + return -ENOENT; + + return err; +} + +static void __fw_status_set(struct fw_status *fw_st, + unsigned long status) +{ + WRITE_ONCE(fw_st->status, status); + + if (status == FW_STATUS_DONE || + status == FW_STATUS_ABORTED) + complete_all(&fw_st->completion); } +#define fw_status_start(fw_st) \ + __fw_status_set(fw_st, FW_STATUS_LOADING) +#define fw_status_done(fw_st) \ + __fw_status_set(fw_st, FW_STATUS_DONE) +#define fw_status_aborted(fw_st) \ + __fw_status_set(fw_st, FW_STATUS_ABORTED) +#define fw_status_is_loading(fw_st) \ + (__fw_status_get(fw_st) == FW_STATUS_LOADING) +#define fw_status_is_done(fw_st) \ + (__fw_status_get(fw_st) == FW_STATUS_DONE) +#define fw_status_is_aborted(fw_st) \ + (__fw_status_get(fw_st) == FW_STATUS_ABORTED) + +#else /* CONFIG_FW_LOADER_USER_HELPER */ + +#define fw_status_wait_timeout(fw_st, long) 0 + +#define fw_status_start(fw_st) +#define fw_status_done(fw_st) +#define fw_status_aborted(fw_st) +#define fw_status_is_loading(fw_st) +#define fw_status_is_done(fw_st) +#define fw_status_is_aborted(fw_st) false + +#endif /* !CONFIG_FW_LOADER_USER_HELPER */ + /* firmware behavior options */ #define FW_OPT_UEVENT (1U << 0) #define FW_OPT_NOWAIT (1U << 1) @@ -145,13 +214,12 @@ struct firmware_cache { struct firmware_buf { struct kref ref; struct list_head list; - struct completion completion; struct firmware_cache *fwc; - unsigned long status; void *data; size_t size; size_t allocated_size; #ifdef CONFIG_FW_LOADER_USER_HELPER + struct fw_status fw_st; bool is_paged_buf; bool need_uevent; struct page **pages; @@ -205,8 +273,8 @@ static struct firmware_buf *__allocate_fw_buf(const char *fw_name, buf->fwc = fwc; buf->data = dbuf; buf->allocated_size = size; - init_completion(&buf->completion); #ifdef CONFIG_FW_LOADER_USER_HELPER + fw_status_init(&buf->fw_st); INIT_LIST_HEAD(&buf->pending_list); #endif @@ -305,15 +373,6 @@ static const char * const fw_path[] = { module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644); MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path"); -static void fw_finish_direct_load(struct device *device, - struct firmware_buf *buf) -{ - mutex_lock(&fw_lock); - set_bit(FW_STATUS_DONE, &buf->status); - complete_all(&buf->completion); - mutex_unlock(&fw_lock); -} - static int fw_get_filesystem_firmware(struct device *device, struct firmware_buf *buf) { @@ -360,7 +419,7 @@ fw_get_filesystem_firmware(struct device *device, struct firmware_buf *buf) } dev_dbg(device, "direct-loading %s\n", buf->fw_id); buf->size = size; - fw_finish_direct_load(device, buf); + fw_status_done(&buf->fw_st); break; } __putname(path); @@ -478,12 +537,11 @@ static void __fw_load_abort(struct firmware_buf *buf) * There is a small window in which user can write to 'loading' * between loading done and disappearance of 'loading' */ - if (test_bit(FW_STATUS_DONE, &buf->status)) + if (fw_status_is_done(&buf->fw_st)) return; list_del_init(&buf->pending_list); - set_bit(FW_STATUS_ABORT, &buf->status); - complete_all(&buf->completion); + fw_status_aborted(&buf->fw_st); } static void fw_load_abort(struct firmware_priv *fw_priv) @@ -496,9 +554,6 @@ static void fw_load_abort(struct firmware_priv *fw_priv) fw_priv->buf = NULL; } -#define is_fw_load_aborted(buf) \ - test_bit(FW_STATUS_ABORT, &(buf)->status) - static LIST_HEAD(pending_fw_head); /* reboot notifier for avoid deadlock with usermode_lock */ @@ -596,10 +651,8 @@ static ssize_t firmware_loading_show(struct device *dev, struct firmware_priv *fw_priv = to_firmware_priv(dev); int loading = 0; - mutex_lock(&fw_lock); if (fw_priv->buf) - loading = test_bit(FW_STATUS_LOADING, &fw_priv->buf->status); - mutex_unlock(&fw_lock); + loading = fw_status_is_loading(&fw_priv->buf->fw_st); return sprintf(buf, "%d\n", loading); } @@ -653,23 +706,20 @@ static ssize_t firmware_loading_store(struct device *dev, switch (loading) { case 1: /* discarding any previous partial load */ - if (!test_bit(FW_STATUS_DONE, &fw_buf->status)) { + if (!fw_status_is_done(&fw_buf->fw_st)) { for (i = 0; i < fw_buf->nr_pages; i++) __free_page(fw_buf->pages[i]); vfree(fw_buf->pages); fw_buf->pages = NULL; fw_buf->page_array_size = 0; fw_buf->nr_pages = 0; - set_bit(FW_STATUS_LOADING, &fw_buf->status); + fw_status_start(&fw_buf->fw_st); } break; case 0: - if (test_bit(FW_STATUS_LOADING, &fw_buf->status)) { + if (fw_status_is_loading(&fw_buf->fw_st)) { int rc; - set_bit(FW_STATUS_DONE, &fw_buf->status); - clear_bit(FW_STATUS_LOADING, &fw_buf->status); - /* * Several loading requests may be pending on * one same firmware buf, so let all requests @@ -691,10 +741,11 @@ static ssize_t firmware_loading_store(struct device *dev, */ list_del_init(&fw_buf->pending_list); if (rc) { - set_bit(FW_STATUS_ABORT, &fw_buf->status); + fw_status_aborted(&fw_buf->fw_st); written = rc; + } else { + fw_status_done(&fw_buf->fw_st); } - complete_all(&fw_buf->completion); break; } /* fallthrough */ @@ -702,7 +753,7 @@ static ssize_t firmware_loading_store(struct device *dev, dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading); /* fallthrough */ case -1: - fw_load_abort(fw_priv); + fw_status_aborted(&fw_buf->fw_st); break; } out: @@ -755,7 +806,7 @@ static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj, mutex_lock(&fw_lock); buf = fw_priv->buf; - if (!buf || test_bit(FW_STATUS_DONE, &buf->status)) { + if (!buf || fw_status_is_done(&buf->fw_st)) { ret_count = -ENODEV; goto out; } @@ -842,7 +893,7 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj, mutex_lock(&fw_lock); buf = fw_priv->buf; - if (!buf || test_bit(FW_STATUS_DONE, &buf->status)) { + if (!buf || fw_status_is_done(&buf->fw_st)) { retval = -ENODEV; goto out; } @@ -955,8 +1006,7 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, timeout = MAX_JIFFY_OFFSET; } - retval = wait_for_completion_interruptible_timeout(&buf->completion, - timeout); + retval = fw_status_wait_timeout(&buf->fw_st, timeout); if (retval == -ERESTARTSYS || !retval) { mutex_lock(&fw_lock); fw_load_abort(fw_priv); @@ -965,7 +1015,7 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, retval = 0; } - if (is_fw_load_aborted(buf)) + if (fw_status_is_aborted(&buf->fw_st)) retval = -EAGAIN; else if (buf->is_paged_buf && !buf->data) retval = -ENOMEM; @@ -1015,35 +1065,12 @@ fw_load_from_user_helper(struct firmware *firmware, const char *name, return -ENOENT; } -/* No abort during direct loading */ -#define is_fw_load_aborted(buf) false - #ifdef CONFIG_PM_SLEEP static inline void kill_requests_without_uevent(void) { } #endif #endif /* CONFIG_FW_LOADER_USER_HELPER */ - -/* wait until the shared firmware_buf becomes ready (or error) */ -static int sync_cached_firmware_buf(struct firmware_buf *buf) -{ - int ret = 0; - - mutex_lock(&fw_lock); - while (!test_bit(FW_STATUS_DONE, &buf->status)) { - if (is_fw_load_aborted(buf)) { - ret = -ENOENT; - break; - } - mutex_unlock(&fw_lock); - ret = wait_for_completion_interruptible(&buf->completion); - mutex_lock(&fw_lock); - } - mutex_unlock(&fw_lock); - return ret; -} - /* prepare firmware and firmware_buf structs; * return 0 if a firmware is already assigned, 1 if need to load one, * or a negative error code @@ -1077,7 +1104,8 @@ _request_firmware_prepare(struct firmware **firmware_p, const char *name, firmware->priv = buf; if (ret > 0) { - ret = sync_cached_firmware_buf(buf); + ret = fw_status_wait_timeout(&buf->fw_st, + firmware_loading_timeout()); if (!ret) { fw_set_page_data(buf, firmware); return 0; /* assigned */ @@ -1095,7 +1123,7 @@ static int assign_firmware_buf(struct firmware *fw, struct device *device, struct firmware_buf *buf = fw->priv; mutex_lock(&fw_lock); - if (!buf->size || is_fw_load_aborted(buf)) { + if (!buf->size || fw_status_is_aborted(&buf->fw_st)) { mutex_unlock(&fw_lock); return -ENOENT; } -- 2.7.4
