From: Mason Zhang <[email protected]>
In ufs error handler flow, host will send device management cmd(NOP OUT)
to device for recovery link. If cmd response timeout, and clear doorbell
fail, ufshcd_wait_for_dev_cmd will do nothing and return,
hba->dev_cmd.complete struct not set to null.
In this time, if cmd has been responsed by device, then it will
call complete() in __ufshcd_transfer_req_compl, because of complete
struct is alloced in stack, then the KE will occur.
Fix the following crash:
ipanic_die+0x24/0x38 [mrdump]
die+0x344/0x748
arm64_notify_die+0x44/0x104
do_debug_exception+0x104/0x1e0
el1_dbg+0x38/0x54
el1_sync_handler+0x40/0x88
el1_sync+0x8c/0x140
queued_spin_lock_slowpath+0x2e4/0x3c0
__ufshcd_transfer_req_compl+0x3b0/0x1164
ufshcd_trc_handler+0x15c/0x308
ufshcd_host_reset_and_restore+0x54/0x260
ufshcd_reset_and_restore+0x28c/0x57c
ufshcd_err_handler+0xeb8/0x1b6c
process_one_work+0x288/0x964
worker_thread+0x4bc/0xc7c
kthread+0x15c/0x264
ret_from_fork+0x10/0x30
Signed-off-by: Mason Zhang <[email protected]>
---
changes since v2:
* Modified comment after clear doorbell.
Updated the patch in v3.
changes since v1:
* Remove change id.
---
drivers/ufs/core/ufshcd.c | 46 ++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 25 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index b1f59a5fe632..6fe51b8d41f9 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2979,35 +2979,31 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
err = -ETIMEDOUT;
dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n",
__func__, lrbp->task_tag);
- if (ufshcd_clear_cmds(hba, 1U << lrbp->task_tag) == 0) {
+ if (ufshcd_clear_cmds(hba, 1U << lrbp->task_tag) == 0)
/* successfully cleared the command, retry if needed */
err = -EAGAIN;
+ /*
+ * in case of an error, after clearing the doorbell,
+ * we also need to clear the task tag bit from the
+ * outstanding_reqs variable.
+ */
+ spin_lock_irqsave(&hba->outstanding_lock, flags);
+ pending = test_bit(lrbp->task_tag,
+ &hba->outstanding_reqs);
+ if (pending) {
+ hba->dev_cmd.complete = NULL;
+ __clear_bit(lrbp->task_tag,
+ &hba->outstanding_reqs);
+ }
+ spin_unlock_irqrestore(&hba->outstanding_lock, flags);
+
+ if (!pending) {
/*
- * Since clearing the command succeeded we also need to
- * clear the task tag bit from the outstanding_reqs
- * variable.
+ * The completion handler ran while we tried to
+ * clear the command.
*/
- spin_lock_irqsave(&hba->outstanding_lock, flags);
- pending = test_bit(lrbp->task_tag,
- &hba->outstanding_reqs);
- if (pending) {
- hba->dev_cmd.complete = NULL;
- __clear_bit(lrbp->task_tag,
- &hba->outstanding_reqs);
- }
- spin_unlock_irqrestore(&hba->outstanding_lock, flags);
-
- if (!pending) {
- /*
- * The completion handler ran while we tried to
- * clear the command.
- */
- time_left = 1;
- goto retry;
- }
- } else {
- dev_err(hba->dev, "%s: failed to clear tag %d\n",
- __func__, lrbp->task_tag);
+ time_left = 1;
+ goto retry;
}
}
--
2.18.0
On 12/9/22 02:13, Mason Zhang wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index b1f59a5fe632..6fe51b8d41f9 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2979,35 +2979,31 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba,
> err = -ETIMEDOUT;
> dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n",
> __func__, lrbp->task_tag);
> - if (ufshcd_clear_cmds(hba, 1U << lrbp->task_tag) == 0) {
> + if (ufshcd_clear_cmds(hba, 1U << lrbp->task_tag) == 0)
> /* successfully cleared the command, retry if needed */
> err = -EAGAIN;
> + /*
> + * in case of an error, after clearing the doorbell,
> + * we also need to clear the task tag bit from the
> + * outstanding_reqs variable.
> + */
> + spin_lock_irqsave(&hba->outstanding_lock, flags);
> + pending = test_bit(lrbp->task_tag,
> + &hba->outstanding_reqs);
> + if (pending) {
> + hba->dev_cmd.complete = NULL;
> + __clear_bit(lrbp->task_tag,
> + &hba->outstanding_reqs);
> + }
> + spin_unlock_irqrestore(&hba->outstanding_lock, flags);
This patch causes the 'task_tag' bit to be cleared from outstanding_reqs
even if ufshcd_clear_cmds() failed. I think that's wrong.
Bart.