[PATCH 0/9] scsi: ufs: core: Fix device management and PM recovery

[PATCH 0/9] scsi: ufs: core: Fix device management and PM recovery

[Stanley Jhu]

A UFS PM callback holds hba->dev_cmd.lock and the only reserved tag
while it sends device management and START STOP UNIT commands. When
one of these commands times out, ufshcd_eh_timed_out() resets the host
inline, but the reset path needs the same lock and tag, so it
deadlocks or fails. The error handler can also reset the host while
the PM callback is still using it.

This series:
- skips the MCQ completion queue poll when error handling preparation
  left the queues unconfigured (patch 1/9).
- hands PM command timeouts to ufshcd_err_handler() instead of
  resetting the host inline, and fails later PM commands fast
  (patches 2/9 to 4/9).
- disables W-LUN runtime PM during error handling instead of calling
  ufshcd_rpm_get_sync(), so the error handler powers up the host
  itself (patches 5/9 and 6/9).
- keeps the host variant power state balanced across error handling,
  resumes the UFS device W-LUN and wakes suspended logical units after
  a successful reset, and makes the error handler wait for a running
  PM callback (patches 7/9 to 9/9).

Based on mkp/scsi 7.4/scsi-fixes (e550f2b2b4aa) plus Hoyoung Seo's v10
series [1]. On this base, its patch 4/4 needs a context fixup in
__ufshcd_wl_resume() because of commit 6da1bc55c060 ("scsi: ufs: core:
Keep clock-gating hold on failed W-LUN runtime resume").

This series folds in [2] and supersedes [3].

[1] https://lore.kernel.org/r/2052256418.81794387787276.JavaMail.epsvc@epcpadp2new
[2] https://lore.kernel.org/r/20261104112744.1630864-1-stanleyjhu@google.com
[3] https://lore.kernel.org/r/20261105071518.1458428-1-stanleyjhu@google.com

Brian Kao (1):
  scsi: ufs: core: Do not recover the link concurrently with a PM
    callback

Stanley Jhu (8):
  scsi: ufs: core: Do not poll unconfigured MCQ queues in the error
    handler
  scsi: ufs: core: Defer link recovery on reserved PM command timeout
  scsi: ufs: core: Wait for the reserved tag in
    ufshcd_get_dev_mgmt_cmd()
  scsi: ufs: core: Fail PM commands fast when fatal recovery is
    scheduled
  scsi: ufs: core: Disable W-LUN runtime PM in
    ufshcd_err_handling_prepare()
  block: Keep RPM_ACTIVE in blk_pre_runtime_resume() when already active
  scsi: ufs: core: Unwind variant resume in EH when reset fails
  scsi: ufs: core: Wake suspended consumer queues after host reset

 block/blk-pm.c                 |  13 +-
 drivers/ufs/core/ufshcd-priv.h |  19 ++-
 drivers/ufs/core/ufshcd.c      | 244 ++++++++++++++++++++++-----------
 include/ufs/ufshcd.h           |   3 +
 4 files changed, 189 insertions(+), 90 deletions(-)


base-commit: e550f2b2b4aab4c5f12db89d04d8978ce4ed7b4e
prerequisite-patch-id: 0c2cfc48bd97246e47ce689a3a8a364c18a56621
prerequisite-patch-id: 9a413c053d618cedfc6a1a439469bc49a51e060a
prerequisite-patch-id: 961b8b65ac8c9fd1b95d0901347cf504632531e4
prerequisite-patch-id: 9f5c103bac288c258b541cff3742a69fd07892f1
-- 
2.56.0.506.g506a7eabc5-goog

[PATCH 1/9] scsi: ufs: core: Do not poll unconfigured MCQ queues in the error handler

[Stanley Jhu]

In MCQ mode, ufshcd_err_handler() can poll completion queues that a
failed reset left unconfigured:

1. The UFS device W-LUN is runtime suspended with the link in hibern8.
   The device stops responding and the host reports a fatal UIC error,
   so the interrupt handler schedules ufshcd_err_handler().
2. ufshcd_err_handler() passes its first stop check and calls
   ufshcd_err_handling_prepare(), which resumes the W-LUN through
   ufshcd_rpm_get_sync().
3. The resume fails to exit hibern8 and calls ufshcd_link_recovery().
   The reset completes all outstanding requests, but link startup fails
   before the MCQ queues are configured, leaving UFSHCD_STATE_ERROR.
4. ufshcd_err_handler() calls ufshcd_complete_requests(), which logs
   "Abnormal CQ entry!" for each empty slot and writes the head pointer
   of each unconfigured queue.
5. Only the second stop check sees UFSHCD_STATE_ERROR and gives up.

Check the stop condition under host_lock right after preparation and
skip ufshcd_complete_requests() when error handling is going to stop.
The failed reset has already completed every outstanding request
and has not set up the completion queues again, so there is nothing
to poll.

Fixes: ab248643d3d6 ("scsi: ufs: core: Add error handling for MCQ mode")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 96e25a0a1dbe..e9dcab35a4cd 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7093,6 +7093,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 	unsigned long flags;
 	bool needs_restore;
 	bool needs_reset;
+	bool stop;
 	int pmc_err;
 
 	hba = container_of(work, struct ufs_hba, eh_work);
@@ -7141,10 +7142,13 @@ static void ufshcd_err_handler(struct work_struct *work)
 
 	spin_lock_irqsave(&hba->host->host_lock, flags);
 	ufshcd_set_eh_in_progress(hba);
+	/* A PM callback in the preparation above may have reset the host. */
+	stop = ufshcd_err_handling_should_stop(hba);
 	spin_unlock_irqrestore(&hba->host->host_lock, flags);
 
 	/* Complete requests that have door-bell cleared by h/w */
-	ufshcd_complete_requests(hba, false);
+	if (!stop)
+		ufshcd_complete_requests(hba, false);
 	spin_lock_irqsave(&hba->host->host_lock, flags);
 again:
 	needs_restore = false;
-- 
2.56.0.567.gdb80ae8cd5-goog

[PATCH 2/9] scsi: ufs: core: Defer link recovery on reserved PM command timeout

[Stanley Jhu]

When a device management command sent by a PM callback times out,
ufshcd_eh_timed_out() runs ufshcd_link_recovery() inline:

1. The PM callback holds hba->dev_cmd.lock, a read lock on
   hba->clk_scaling_lock, and the only reserved tag while it waits
   for the command.
2. The command times out and ufshcd_eh_timed_out() resets the host.
3. The reset runs ufshcd_verify_dev_init(), which needs the same tag
   and lock, so host init fails with -ENOMEM in SDB mode and
   deadlocks on hba->dev_cmd.lock in MCQ mode.

Instead, stop the host controller, schedule the error handler and
complete the command with DID_TIME_OUT, so the PM callback releases
the tag and locks before recovery runs.

The remaining tail of ufshcd_eh_timed_out() then only handles SCSI
commands, so drop the reserved command branch there.

Fixes: 01d5e237b339 ("scsi: ufs: core: Handle PM commands timeout before SCSI EH")
Cc: stable@vger.kernel.org # 7.3.x: cd37a69c367b: scsi: ufs: core: Remove an always-false WARN_ONCE()
Cc: stable@vger.kernel.org # 7.3.x: 22d14117ec18: scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs
Cc: stable@vger.kernel.org # 7.3.x: 3a7e551ae4bc: scsi: ufs: core: Fix double release in ufshcd_eh_timed_out()
Cc: stable@vger.kernel.org # 7.3.x
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 60 +++++++++++++++++++++++++++++++++------
 1 file changed, 52 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index e9dcab35a4cd..6a5f5fa732d9 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7147,7 +7147,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 	spin_unlock_irqrestore(&hba->host->host_lock, flags);
 
 	/* Complete requests that have door-bell cleared by h/w */
-	if (!stop)
+	if (!stop && ufshcd_is_hba_active(hba))
 		ufshcd_complete_requests(hba, false);
 	spin_lock_irqsave(&hba->host->host_lock, flags);
 again:
@@ -9689,6 +9689,50 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 		dev_err(hba->dev, "%s failed: %d\n", __func__, ret);
 }
 
