This is a note to let you know that I've just added the patch titled
firmware loader: Fix the race FW_STATUS_DONE is followed by
to my driver-core git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
in the driver-core-next branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will also be merged in the next major kernel release
during the merge window.
If you have any questions about this process, please let me know.
>From ce2fcbd99cef580623116bb33531dbc3e6f690b0 Mon Sep 17 00:00:00 2001
From: Chuansheng Liu <chuansheng....@intel.com>
Date: Thu, 8 Nov 2012 19:14:40 +0800
Subject: firmware loader: Fix the race FW_STATUS_DONE is followed by
class_timeout
There is a race as below when calling request_firmware():
CPU1 CPU2
write 0 > loading
mutex_lock(&fw_lock)
...
set_bit FW_STATUS_DONE class_timeout is coming
set_bit FW_STATUS_ABORT
complete_all &completion
...
mutex_unlock(&fw_lock)
In this time, the bit FW_STATUS_DONE and FW_STATUS_ABORT are set,
and request_firmware() will return failure due to condition in
_request_firmware_load():
if (!buf->size || test_bit(FW_STATUS_ABORT, &buf->status))
retval = -ENOENT;
But from the above scenerio, it should be a successful requesting.
So we need judge if the bit FW_STATUS_DONE is already set before
calling fw_load_abort() in timeout function.
As Ming's proposal, we need change the timer into sched_work to
benefit from using &fw_lock mutex also.
Signed-off-by: liu chuansheng <chuansheng....@intel.com>
Acked-by: Ming Lei <ming....@canonical.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
drivers/base/firmware_class.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8945f4e..b44ed35 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -143,7 +143,7 @@ struct fw_cache_entry {
};
struct firmware_priv {
- struct timer_list timeout;
+ struct delayed_work timeout_work;
bool nowait;
struct device dev;
struct firmware_buf *buf;
@@ -667,11 +667,18 @@ static struct bin_attribute firmware_attr_data = {
.write = firmware_data_write,
};
-static void firmware_class_timeout(u_long data)
+static void firmware_class_timeout_work(struct work_struct *work)
{
- struct firmware_priv *fw_priv = (struct firmware_priv *) data;
+ struct firmware_priv *fw_priv = container_of(work,
+ struct firmware_priv, timeout_work.work);
+ mutex_lock(&fw_lock);
+ if (test_bit(FW_STATUS_DONE, &(fw_priv->buf->status))) {
+ mutex_unlock(&fw_lock);
+ return;
+ }
fw_load_abort(fw_priv);
+ mutex_unlock(&fw_lock);
}
static struct firmware_priv *
@@ -690,8 +697,8 @@ fw_create_instance(struct firmware *firmware, const char
*fw_name,
fw_priv->nowait = nowait;
fw_priv->fw = firmware;
- setup_timer(&fw_priv->timeout,
- firmware_class_timeout, (u_long) fw_priv);
+ INIT_DELAYED_WORK(&fw_priv->timeout_work,
+ firmware_class_timeout_work);
f_dev = &fw_priv->dev;
@@ -858,7 +865,9 @@ static int _request_firmware_load(struct firmware_priv
*fw_priv, bool uevent,
dev_dbg(f_dev->parent, "firmware: direct-loading"
" firmware %s\n", buf->fw_id);
+ mutex_lock(&fw_lock);
set_bit(FW_STATUS_DONE, &buf->status);
+ mutex_unlock(&fw_lock);
complete_all(&buf->completion);
direct_load = 1;
goto handle_fw;
@@ -894,15 +903,14 @@ static int _request_firmware_load(struct firmware_priv
*fw_priv, bool uevent,
dev_set_uevent_suppress(f_dev, false);
dev_dbg(f_dev, "firmware: requesting %s\n", buf->fw_id);
if (timeout != MAX_SCHEDULE_TIMEOUT)
- mod_timer(&fw_priv->timeout,
- round_jiffies_up(jiffies + timeout));
+ schedule_delayed_work(&fw_priv->timeout_work, timeout);
kobject_uevent(&fw_priv->dev.kobj, KOBJ_ADD);
}
wait_for_completion(&buf->completion);
- del_timer_sync(&fw_priv->timeout);
+ cancel_delayed_work_sync(&fw_priv->timeout_work);
handle_fw:
mutex_lock(&fw_lock);
--
1.7.12.2.421.g261b511
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
