[PATCH 9/9] scsi: ufs: core: Do not recover the link concurrently with a PM callback

[Stanley Jhu]

From: Brian Kao <powenkao@xxxxxxxxxx>

Commit f966e02ae521 ("scsi: ufs: core: Fix runtime suspend error
deadlock") made ufshcd_err_handler() call ufshcd_link_recovery() and
return early if hba->pm_op_in_progress is set. Commit e23ef4f22db3
("scsi: ufs: core: Fix error handler host_sem issue") moved that check
ahead of down(&hba->host_sem). The patch "scsi: ufs: core: Limit early
link recovery in the error handler to runtime PM" restricts that early
path to ufshcd_in_wl_rpm_callback() and notes in its cover
letter that during runtime resume it can still run concurrently with
__ufshcd_wl_resume(), and removing it there needs a closer look at the
deadlock that it avoids.

With this shortcut, when a fatal UIC error arrives while
__ufshcd_wl_resume() is in ufshcd_bkops_ctrl(), ufshcd_err_handler()
calls ufshcd_link_recovery() -> ufshcd_host_reset_and_restore() without
holding host_sem or waiting for the PM callback, putting the host
controller into a reset (non-working) state while ufshcd_issue_dev_cmd()
on the resume thread is still sending query requests and accessing the
controller:

  Process A (PM callback, e.g. runtime resume):
  __ufshcd_wl_resume()
    -> ufshcd_bkops_ctrl()
      -> ufshcd_query_attr_retry()
        -> ufshcd_exec_dev_cmd()
          -> ufshcd_issue_dev_cmd()
            (accesses controller while in reset state)

  Process B (UFS error handler, triggered by UIC error):
  ufshcd_err_handler()
    -> if (ufshcd_in_wl_rpm_callback(hba))
      -> ufshcd_link_recovery()
        -> ufshcd_host_reset_and_restore()
          (puts controller into reset state)

Remove the early link recovery shortcut and
ufshcd_in_wl_rpm_callback() from ufshcd_err_handler() so that
error handling waits in ufshcd_err_handling_prepare() for a runtime PM
callback and on hba->host_sem for a system PM callback to finish before
resetting the controller. This does not reintroduce the runtime suspend
deadlock because:
- If the state is UFSHCD_STATE_EH_SCHEDULED_FATAL, ufshcd_queuecommand()
  fails SCSI commands such as START STOP UNIT sent from a PM callback
  immediately with DID_BAD_TARGET, and ufshcd_issue_dev_cmd() fails
  reserved device management commands immediately with -EIO, while any
  already in-flight PM command timeout is completed by
  ufshcd_eh_timed_out().
- If the state is UFSHCD_STATE_EH_SCHEDULED_NON_FATAL,
  ufshcd_queuecommand() still dispatches START STOP UNIT because
  eh_in_progress is not set yet.
- If a dispatched START STOP UNIT times out without a pending fatal
  error, ufshcd_eh_timed_out() recovers the link directly and returns
  SCSI_EH_DONE without waiting on eh_work.
- Runtime suspend errors call ufshcd_force_error_recovery() and return
  -EBUSY.
- Runtime resume (__ufshcd_wl_resume()) does not wait on eh_work or
  host_sem. A hibern8 exit failure recovers on the PM thread, a START
  STOP UNIT failure returns an error, and query errors are ignored.

Removing the shortcut relies on the companion patches in this series,
which complete PM command timeouts in ufshcd_eh_timed_out() before
reaching SCSI error handling and disable W-LUN runtime PM in
ufshcd_err_handling_prepare().

Fixes: f966e02ae521 ("scsi: ufs: core: Fix runtime suspend error deadlock")
Fixes: e23ef4f22db3 ("scsi: ufs: core: Fix error handler host_sem issue")
Cc: stable@xxxxxxxxxxxxxxx # 7.3.x
Signed-off-by: Brian Kao <powenkao@xxxxxxxxxx>
[stanleyjhu: rebased onto "scsi: ufs: core: Limit early link recovery
 in the error handler to runtime PM", clarified the reserved command
 path in the commit message, and limited Cc: stable to 7.3.x]
Signed-off-by: Stanley Jhu <stanleyjhu@xxxxxxxxxx>
---
 drivers/ufs/core/ufshcd.c | 39 ---------------------------------------
 1 file changed, 39 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 87d475e323b1..1aecf55c0646 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7120,23 +7120,6 @@ static bool ufshcd_abort_all(struct ufs_hba *hba)
 	return ret != 0;
 }
 
-/*
- * Whether a runtime PM callback of the WLUN runs __ufshcd_wl_suspend() or
- * __ufshcd_wl_resume().
- */
-static bool ufshcd_in_wl_rpm_callback(struct ufs_hba *hba)
-{
-#ifdef CONFIG_PM
-	enum rpm_status status =
-		READ_ONCE(hba->ufs_device_wlun->sdev_gendev.power.runtime_status);
-
-	return READ_ONCE(hba->pm_op_in_progress) &&
-	       (status == RPM_SUSPENDING || status == RPM_RESUMING);
-#else
-	return false;
-#endif
-}
-
 /**
  * ufshcd_err_handler - handle UFS errors that require s/w attention
  * @work: pointer to work structure
@@ -7163,28 +7146,6 @@ static void ufshcd_err_handler(struct work_struct *work)
 		 hba->saved_uic_err, hba->force_reset,
 		 ufshcd_is_link_broken(hba) ? "; link is broken" : "");
 
-	if (hba->ufs_device_wlun) {
-		/*
-		 * Use ufshcd_rpm_get_noresume() here to safely perform link
-		 * recovery even if an error occurs during runtime suspend or
-		 * runtime resume. This avoids potential deadlocks that could
-		 * happen if we tried to resume the device while a PM operation
-		 * is already in progress.
-		 *
-		 * Do this only while the PM callbacks run for a runtime PM
-		 * operation. The system PM and poweroff callbacks hold
-		 * host_sem, and shutting_down makes the regular path below
-		 * stop, so it does not reset the link concurrently with them.
-		 */
-		ufshcd_rpm_get_noresume(hba);
-		if (ufshcd_in_wl_rpm_callback(hba)) {
-			ufshcd_link_recovery(hba);
-			ufshcd_rpm_put(hba);
-			return;
-		}
-		ufshcd_rpm_put(hba);
-	}
-
 	down(&hba->host_sem);
 	spin_lock_irqsave(&hba->host->host_lock, flags);
 	if (ufshcd_err_handling_should_stop(hba)) {
-- 
2.56.0.567.gdb80ae8cd5-goog