+/*
+ * Claim @scmd before stopping the HBA in case normal completion raced with
+ * scsi_timeout(), then complete @scmd so the PM caller releases
+ * hba->dev_cmd.lock and the reserved tag before error recovery runs.
+ */
+static void ufshcd_eh_pm_cmd_timed_out(struct ufs_hba *hba,
+				       struct scsi_cmnd *scmd)
+{
+	struct request *rq = scsi_cmd_to_rq(scmd);
+	struct ufs_hw_queue *hwq;
+
+	if (!hba->mcq_enabled) {
+		if (!ufshcd_clear_outstanding_reqs(hba, BIT(rq->tag)))
+			return;
+		ufshcd_hba_stop(hba);
+		ufshcd_force_error_recovery(hba);
+		set_host_byte(scmd, DID_TIME_OUT);
+		scsi_done(scmd);
+		return;
+	}
+
+	hwq = ufshcd_mcq_req_to_hwq(hba, rq);
+	if (ufshcd_is_hba_active(hba))
+		ufshcd_mcq_poll_cqe_lock(hba, hwq);
+	scoped_guard(spinlock_irqsave, &hwq->cq_lock) {
+		if (test_bit(SCMD_STATE_COMPLETE, &scmd->state))
+			return;
+	}
+	spin_lock_irq(&hba->host->host_lock);
+	hba->force_reset = true;
+	if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
+		hba->ufshcd_state = UFSHCD_STATE_EH_SCHEDULED_FATAL;
+	spin_unlock_irq(&hba->host->host_lock);
+	ufshcd_hba_stop(hba);
+	ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
+	ufshcd_force_error_recovery(hba);
+	scoped_guard(spinlock_irqsave, &hwq->cq_lock) {
+		if (!test_bit(SCMD_STATE_COMPLETE, &scmd->state)) {
+			set_host_byte(scmd, DID_TIME_OUT);
+			scsi_done(scmd);
+		}
+	}
+}
+
 static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 {
 	struct ufs_hba *hba = shost_priv(scmd->device->host);
@@ -9698,6 +9742,11 @@ static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 		return SCSI_EH_NOT_HANDLED;
 	}
 
