mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH
@ 2026-06-05 11:20 Hongjie Fang
  2026-06-05 15:41 ` Bart Van Assche
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Hongjie Fang @ 2026-06-05 11:20 UTC (permalink / raw)
  To: alim.akhtar, avri.altman, bvanassche, James.Bottomley,
	martin.petersen, peter.wang, beanhuo
  Cc: linux-scsi, linux-kernel

A PM START STOP sent from the UFS well-known LU resume path can race with
SCSI EH.

The "wl resume" task flow is:
  __ufshcd_wl_resume()
    ufshcd_set_dev_pwr_mode(UFS_ACTIVE_PWR_MODE)
      ufshcd_execute_start_stop()
        scsi_execute_cmd()
          blk_execute_rq           <-- wait
          scsi_check_passthrough() <-- may retry START STOP

If the first START STOP time out, SCSI EH may already recover the link and
reset the device before scsi_execute_cmd() returns:
  scsi_timeout()
    scsi_eh_scmd_add()
      scsi_error_handler()
        scsi_unjam_host()
          scsi_eh_ready_devs()
            scsi_eh_host_reset()
              ufshcd_eh_host_reset_handler()
                if (hba->pm_op_in_progress)
                  ufshcd_link_recovery()
                    ufshcd_device_reset()
                    ufshcd_host_reset_and_restore()
          ...
          scsi_eh_flush_done_q()   <-- wakeup "wl resume" task
        ...                        <-- host still in SHOST_RECOVERY
        scsi_restart_operations()

A later passthrough retry can then run while the host is still in
SHOST_RECOVERY and hit the SCMD_FAIL_IF_RECOVERING path:
  scsi_queue_rq()
    if (scsi_host_in_recovery(shost) &&
        cmd->flags & SCMD_FAIL_IF_RECOVERING)
      return BLK_STS_OFFLINE

That retry completes with DID_ERROR or DID_NO_CONNECT even though EH may
already have restored the device to an operational ACTIVE state.

Handle these PM timeouts directly from ufshcd_eh_timed_out() instead.
After ufshcd_link_recovery(), complete the timed-out command immediately
if it has not been completed already.

For regular SCSI commands, complete them with DID_REQUEUE to match the
existing MCQ force-completion semantics and allow scsi_execute_cmd() to
retry if needed. For reserved internal device-management commands, finish
the request with DID_TIME_OUT without calling ufshcd_release_scsi_cmd()
since those commands use different resource lifetime rules.

The system_suspending flag is no longer needed because PM command timeout
handling now uses pm_op_in_progress.

Fixes: b8c3a7bac9b6 ("scsi: ufs: Have midlayer retry start stop errors")
Signed-off-by: Hongjie Fang <hongjiefang@asrmicro.com>
---

v5: complete SCSI commands with DID_REQUEUE and use DID_TIME_OUT for
reserved commands suggested by Peter Wang

v4: handle PM timeouts directly from ufshcd_eh_timed_out(), no longer
relay on the extra legacy single-doorbell force-completion path

v3: ufshcd_eh_timed_out() no longer checks the UFS device WLUN or the
START STOP opcode. It only checks whether a PM operation is in progress
for normal SCSI command

v2: handle PM SSU timeout directly from ufshcd_eh_timed_out() suggested
by Bart Van Assche

 drivers/ufs/core/ufshcd.c | 34 +++++++++++++++++++++++++++-------
 include/ufs/ufshcd.h      |  3 ---
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index c3f08957d179..cc38588d99eb 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9466,22 +9466,44 @@ static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 {
 	struct ufs_hba *hba = shost_priv(scmd->device->host);
 
-	if (!hba->system_suspending) {
+	if (!hba->pm_op_in_progress) {
 		/* Activate the error handler in the SCSI core. */
 		return SCSI_EH_NOT_HANDLED;
 	}
 
 	/*
-	 * If we get here we know that no TMFs are outstanding and also that
-	 * the only pending command is a START STOP UNIT command. Handle the
-	 * timeout of that command directly to prevent a deadlock between
+	 * Handle the timeout directly to prevent a deadlock between
 	 * ufshcd_set_dev_pwr_mode() and ufshcd_err_handler().
 	 */
 	ufshcd_link_recovery(hba);
 	dev_info(hba->dev, "%s() finished; outstanding_tasks = %#lx.\n",
 		 __func__, hba->outstanding_tasks);
 
-	return scsi_host_busy(hba->host) ? SCSI_EH_RESET_TIMER : SCSI_EH_DONE;
+	/*
+	 * ufshcd_link_recovery() may already have completed @scmd, e.g. via
+	 * the existing MCQ force-completion path.
+	 */
+	if (!test_bit(SCMD_STATE_COMPLETE, &scmd->state)) {
+		if (!hba->mcq_enabled) {
+			unsigned long flags;
+			struct request *rq = scsi_cmd_to_rq(scmd);
+
+			spin_lock_irqsave(&hba->outstanding_lock, flags);
+			__clear_bit(rq->tag, &hba->outstanding_reqs);
+			spin_unlock_irqrestore(&hba->outstanding_lock, flags);
+		}
+
+		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);
+		}
+
+		scsi_done(scmd);
+	}
+
+	return SCSI_EH_DONE;
 }
 
 static const struct attribute_group *ufshcd_driver_groups[] = {
@@ -10518,7 +10540,6 @@ static int ufshcd_wl_suspend(struct device *dev)
 
 	hba = shost_priv(sdev->host);
 	down(&hba->host_sem);
-	hba->system_suspending = true;
 
 	if (pm_runtime_suspended(dev))
 		goto out;
@@ -10560,7 +10581,6 @@ static int ufshcd_wl_resume(struct device *dev)
 		hba->curr_dev_pwr_mode, hba->uic_link_state);
 	if (!ret)
 		hba->is_sys_suspended = false;
-	hba->system_suspending = false;
 	up(&hba->host_sem);
 	return ret;
 }
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index cfbc75d8df83..8280a95c00c7 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1020,8 +1020,6 @@ enum ufshcd_mcq_opr {
  * @caps: bitmask with information about UFS controller capabilities
  * @devfreq: frequency scaling information owned by the devfreq core
  * @clk_scaling: frequency scaling information owned by the UFS driver
- * @system_suspending: system suspend has been started and system resume has
- *	not yet finished.
  * @is_sys_suspended: UFS device has been suspended because of system suspend
  * @urgent_bkops_lvl: keeps track of urgent bkops level for device
  * @is_urgent_bkops_lvl_checked: keeps track if the urgent bkops level for
@@ -1197,7 +1195,6 @@ struct ufs_hba {
 
 	struct devfreq *devfreq;
 	struct ufs_clk_scaling clk_scaling;
-	bool system_suspending;
 	bool is_sys_suspended;
 
 	enum bkops_status urgent_bkops_lvl;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH
  2026-06-05 11:20 [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH Hongjie Fang
@ 2026-06-05 15:41 ` Bart Van Assche
  2026-06-08  8:51 ` Peter Wang (王信友)
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Bart Van Assche @ 2026-06-05 15:41 UTC (permalink / raw)
  To: Hongjie Fang, alim.akhtar, avri.altman, James.Bottomley,
	martin.petersen, peter.wang, beanhuo
  Cc: linux-scsi, linux-kernel

On 6/5/26 4:20 AM, Hongjie Fang wrote:
> A PM START STOP sent from the UFS well-known LU resume path can race with
> SCSI EH.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH
  2026-06-05 11:20 [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH Hongjie Fang
  2026-06-05 15:41 ` Bart Van Assche
