Hi Asutosh,That'd work too.
On 2020-07-29 02:06, Asutosh Das (asd) wrote:
On 7/27/2020 10:00 PM, Can Guo wrote:
Sometime dumps in IRQ handler are heavy enough to cause system stabilityHow about keep the above prints and move the tmrs and trs to eh?
issues, move them to error handler.
Signed-off-by: Can Guo <cang@xxxxxxxxxxxxxx>
---
 drivers/scsi/ufs/ufshcd.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c480823..b2bafa3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5682,6 +5682,21 @@ static void ufshcd_err_handler(struct work_struct *work)
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ UFSHCD_UIC_DL_TCx_REPLAY_ERROR))))
ÂÂÂÂÂÂÂÂÂ needs_reset = true;
 + if (hba->saved_err & (INT_FATAL_ERRORS | UIC_ERROR |
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ UFSHCD_UIC_HIBERN8_MASK)) {
+ÂÂÂÂÂÂÂ bool pr_prdt = !!(hba->saved_err & SYSTEM_BUS_FATAL_ERROR);
+
+ÂÂÂÂÂÂÂ dev_err(hba->dev, "%s: saved_err 0x%x saved_uic_err 0x%x\n",
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ __func__, hba->saved_err, hba->saved_uic_err);
+ÂÂÂÂÂÂÂ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ÂÂÂÂÂÂÂ ufshcd_print_host_state(hba);
+ÂÂÂÂÂÂÂ ufshcd_print_pwr_info(hba);
+ÂÂÂÂÂÂÂ ufshcd_print_host_regs(hba);
+ÂÂÂÂÂÂÂ ufshcd_print_tmrs(hba, hba->outstanding_tasks);
+ÂÂÂÂÂÂÂ ufshcd_print_trs(hba, hba->outstanding_reqs, pr_prdt);
+ÂÂÂÂÂÂÂ spin_lock_irqsave(hba->host->host_lock, flags);
+ÂÂÂ }
+
ÂÂÂÂÂ /*
ÂÂÂÂÂÂ * if host reset is required then skip clearing the pending
ÂÂÂÂÂÂ * transfers forcefully because they will get cleared during
@@ -5900,22 +5915,6 @@ static irqreturn_t ufshcd_check_errors(struct ufs_hba *hba)
ÂÂÂÂÂÂÂÂÂÂÂ /* block commands from scsi mid-layer */
ÂÂÂÂÂÂÂÂÂ ufshcd_scsi_block_requests(hba);
-
-ÂÂÂÂÂÂÂ /* dump controller state before resetting */
-ÂÂÂÂÂÂÂ if (hba->saved_err & (INT_FATAL_ERRORS | UIC_ERROR)) {
-ÂÂÂÂÂÂÂÂÂÂÂ bool pr_prdt = !!(hba->saved_err &
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ SYSTEM_BUS_FATAL_ERROR);
-
-ÂÂÂÂÂÂÂÂÂÂÂ dev_err(hba->dev, "%s: saved_err 0x%x saved_uic_err 0x%x\n",
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ __func__, hba->saved_err,
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ hba->saved_uic_err);
-
-ÂÂÂÂÂÂÂÂÂÂÂ ufshcd_print_host_regs(hba);
-ÂÂÂÂÂÂÂÂÂÂÂ ufshcd_print_pwr_info(hba);
Sometimes in system instability, the eh may not get a chance to run
even. Still the above prints would provide some clues.
Here is the IRQ handler, ufshcd_print_host_regs() is sometime heavy
enough to cause stability issues during my fault injection test, since
it prints host regs, reg's history, crypto debug infos plus prints
from vops_dump.
How about just printing host regs and reg history here? Most time, these
infos are enough.
Thanks,
Can Guo.
-ÂÂÂÂÂÂÂÂÂÂÂ ufshcd_print_tmrs(hba, hba->outstanding_tasks);
-ÂÂÂÂÂÂÂÂÂÂÂ ufshcd_print_trs(hba, hba->outstanding_reqs,
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ pr_prdt);
-ÂÂÂÂÂÂÂ }
ÂÂÂÂÂÂÂÂÂ ufshcd_schedule_eh_work(hba);
ÂÂÂÂÂÂÂÂÂ retval |= IRQ_HANDLED;
ÂÂÂÂÂ }