+	if (!ufshcd_is_scsi_cmd(scmd)) {
+		ufshcd_eh_pm_cmd_timed_out(hba, scmd);
+		return SCSI_EH_DONE;
+	}
+
 	/*
 	 * Handle the timeout directly to prevent a deadlock between
 	 * ufshcd_set_dev_pwr_mode() and ufshcd_err_handler().
@@ -9718,13 +9767,8 @@ static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 				return SCSI_EH_DONE;
 		}
 
-		if (ufshcd_is_scsi_cmd(scmd)) {
-			set_host_byte(scmd, DID_REQUEUE);
-			ufshcd_release_scsi_cmd(hba, scmd);
-		} else {
-			set_host_byte(scmd, DID_TIME_OUT);
-		}
-
+		set_host_byte(scmd, DID_REQUEUE);
+		ufshcd_release_scsi_cmd(hba, scmd);
 		scsi_done(scmd);
 	}
 
-- 
2.56.0.567.gdb80ae8cd5-goog

Re: [PATCH 2/9] scsi: ufs: core: Defer link recovery on reserved PM command timeout

[Bart Van Assche]

On 11/11/26 9:15 PM, Stanley Jhu wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index e9dcab35a4cd..6a5f5fa732d9 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -7147,7 +7147,7 @@ static void ufshcd_err_handler(struct work_struct *work)
>   	spin_unlock_irqrestore(&hba->host->host_lock, flags);
>   
>   	/* Complete requests that have door-bell cleared by h/w */
> -	if (!stop)
> +	if (!stop && ufshcd_is_hba_active(hba))
>   		ufshcd_complete_requests(hba, false);
>   	spin_lock_irqsave(&hba->host->host_lock, flags);
>   again:
> @@ -9689,6 +9689,50 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
>   		dev_err(hba->dev, "%s failed: %d\n", __func__, ret);
>   }
>   
> +/*
> + * Claim @scmd before stopping the HBA in case normal completion raced with
> + * scsi_timeout(), then complete @scmd so the PM caller releases
> + * hba->dev_cmd.lock and the reserved tag before error recovery runs.
> + */
> +static void ufshcd_eh_pm_cmd_timed_out(struct ufs_hba *hba,
> +				       struct scsi_cmnd *scmd)
> +{
> +	struct request *rq = scsi_cmd_to_rq(scmd);
> +	struct ufs_hw_queue *hwq;
> +
> +	if (!hba->mcq_enabled) {
> +		if (!ufshcd_clear_outstanding_reqs(hba, BIT(rq->tag)))
> +			return;
> +		ufshcd_hba_stop(hba);
> +		ufshcd_force_error_recovery(hba);
> +		set_host_byte(scmd, DID_TIME_OUT);
> +		scsi_done(scmd);
> +		return;
> +	}
> +
> +	hwq = ufshcd_mcq_req_to_hwq(hba, rq);
> +	if (ufshcd_is_hba_active(hba))
> +		ufshcd_mcq_poll_cqe_lock(hba, hwq);
> +	scoped_guard(spinlock_irqsave, &hwq->cq_lock) {
> +		if (test_bit(SCMD_STATE_COMPLETE, &scmd->state))
> +			return;
> +	}
> +	spin_lock_irq(&hba->host->host_lock);
> +	hba->force_reset = true;
> +	if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
> +		hba->ufshcd_state = UFSHCD_STATE_EH_SCHEDULED_FATAL;
> +	spin_unlock_irq(&hba->host->host_lock);
> +	ufshcd_hba_stop(hba);
> +	ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
> +	ufshcd_force_error_recovery(hba);
> +	scoped_guard(spinlock_irqsave, &hwq->cq_lock) {
> +		if (!test_bit(SCMD_STATE_COMPLETE, &scmd->state)) {
> +			set_host_byte(scmd, DID_TIME_OUT);
> +			scsi_done(scmd);
> +		}
> +	}
> +}
> +
>   static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
>   {
>   	struct ufs_hba *hba = shost_priv(scmd->device->host);
> @@ -9698,6 +9742,11 @@ static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
>   		return SCSI_EH_NOT_HANDLED;
>   	}
>   
> +	if (!ufshcd_is_scsi_cmd(scmd)) {
> +		ufshcd_eh_pm_cmd_timed_out(hba, scmd);
> +		return SCSI_EH_DONE;
> +	}
> +
>   	/*
>   	 * Handle the timeout directly to prevent a deadlock between
>   	 * ufshcd_set_dev_pwr_mode() and ufshcd_err_handler().

I don't like the approach of this patch series. I think this series 
changes much more code than what is necessary. There is a much simpler
way to fix the issue reported in the cover letter: instead of calling
ufshcd_link_recovery(hba) from ufshcd_eh_timed_out(), remove that call
from ufshcd_eh_timed_out() and instead call ufshcd_link_recovery(hba)
from ufshcd_set_dev_pwr_mode() if the SCSI error code returned by
ufshcd_execute_start_stop() indicates that the SCSI START STOP UNIT
command has timed out.

Thanks,

Bart.

[PATCH 3/9] scsi: ufs: core: Wait for the reserved tag in ufshcd_get_dev_mgmt_cmd()

[Stanley Jhu]

ufshcd_get_dev_mgmt_cmd() allocates the reserved tag with
BLK_MQ_REQ_NOWAIT and assumes that holding hba->dev_cmd.lock makes the
allocation succeed. That lock only serializes the owners of the tag. A
busy tag iterator holds its own reference on the request, and the tag
is freed only when the last reference is dropped:

1. A PM callback, e.g. __ufshcd_wl_suspend(), sends a query request and
   waits for it in ufshcd_issue_dev_cmd().
2. The request times out. blk_mq_timeout_work() takes a reference on
   it in bt_iter() and calls ufshcd_eh_timed_out(), which completes it
   with scsi_done().
3. The PM callback wakes up, frees the request, releases
   hba->dev_cmd.lock and retries the query from a retry loop such as
   ufshcd_query_attr_retry().
4. The retry calls ufshcd_get_dev_mgmt_cmd() before
   blk_mq_timeout_work() has dropped its reference. The tag is still
   busy, the NOWAIT allocation fails, ufshcd_exec_dev_cmd() triggers
   WARN_ON_ONCE(!cmd) and the query fails with -ENOMEM.

The timeout handler is not the only trigger. Other busy tag iterators,
e.g. scsi_host_busy() called from ufshcd_gate_work(), take the same
reference, so the tag can stay busy after its owner has freed the
request. Holding hba->dev_cmd.lock therefore does not guarantee that
the NOWAIT allocation succeeds.

Fix this by dropping BLK_MQ_REQ_NOWAIT so that the allocation waits
until the reference is dropped and the tag is freed. Since the patch
"scsi: ufs: core: Defer link recovery on reserved PM command timeout",
the timeout handler no longer allocates a reserved tag while it holds
a reference on a reserved request, so the wait ends when
ufshcd_eh_timed_out() returns. Also correct the comment about why
hba->dev_cmd.lock is held, and state that the function must not be
called while holding a reference on a reserved request.

Fixes: 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
Cc: stable@vger.kernel.org # 7.3.x
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 6a5f5fa732d9..6d789f3badf1 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3366,14 +3366,17 @@ static void ufshcd_dev_man_unlock(struct ufs_hba *hba)
 static struct scsi_cmnd *ufshcd_get_dev_mgmt_cmd(struct ufs_hba *hba)
 {
 	/*
-	 * The caller must hold this lock to guarantee that the NOWAIT
-	 * allocation will succeed.
+	 * hba->dev_cmd.lock serializes the owners of the reserved tag. A busy
+	 * tag iterator, e.g. the block layer timeout handler, may still hold a
+	 * reference on the previous request after its owner freed it. Hence
+	 * the allocation may have to wait until that reference is dropped.
+	 * Must not be called while holding a reference on a reserved request.
 	 */
 	lockdep_assert_held(&hba->dev_cmd.lock);
 
 	return scsi_get_internal_cmd(
 		hba->host->pseudo_sdev, DMA_TO_DEVICE,
-		BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
+		BLK_MQ_REQ_RESERVED);
 }
 
 static void ufshcd_put_dev_mgmt_cmd(struct scsi_cmnd *cmd)
-- 
2.56.0.567.gdb80ae8cd5-goog

[PATCH 4/9] scsi: ufs: core: Fail PM commands fast when fatal recovery is scheduled

[Stanley Jhu]

Once a fatal error has scheduled the error handler during a PM
callback, the callback should fail fast and leave recovery to the
error handler. Two paths do not:

1. ufshcd_issue_dev_cmd() keeps sending device management commands to
   the stopped controller, and each one waits for a full timeout. This
   includes the NOP OUT retries in ufshcd_verify_dev_init() after the
   controller has been stopped.
