| From 2451e0ae307f242fe76d61e911df76ce2f04decf Mon Sep 17 00:00:00 2001 |
| From: Junyong Sun <sunjy516@gmail.com> |
| Date: Tue, 3 Mar 2020 10:36:08 +0800 |
| Subject: [PATCH] firmware: fix a double abort case with fw_load_sysfs_fallback |
| |
| commit bcfbd3523f3c6eea51a74d217a8ebc5463bcb7f4 upstream. |
| |
| fw_sysfs_wait_timeout may return err with -ENOENT |
| at fw_load_sysfs_fallback and firmware is already |
| in abort status, no need to abort again, so skip it. |
| |
| This issue is caused by concurrent situation like below: |
| when thread 1# wait firmware loading, thread 2# may write |
| -1 to abort loading and wakeup thread 1# before it timeout. |
| so wait_for_completion_killable_timeout of thread 1# would |
| return remaining time which is != 0 with fw_st->status |
| FW_STATUS_ABORTED.And the results would be converted into |
| err -ENOENT in __fw_state_wait_common and transfered to |
| fw_load_sysfs_fallback in thread 1#. |
| The -ENOENT means firmware status is already at ABORTED, |
| so fw_load_sysfs_fallback no need to get mutex to abort again. |
| ----------------------------- |
| thread 1#,wait for loading |
| fw_load_sysfs_fallback |
| ->fw_sysfs_wait_timeout |
| ->__fw_state_wait_common |
| ->wait_for_completion_killable_timeout |
| |
| in __fw_state_wait_common, |
| ... |
| 93 ret = wait_for_completion_killable_timeout(&fw_st->completion, timeout); |
| 94 if (ret != 0 && fw_st->status == FW_STATUS_ABORTED) |
| 95 return -ENOENT; |
| 96 if (!ret) |
| 97 return -ETIMEDOUT; |
| 98 |
| 99 return ret < 0 ? ret : 0; |
| ----------------------------- |
| thread 2#, write -1 to abort loading |
| firmware_loading_store |
| ->fw_load_abort |
| ->__fw_load_abort |
| ->fw_state_aborted |
| ->__fw_state_set |
| ->complete_all |
| |
| in __fw_state_set, |
| ... |
| 111 if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED) |
| 112 complete_all(&fw_st->completion); |
| ------------------------------------------- |
| BTW,the double abort issue would not cause kernel panic or create an issue, |
| but slow down it sometimes.The change is just a minor optimization. |
| |
| Signed-off-by: Junyong Sun <sunjunyong@xiaomi.com> |
| Acked-by: Luis Chamberlain <mcgrof@kernel.org> |
| Link: https://lore.kernel.org/r/1583202968-28792-1-git-send-email-sunjunyong@xiaomi.com |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c |
| index 103b5d37fa86..91c5091f9c60 100644 |
| --- a/drivers/base/firmware_loader/fallback.c |
| +++ b/drivers/base/firmware_loader/fallback.c |
| @@ -572,7 +572,7 @@ static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs, |
| } |
| |
| retval = fw_sysfs_wait_timeout(fw_priv, timeout); |
| - if (retval < 0) { |
| + if (retval < 0 && retval != -ENOENT) { |
| mutex_lock(&fw_lock); |
| fw_load_abort(fw_sysfs); |
| mutex_unlock(&fw_lock); |
| -- |
| 2.7.4 |
| |