@ 2026-06-08  8:51 ` Peter Wang (王信友)
  2026-06-08 21:42 ` Martin K. Petersen
  2026-06-16  2:26 ` Martin K. Petersen
  3 siblings, 0 replies; 5+ messages in thread
From: Peter Wang (王信友) @ 2026-06-08  8:51 UTC (permalink / raw)
  To: beanhuo, James.Bottomley, hongjiefang, alim.akhtar, avri.altman,
	martin.petersen, bvanassche
  Cc: linux-scsi, linux-kernel

On Fri, 2026-06-05 at 19:20 +0800, Hongjie Fang wrote:
> A PM START STOP sent from the UFS well-known LU resume path can race
> with
> SCSI EH.
> 
> The "wl resume" task flow is:
>   __ufshcd_wl_resume()
>     ufshcd_set_dev_pwr_mode(UFS_ACTIVE_PWR_MODE)
>       ufshcd_execute_start_stop()
>         scsi_execute_cmd()
>           blk_execute_rq           <-- wait
>           scsi_check_passthrough() <-- may retry START STOP
> 
> If the first START STOP time out, SCSI EH may already recover the
> link and
> reset the device before scsi_execute_cmd() returns:
>   scsi_timeout()
>     scsi_eh_scmd_add()
>       scsi_error_handler()
>         scsi_unjam_host()
>           scsi_eh_ready_devs()
>             scsi_eh_host_reset()
>               ufshcd_eh_host_reset_handler()
>                 if (hba->pm_op_in_progress)
>                   ufshcd_link_recovery()
>                     ufshcd_device_reset()
>                     ufshcd_host_reset_and_restore()
>           ...
>           scsi_eh_flush_done_q()   <-- wakeup "wl resume" task
>         ...                        <-- host still in SHOST_RECOVERY
>         scsi_restart_operations()
> 
> A later passthrough retry can then run while the host is still in
> SHOST_RECOVERY and hit the SCMD_FAIL_IF_RECOVERING path:
>   scsi_queue_rq()
>     if (scsi_host_in_recovery(shost) &&
>         cmd->flags & SCMD_FAIL_IF_RECOVERING)
>       return BLK_STS_OFFLINE
> 
> That retry completes with DID_ERROR or DID_NO_CONNECT even though EH
> may
> already have restored the device to an operational ACTIVE state.
> 
> Handle these PM timeouts directly from ufshcd_eh_timed_out() instead.
> After ufshcd_link_recovery(), complete the timed-out command
> immediately
> if it has not been completed already.
> 
> For regular SCSI commands, complete them with DID_REQUEUE to match
> the
> existing MCQ force-completion semantics and allow scsi_execute_cmd()
> to
> retry if needed. For reserved internal device-management commands,
> finish
> the request with DID_TIME_OUT without calling
> ufshcd_release_scsi_cmd()
> since those commands use different resource lifetime rules.
> 
> The system_suspending flag is no longer needed because PM command
> timeout
> handling now uses pm_op_in_progress.
> 
> Fixes: b8c3a7bac9b6 ("scsi: ufs: Have midlayer retry start stop
> errors")
> Signed-off-by: Hongjie Fang <hongjiefang@asrmicro.com>
> ---

Thanks for fix this bug.
Reviewed-by: Peter Wang <peter.wang@mediatek.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH
  2026-06-05 11:20 [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH Hongjie Fang
  2026-06-05 15:41 ` Bart Van Assche
  2026-06-08  8:51 ` Peter Wang (王信友)
@ 2026-06-08 21:42 ` Martin K. Petersen
  2026-06-16  2:26 ` Martin K. Petersen
  3 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2026-06-08 21:42 UTC (permalink / raw)
  To: Hongjie Fang
  Cc: alim.akhtar, avri.altman, bvanassche, James.Bottomley,
	martin.petersen, peter.wang, beanhuo, linux-scsi, linux-kernel


Hongjie,

> A PM START STOP sent from the UFS well-known LU resume path can race
> with SCSI EH.

Applied to 7.2/scsi-staging, thanks!

-- 
Martin K. Petersen

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH
  2026-06-05 11:20 [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH Hongjie Fang
                   ` (2 preceding siblings ...)
  2026-06-08 21:42 ` Martin K. Petersen
@ 2026-06-16  2:26 ` Martin K. Petersen
  3 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2026-06-16  2:26 UTC (permalink / raw)
  To: alim.akhtar, bvanassche, James.Bottomley, peter.wang, beanhuo,
	Avri Altman, Hongjie Fang
  Cc: Martin K . Petersen, linux-scsi, linux-kernel

On Fri, 05 Jun 2026 19:20:34 +0800, Hongjie Fang wrote:

> A PM START STOP sent from the UFS well-known LU resume path can race with
> SCSI EH.
> 
> The "wl resume" task flow is:
>   __ufshcd_wl_resume()
>     ufshcd_set_dev_pwr_mode(UFS_ACTIVE_PWR_MODE)
>       ufshcd_execute_start_stop()
>         scsi_execute_cmd()
>           blk_execute_rq           <-- wait
>           scsi_check_passthrough() <-- may retry START STOP
> 
> [...]

Applied to 7.2/scsi-queue, thanks!

[1/1] scsi: ufs: core: handle PM commands timeout before SCSI EH
      https://git.kernel.org/mkp/scsi/c/01d5e237b339

-- 
Martin K. Petersen

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-16  2:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-05 11:20 [PATCH v5] scsi: ufs: core: handle PM commands timeout before SCSI EH Hongjie Fang
2026-06-05 15:41 ` Bart Van Assche
2026-06-08  8:51 ` Peter Wang (王信友)
2026-06-08 21:42 ` Martin K. Petersen
2026-06-16  2:26 ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®