2. When START STOP UNIT times out, ufshcd_eh_timed_out() resets the
   link inline and marks the host operational while the error handler
   is still pending. System suspend can then succeed and hold
   hba->host_sem across sleep, which blocks the error handler until
   resume.

Fail device management commands with -EIO in that state, and let
START STOP UNIT, the only SCSI command a PM callback sends, take the
same deferred recovery path. __ufshcd_wl_suspend() then returns
-EBUSY, so system suspend is aborted and releases hba->host_sem for
the error handler, and runtime suspend is retried once recovery is
done. Also keep UFSHCD_STATE_EH_SCHEDULED_FATAL when
ufshcd_err_handler() stops early during shutdown so remaining shutdown
PM commands fail fast as well.

Fixes: 01d5e237b339 ("scsi: ufs: core: Handle PM commands timeout before SCSI EH")
Cc: stable@vger.kernel.org # 7.3.x
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 6d789f3badf1..5563d28ef92e 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3395,6 +3395,11 @@ static int ufshcd_issue_dev_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 	struct request *rq = scsi_cmd_to_rq(cmd);
 	blk_status_t sts;
 
+	if (hba->pm_op_in_progress &&
+	    hba->ufshcd_state == UFSHCD_STATE_EH_SCHEDULED_FATAL &&
+	    !(ufshcd_eh_in_progress(hba) && ufshcd_is_hba_active(hba)))
+		return -EIO;
+
 	rq->timeout = msecs_to_jiffies(timeout);
 	sts = blk_execute_rq(rq, true);
 	if (sts != BLK_STS_OK)
@@ -7133,7 +7138,9 @@ static void ufshcd_err_handler(struct work_struct *work)
 	down(&hba->host_sem);
 	spin_lock_irqsave(&hba->host->host_lock, flags);
 	if (ufshcd_err_handling_should_stop(hba)) {
-		if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
+		if (!(hba->shutting_down &&
+		      hba->ufshcd_state == UFSHCD_STATE_EH_SCHEDULED_FATAL) &&
+		    hba->ufshcd_state != UFSHCD_STATE_ERROR)
 			hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
 		spin_unlock_irqrestore(&hba->host->host_lock, flags);
 		up(&hba->host_sem);
@@ -9701,6 +9708,7 @@ static void ufshcd_eh_pm_cmd_timed_out(struct ufs_hba *hba,
 				       struct scsi_cmnd *scmd)
 {
 	struct request *rq = scsi_cmd_to_rq(scmd);
+	bool is_scsi = ufshcd_is_scsi_cmd(scmd);
 	struct ufs_hw_queue *hwq;
 
 	if (!hba->mcq_enabled) {
@@ -9708,6 +9716,8 @@ static void ufshcd_eh_pm_cmd_timed_out(struct ufs_hba *hba,
 			return;
 		ufshcd_hba_stop(hba);
 		ufshcd_force_error_recovery(hba);
+		if (is_scsi)
+			ufshcd_release_scsi_cmd(hba, scmd);
 		set_host_byte(scmd, DID_TIME_OUT);
 		scsi_done(scmd);
 		return;
@@ -9730,6 +9740,8 @@ static void ufshcd_eh_pm_cmd_timed_out(struct ufs_hba *hba,
 	ufshcd_force_error_recovery(hba);
 	scoped_guard(spinlock_irqsave, &hwq->cq_lock) {
 		if (!test_bit(SCMD_STATE_COMPLETE, &scmd->state)) {
+			if (is_scsi)
+				ufshcd_release_scsi_cmd(hba, scmd);
 			set_host_byte(scmd, DID_TIME_OUT);
 			scsi_done(scmd);
 		}
@@ -9739,13 +9751,14 @@ static void ufshcd_eh_pm_cmd_timed_out(struct ufs_hba *hba,
 static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 {
 	struct ufs_hba *hba = shost_priv(scmd->device->host);
+	bool is_scsi = ufshcd_is_scsi_cmd(scmd);
 
 	if (!hba->pm_op_in_progress) {
 		/* Activate the error handler in the SCSI core. */
 		return SCSI_EH_NOT_HANDLED;
 	}
 
-	if (!ufshcd_is_scsi_cmd(scmd)) {
+	if (!is_scsi || hba->ufshcd_state == UFSHCD_STATE_EH_SCHEDULED_FATAL) {
 		ufshcd_eh_pm_cmd_timed_out(hba, scmd);
 		return SCSI_EH_DONE;
 	}
-- 
2.56.0.567.gdb80ae8cd5-goog

[PATCH 5/9] scsi: ufs: core: Disable W-LUN runtime PM in ufshcd_err_handling_prepare()

[Stanley Jhu]

ufshcd_err_handling_prepare() calls ufshcd_rpm_get_sync() to power up
the host. Since commit eef1d74bf88f ("PM: runtime: Only set
runtime_error on suspend callback failures"), a failed W-LUN resume no
longer sets runtime_error, so this call runs the W-LUN resume again:

1. A request to a runtime-suspended logical unit resumes its W-LUN
   supplier, and __ufshcd_wl_resume() sends START STOP UNIT.
2. A fatal error sets UFSHCD_STATE_EH_SCHEDULED_FATAL, and
   ufshcd_queuecommand() fails START STOP UNIT with DID_BAD_TARGET.
3. sdev_runtime_resume() fails and clears pm_only on the W-LUN queue
   while rpm_resume() leaves the W-LUN RPM_SUSPENDED.
4. ufshcd_err_handling_prepare() calls ufshcd_rpm_get_sync() while the
   state is still UFSHCD_STATE_EH_SCHEDULED_FATAL. START STOP UNIT
   fails the same way, and blk_clear_pm_only() underflows pm_only
   to -1.

Instead, have the error handler power up the host itself by taking
ufshcd_rpm_get_noresume() and disabling W-LUN runtime PM in
ufshcd_err_handling_prepare(), then re-enabling it in
ufshcd_err_handling_unprepare(). Not resuming the W-LUN in
ufshcd_err_handling_prepare() needs two more changes.

First, force a host reset when the W-LUN is runtime suspended. On a
non-fatal UIC error with the link active in runtime suspend, a logical
unit resuming during error handling ignores the disabled W-LUN.
Skipping reset then leaves the device asleep, and I/O after unquiesce
fails with NOT READY.

Second, block non-W-LUN commands before blk_mq_quiesce_tagset() freezes
the queues. Once W-LUN runtime PM is disabled, another logical unit
resuming in that gap ignores the disabled W-LUN and sends commands that
hit a sleeping device or fail with DID_BAD_TARGET. Add
UFSHCD_EH_WLUN_FENCE to return SCSI_MLQUEUE_HOST_BUSY for non-W-LUN
commands while letting in-flight W-LUN PM commands finish.

Fixes: eef1d74bf88f ("PM: runtime: Only set runtime_error on suspend callback failures")
Cc: stable@vger.kernel.org # 7.3.x
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 5563d28ef92e..7e3f9abc7e7c 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -215,6 +215,7 @@ static const char *const ufshcd_state_name[] = {
 /* UFSHCD error handling flags */
 enum {
 	UFSHCD_EH_IN_PROGRESS = (1 << 0),
+	UFSHCD_EH_WLUN_FENCE = (1 << 1),
 };
 
 /* UFSHCD UIC layer error flags */
@@ -235,6 +236,13 @@ enum {
 #define ufshcd_clear_eh_in_progress(h) \
 	((h)->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)
 
+#define ufshcd_set_eh_wlun_fence(h) \
+	((h)->eh_flags |= UFSHCD_EH_WLUN_FENCE)
+#define ufshcd_eh_wlun_fence(h) \
+	((h)->eh_flags & UFSHCD_EH_WLUN_FENCE)
+#define ufshcd_clear_eh_wlun_fence(h) \
+	((h)->eh_flags &= ~UFSHCD_EH_WLUN_FENCE)
+
 const struct ufs_pm_lvl_states ufs_pm_lvl_states[] = {
 	[UFS_PM_LVL_0] = {UFS_ACTIVE_PWR_MODE, UIC_LINK_ACTIVE_STATE},
 	[UFS_PM_LVL_1] = {UFS_ACTIVE_PWR_MODE, UIC_LINK_HIBERN8_STATE},
@@ -3134,12 +3142,19 @@ static enum scsi_qc_status ufshcd_queuecommand(struct Scsi_Host *host,
 		 * UFSHCD_STATE_EH_SCHEDULED_NON_FATAL. Prevent requests
 		 * being issued in that case.
 		 */
-		if (ufshcd_eh_in_progress(hba)) {
+		if (ufshcd_eh_in_progress(hba) ||
+		    (ufshcd_eh_wlun_fence(hba) &&
+		     cmd->device != hba->ufs_device_wlun)) {
 			err = SCSI_MLQUEUE_HOST_BUSY;
 			goto out;
 		}
 		break;
 	case UFSHCD_STATE_EH_SCHEDULED_FATAL:
+		if (ufshcd_eh_wlun_fence(hba) &&
+		    cmd->device != hba->ufs_device_wlun) {
+			err = SCSI_MLQUEUE_HOST_BUSY;
+			goto out;
+		}
 		/*
 		 * pm_runtime_get_sync() is used at error handling preparation
 		 * stage. If a scsi cmd, e.g. the SSU cmd, is sent from hba's
@@ -6899,7 +6914,8 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 	 * runtime suspended, so take an extra reference on hba->dev.
 	 */
 	pm_runtime_get_sync(hba->dev);
-	ufshcd_rpm_get_sync(hba);
+	ufshcd_rpm_get_noresume(hba);
+	__pm_runtime_disable(&hba->ufs_device_wlun->sdev_gendev, false);
 	if (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev) ||
 	    hba->is_sys_suspended) {
 		enum ufs_pm_op pm_op;
@@ -6943,6 +6959,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 	ufshcd_release(hba);
 	if (ufshcd_is_clkscaling_supported(hba))
 		ufshcd_clk_scaling_suspend(hba, false);
+	pm_runtime_enable(&hba->ufs_device_wlun->sdev_gendev);
 	ufshcd_rpm_put(hba);
 	pm_runtime_put(hba->dev);
 }
@@ -7146,6 +7163,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 		up(&hba->host_sem);
 		return;
 	}
+	ufshcd_set_eh_wlun_fence(hba);
 	spin_unlock_irqrestore(&hba->host->host_lock, flags);
 
 	ufshcd_err_handling_prepare(hba);
@@ -7166,6 +7184,8 @@ static void ufshcd_err_handler(struct work_struct *work)
 
 	if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
 		hba->ufshcd_state = UFSHCD_STATE_RESET;
+	if (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev))
+		hba->force_reset = true;
 	/*
 	 * A full reset and restore might have happened after preparation
 	 * is finished, double check whether we should stop.
@@ -7295,6 +7315,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 			goto again;
 		hba->ufshcd_state = UFSHCD_STATE_ERROR;
 	}
+	ufshcd_clear_eh_wlun_fence(hba);
 	ufshcd_clear_eh_in_progress(hba);
 	spin_unlock_irqrestore(&hba->host->host_lock, flags);
 	ufshcd_err_handling_unprepare(hba);
-- 
2.56.0.567.gdb80ae8cd5-goog

[PATCH 6/9] block: Keep RPM_ACTIVE in blk_pre_runtime_resume() when already active

[Stanley Jhu]

Since commit eef1d74bf88f ("PM: runtime: Only set runtime_error on
suspend callback failures"), a failed runtime resume leaves
dev->power.runtime_error at 0 with runtime_status set to RPM_SUSPENDED,
so later runtime resume requests run sdev_runtime_resume() again:

1. A runtime suspend increments q->pm_only from 0 to 1 and sets
   q->rpm_status to RPM_SUSPENDED.
2. The first runtime resume fails in the driver callback, and
   blk_post_runtime_resume() sets q->rpm_status to RPM_ACTIVE and calls
   blk_clear_pm_only(), decrementing q->pm_only from 1 to 0 so recovery
   commands can enter the queue (intended since
   commit 6e1fcab00a23 ("scsi: block: pm: Always set request queue
   runtime active in blk_post_runtime_resume()")).
3. A retried runtime resume calls blk_pre_runtime_resume(), which
   overwrites q->rpm_status from RPM_ACTIVE to RPM_RESUMING, so
   blk_post_runtime_resume() sees old_status != RPM_ACTIVE and calls
   blk_clear_pm_only() a second time without a matching
   blk_set_pm_only().
4. q->pm_only underflows from 0 to -1 (triggering WARN_ON_ONCE in
   blk_clear_pm_only()), blocking non-PM requests in blk_queue_enter()
   and __bio_queue_enter() while the device is RPM_ACTIVE, and the next
   runtime suspend increments q->pm_only from -1 to 0, letting non-PM
   requests dispatch while RPM_SUSPENDED. This hits the UFS device
   well-known LUN by default and SCSI disks with
   manage_runtime_start_stop enabled (ATA and FireWire sbp2 defaults).

Transition q->rpm_status to RPM_RESUMING in blk_pre_runtime_resume()
only when it is RPM_SUSPENDED, and call blk_clear_pm_only() in
blk_post_runtime_resume() only when old_status is RPM_RESUMING, pairing
each clear with a completed runtime suspend.

Fixes: eef1d74bf88f ("PM: runtime: Only set runtime_error on suspend callback failures")
Cc: stable@vger.kernel.org # 7.3.x
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 block/blk-pm.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/block/blk-pm.c b/block/blk-pm.c
index 8d3e052f91da..34cc7c5ca04e 100644
--- a/block/blk-pm.c
+++ b/block/blk-pm.c
@@ -142,7 +142,8 @@ EXPORT_SYMBOL(blk_post_runtime_suspend);
  *
  * Description:
  *    Update the queue's runtime status to RESUMING in preparation for the
- *    runtime resume of the device.
+ *    runtime resume of the device if the queue is runtime suspended, or keep
+ *    RPM_ACTIVE if a previous failed runtime resume already reactivated it.
  *
  *    This function should be called near the start of the device's
  *    runtime_resume callback.
@@ -153,7 +154,9 @@ void blk_pre_runtime_resume(struct request_queue *q)
 		return;
 
 	spin_lock_irq(&q->queue_lock);
-	q->rpm_status = RPM_RESUMING;
+	/* Keep RPM_ACTIVE if a prior failed resume reactivated the queue. */
+	if (q->rpm_status == RPM_SUSPENDED)
+		q->rpm_status = RPM_RESUMING;
 	spin_unlock_irq(&q->queue_lock);
 }
 EXPORT_SYMBOL(blk_pre_runtime_resume);
@@ -168,8 +171,8 @@ EXPORT_SYMBOL(blk_pre_runtime_resume);
  *    driver or error handler will need to communicate with the device.
  *
  *    This function should be called near the end of the device's
- *    runtime_resume callback to correct queue runtime PM status and re-enable
- *    peeking requests from the queue.
+ *    runtime_resume callback to mark the queue runtime active and clear
+ *    pm_only if the queue was resuming.
  */
 void blk_post_runtime_resume(struct request_queue *q)
 {
@@ -185,7 +188,7 @@ void blk_post_runtime_resume(struct request_queue *q)
 	pm_request_autosuspend(q->dev);
 	spin_unlock_irq(&q->queue_lock);
 
-	if (old_status != RPM_ACTIVE)
+	if (old_status == RPM_RESUMING)
 		blk_clear_pm_only(q);
 }
 EXPORT_SYMBOL(blk_post_runtime_resume);
-- 
2.56.0.567.gdb80ae8cd5-goog

[PATCH 7/9] scsi: ufs: core: Unwind variant resume in EH when reset fails

[Stanley Jhu]

ufshcd_err_handling_prepare() resumes the host variant with
UFS_SYSTEM_PM or UFS_RUNTIME_PM when hba->is_sys_suspended is true or
the UFS device W-LUN is runtime suspended. When no host reset
succeeds, ufshcd_err_handling_unprepare() does not undo that resume.

Along the runtime PM path:

1. __ufshcd_wl_suspend() suspends the variant, and the W-LUN enters
   RPM_SUSPENDED.
2. A fatal controller error schedules ufshcd_err_handler().
3. ufshcd_err_handling_prepare() calls ufshcd_vops_resume() with
   UFS_RUNTIME_PM.
4. ufshcd_reset_and_restore() fails and sets hba->ufshcd_state to
   UFSHCD_STATE_ERROR.
5. ufshcd_err_handling_unprepare() returns without calling
   ufshcd_vops_suspend(), and the variant stays runtime-resumed on the
   dead host.

Along the system PM path:

1. ufshcd_wl_suspend() suspends the variant and sets
   hba->is_sys_suspended.
2. START STOP UNIT fails on all three attempts in ufshcd_wl_resume().
3. __ufshcd_wl_resume() suspends the variant again and returns -EIO.
4. A fatal controller error schedules ufshcd_err_handler().
5. ufshcd_err_handling_prepare() calls ufshcd_vops_resume() with
   UFS_SYSTEM_PM.
6. ufshcd_reset_and_restore() fails and leaves the variant
   system-resumed on the dead host.

Suspend the variant in ufshcd_err_handling_unprepare() when no host
reset in the error handler run succeeded, so that the variant power
state matches the W-LUN and system power state. Suspending the variant
there needs three more changes.

First, track hba->vops_suspended in ufshcd_vops_suspend() and
ufshcd_vops_resume(). The link is active after a successful host reset,
so ufshcd_err_handling_unprepare() keeps the variant resumed even when
the W-LUN stays RPM_SUSPENDED. hba->vops_suspended then makes the next
__ufshcd_wl_resume() skip the variant resume callback.

Second, set hba->force_reset in ufshcd_err_handler() when
hba->is_sys_suspended is true. A non-fatal UIC error otherwise ends
error handling without a host reset. ufshcd_err_handling_unprepare()
would then suspend the variant before it unquiesces the queues.

Third, keep pm_recovered set once a host reset succeeds. Error recovery
may run several passes. A pass with a successful reset runs
ufshcd_recover_pm_error(), which clears hba->is_sys_suspended and may
activate the W-LUN before a later pass fails to reset the host.

Fixes: c72e79c0ad2b ("scsi: ufs: Recover HBA runtime PM error in error handler")
Cc: stable@vger.kernel.org # 7.3.x
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd-priv.h | 19 +++++++++++----
 drivers/ufs/core/ufshcd.c      | 43 +++++++++++++++++++++++++++-------
 include/ufs/ufshcd.h           |  3 +++
 3 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 99f654b79035..a63f7567017e 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -238,18 +238,29 @@ static inline void ufshcd_vops_fixup_dev_quirks(struct ufs_hba *hba)
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op,
 				enum ufs_notify_change_status status)
 {
+	int ret = 0;
+
 	if (hba->vops && hba->vops->suspend)
-		return hba->vops->suspend(hba, op, status);
+		ret = hba->vops->suspend(hba, op, status);
+	if (!ret && status == POST_CHANGE)
+		hba->vops_suspended = true;
 
-	return 0;
+	return ret;
 }
 
 static inline int ufshcd_vops_resume(struct ufs_hba *hba, enum ufs_pm_op op)
 {
+	int ret = 0;
+
+	if (!hba->vops_suspended)
+		return 0;
+
 	if (hba->vops && hba->vops->resume)
-		return hba->vops->resume(hba, op);
+		ret = hba->vops->resume(hba, op);
+	if (!ret)
+		hba->vops_suspended = false;
 
-	return 0;
+	return ret;
 }
 
 static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 7e3f9abc7e7c..286ffb44772d 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6907,8 +6907,16 @@ static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
 	}
 }
 
-static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
+/*
+ * Return true if the W-LUN or the system is suspended and the host variant is
+ * not suspended when this function returns. This includes the case where an
+ * earlier error handler run left the variant resumed and ufshcd_vops_resume()
+ * returns 0 without calling the variant.
+ */
+static bool ufshcd_err_handling_prepare(struct ufs_hba *hba)
 {
+	bool vops_resumed = false;
+
 	/*
 	 * A WLUN resume failure could potentially lead to the HBA being
 	 * runtime suspended, so take an extra reference on hba->dev.
@@ -6934,7 +6942,7 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 		if (!ufshcd_is_clkgating_allowed(hba))
 			ufshcd_setup_clocks(hba, true);
 		pm_op = hba->is_sys_suspended ? UFS_SYSTEM_PM : UFS_RUNTIME_PM;
-		ufshcd_vops_resume(hba, pm_op);
+		vops_resumed = !ufshcd_vops_resume(hba, pm_op);
 	} else {
 		ufshcd_hold(hba);
 		if (ufshcd_is_clkscaling_supported(hba) &&
@@ -6950,11 +6958,24 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 	 */
 	blk_mq_unquiesce_queue(hba->host->pseudo_sdev->request_queue);
 	cancel_work_sync(&hba->eeh_work);
+	return vops_resumed;
 }
 
-static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
+static void ufshcd_err_handling_unprepare(struct ufs_hba *hba,
+					  bool vops_resumed, bool pm_recovered)
 {
 	blk_mq_quiesce_queue_nowait(hba->host->pseudo_sdev->request_queue);
+	/*
+	 * !pm_recovered: no recovery ran, so the W-LUN or system is
+	 * suspended.
+	 */
+	if (vops_resumed && !pm_recovered) {
+		enum ufs_pm_op pm_op = hba->is_sys_suspended ?
+				       UFS_SYSTEM_PM : UFS_RUNTIME_PM;
+
+		ufshcd_vops_suspend(hba, pm_op, PRE_CHANGE);
+		ufshcd_vops_suspend(hba, pm_op, POST_CHANGE);
+	}
 	blk_mq_unquiesce_tagset(&hba->host->tag_set);
 	ufshcd_release(hba);
 	if (ufshcd_is_clkscaling_supported(hba))
@@ -7116,7 +7137,10 @@ static void ufshcd_err_handler(struct work_struct *work)
 	int retries = MAX_ERR_HANDLER_RETRIES;
 	struct ufs_hba *hba;
 	unsigned long flags;
+	/* Set once a reset succeeds and ufshcd_recover_pm_error() has run. */
+	bool pm_recovered = false;
 	bool needs_restore;
+	bool vops_resumed;
 	bool needs_reset;
 	bool stop;
 	int pmc_err;
@@ -7166,7 +7190,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 	ufshcd_set_eh_wlun_fence(hba);
 	spin_unlock_irqrestore(&hba->host->host_lock, flags);
 
-	ufshcd_err_handling_prepare(hba);
+	vops_resumed = ufshcd_err_handling_prepare(hba);
 
 	spin_lock_irqsave(&hba->host->host_lock, flags);
 	ufshcd_set_eh_in_progress(hba);
@@ -7184,7 +7208,8 @@ static void ufshcd_err_handler(struct work_struct *work)
 
 	if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
 		hba->ufshcd_state = UFSHCD_STATE_RESET;
-	if (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev))
+	if (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev) ||
+	    hba->is_sys_suspended)
 		hba->force_reset = true;
 	/*
 	 * A full reset and restore might have happened after preparation
@@ -7292,11 +7317,13 @@ static void ufshcd_err_handler(struct work_struct *work)
 		hba->force_reset = false;
 		spin_unlock_irqrestore(&hba->host->host_lock, flags);
 		err = ufshcd_reset_and_restore(hba);
-		if (err)
+		if (err) {
 			dev_err(hba->dev, "%s: reset and restore failed with err %d\n",
 					__func__, err);
-		else
+		} else {
+			pm_recovered = true;
 			ufshcd_recover_pm_error(hba);
+		}
 		spin_lock_irqsave(&hba->host->host_lock, flags);
 	}
 
@@ -7318,7 +7345,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 	ufshcd_clear_eh_wlun_fence(hba);
 	ufshcd_clear_eh_in_progress(hba);
 	spin_unlock_irqrestore(&hba->host->host_lock, flags);
-	ufshcd_err_handling_unprepare(hba);
+	ufshcd_err_handling_unprepare(hba, vops_resumed, pm_recovered);
 	up(&hba->host_sem);
 
 	dev_info(hba->dev, "%s finished; HBA state %s\n", __func__,
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index be62165b785e..df366d11f6ae 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1038,6 +1038,8 @@ enum ufshcd_mcq_opr {
  * @devfreq: frequency scaling information owned by the devfreq core
  * @clk_scaling: frequency scaling information owned by the UFS driver
  * @is_sys_suspended: UFS device has been suspended because of system suspend
+ * @vops_suspended: set by a successful POST_CHANGE ufshcd_vops_suspend(),
+ *  cleared by a successful ufshcd_vops_resume()
  * @urgent_bkops_lvl: keeps track of urgent bkops level for device
  * @is_urgent_bkops_lvl_checked: keeps track if the urgent bkops level for
  *  device is known or not.
@@ -1226,6 +1228,7 @@ struct ufs_hba {
 	struct devfreq *devfreq;
 	struct ufs_clk_scaling clk_scaling;
 	bool is_sys_suspended;
+	bool vops_suspended;
 
 	enum bkops_status urgent_bkops_lvl;
 	bool is_urgent_bkops_lvl_checked;
-- 
2.56.0.567.gdb80ae8cd5-goog

[PATCH 8/9] scsi: ufs: core: Wake suspended consumer queues after host reset

[Stanley Jhu]

ufshcd_err_handling_prepare() now disables runtime PM of the UFS device
W-LUN, and ufshcd_err_handling_unprepare() enables it again and learns
whether a host reset in the error handler run succeeded.
ufshcd_recover_pm_error() wakes the runtime-suspended logical units
after a host reset only when hba->dev or the W-LUN has
power.runtime_error set, so I/O waiting on a runtime-suspended logical
unit stays blocked when both flags are clear:

1. I/O to a runtime-suspended logical unit blocks in
   __bio_queue_enter() and queues a runtime resume of the logical unit.
2. START STOP UNIT fails in the resume of the W-LUN supplier, and
   rpm_resume() rolls the logical unit back to RPM_SUSPENDED with
   power.runtime_error clear.
3. A later W-LUN runtime resume succeeds.
4. A fatal error makes ufshcd_err_handler() reset the host.
5. ufshcd_recover_pm_error() finds power.runtime_error clear on both
   devices and skips the wake loop.
6. The I/O stays blocked in __bio_queue_enter() until another request
   reaches the same queue.

Request a runtime resume of every runtime-suspended logical unit from
ufshcd_err_handling_unprepare() after every successful host reset,
whether or not power.runtime_error is set. Waking an idle logical unit
sends no SCSI command, and the logical unit returns to runtime suspend
after its autosuspend delay.

Requesting the resumes from ufshcd_err_handling_unprepare() needs two
more changes. First, issue them after pm_runtime_enable() on the W-LUN.
ufshcd_recover_pm_error() runs while W-LUN runtime PM is disabled, and
rpm_get_suppliers() ignores -EACCES from a disabled supplier. A logical
unit woken there resumes while its W-LUN supplier stays RPM_SUSPENDED.
Second, also runtime resume the UFS device W-LUN, synchronously and
before the logical units. ufshcd_err_handling_prepare() disables W-LUN
runtime PM without resuming the W-LUN, so the W-LUN stays RPM_SUSPENDED
after the reset when no logical unit is left runtime suspended to resume
it through the runtime PM device link. pm_request_resume() takes no
reference on hba->dev, so the pm_runtime_put(hba->dev) that follows
can runtime suspend hba->dev while the link is active and every
logical unit is runtime suspended.

Fixes: eef1d74bf88f ("PM: runtime: Only set runtime_error on suspend callback failures")
Cc: stable@vger.kernel.org # 7.3.x
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 51 +++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 286ffb44772d..87d475e323b1 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6961,6 +6961,33 @@ static bool ufshcd_err_handling_prepare(struct ufs_hba *hba)
 	return vops_resumed;
 }
 
+#ifdef CONFIG_PM
+/*
+ * A logical unit whose runtime resume failed against the W-LUN may still have
+ * bios waiting in blk_queue_enter(). Request a runtime resume for every
+ * suspended logical unit to unblock them. Resume the W-LUN first and
+ * synchronously. ufshcd_err_handling_prepare() disabled its runtime PM
+ * without resuming it, and the caller drops its hba->dev reference next.
+ */
+static void ufshcd_resume_sdevs_after_reset(struct ufs_hba *hba)
+{
+	struct scsi_device *sdev;
+	struct request_queue *q;
+
+	ufshcd_rpm_resume(hba);
+	shost_for_each_device(sdev, hba->host) {
+		q = sdev->request_queue;
+		if (q->dev && (q->rpm_status == RPM_SUSPENDED ||
+			       q->rpm_status == RPM_SUSPENDING))
+			pm_request_resume(q->dev);
+	}
+}
+#else
+static inline void ufshcd_resume_sdevs_after_reset(struct ufs_hba *hba)
+{
+}
+#endif
+
 static void ufshcd_err_handling_unprepare(struct ufs_hba *hba,
 					  bool vops_resumed, bool pm_recovered)
 {
@@ -6981,6 +7008,9 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba,
 	if (ufshcd_is_clkscaling_supported(hba))
 		ufshcd_clk_scaling_suspend(hba, false);
 	pm_runtime_enable(&hba->ufs_device_wlun->sdev_gendev);
+	/* The W-LUN and logical units resume only after pm_runtime_enable(). */
+	if (pm_recovered)
+		ufshcd_resume_sdevs_after_reset(hba);
 	ufshcd_rpm_put(hba);
 	pm_runtime_put(hba->dev);
 }
@@ -6998,10 +7028,6 @@ static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
 static void ufshcd_recover_pm_error(struct ufs_hba *hba)
 {
 	struct scsi_target *starget = hba->ufs_device_wlun->sdev_target;
-	struct Scsi_Host *shost = hba->host;
-	struct scsi_device *sdev;
-	struct request_queue *q;
-	bool resume_sdev_queues = false;
 
 	hba->is_sys_suspended = false;
 
@@ -7012,7 +7038,6 @@ static void ufshcd_recover_pm_error(struct ufs_hba *hba)
 	if (hba->dev->power.runtime_error) {
 		/* hba->dev has no functional parent thus simplily set RPM_ACTIVE */
 		pm_runtime_set_active(hba->dev);
-		resume_sdev_queues = true;
 	}
 
 	if (hba->ufs_device_wlun->sdev_gendev.power.runtime_error) {
@@ -7023,22 +7048,6 @@ static void ufshcd_recover_pm_error(struct ufs_hba *hba)
 		pm_runtime_get_sync(&starget->dev);
 		pm_runtime_set_active(&hba->ufs_device_wlun->sdev_gendev);
 		pm_runtime_put_sync(&starget->dev);
-		resume_sdev_queues = true;
-	}
-
-	/*
-	 * If wlun device had runtime error, we also need to resume those
-	 * consumer scsi devices in case any of them has failed to be
-	 * resumed due to supplier runtime resume failure. This is to unblock
-	 * blk_queue_enter in case there are bios waiting inside it.
-	 */
-	if (resume_sdev_queues) {
-		shost_for_each_device(sdev, shost) {
-			q = sdev->request_queue;
-			if (q->dev && (q->rpm_status == RPM_SUSPENDED ||
-				       q->rpm_status == RPM_SUSPENDING))
-				pm_request_resume(q->dev);
-		}
 	}
 }
 #else
-- 
2.56.0.567.gdb80ae8cd5-goog

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

[Stanley Jhu]

From: Brian Kao <powenkao@google.com>

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@vger.kernel.org # 7.3.x
Signed-off-by: Brian Kao <powenkao@google.com>
[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@google.com>
---
 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