Linux SCSI subsystem development
 help / color / mirror / Atom feed
* [PATCH 0/2] scsi: ufs: core: Fix post-reset runtime PM recovery after eef1d74bf88f
@ 2026-11-05  7:15 Stanley Jhu
  2026-11-05  7:15 ` [PATCH 1/2] scsi: ufs: core: Skip redundant variant resume after error recovery power-up Stanley Jhu
  2026-11-05  7:15 ` [PATCH 2/2] scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery Stanley Jhu
  0 siblings, 2 replies; 7+ messages in thread
From: Stanley Jhu @ 2026-11-05  7:15 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Brian Norris, Can Guo,
	Peter Wang, John Garry, Rafael J . Wysocki, Praveen Talari,
	linux-scsi, linux-pm, linux-kernel, stable, Stanley Jhu

Since commit eef1d74bf88f ("PM: runtime: Only set runtime_error on
suspend callback failures"), rpm_resume() resets ufs_device_wlun to
RPM_SUSPENDED and leaves power.runtime_error at 0 when W-LUN runtime
resume fails. When error handling subsequently resets and restores the
host and link, two runtime PM recovery gaps are exposed:

1. ufshcd_err_handling_prepare() calls ufshcd_vops_resume() while
   ufs_device_wlun remains RPM_SUSPENDED, so the next ufs_device_wlun
   runtime resume calls ufshcd_vops_resume() a second consecutive time
   without an intervening ufshcd_vops_suspend(), leaking core_clk and
   iface_clk enable counts in ufs_qcom_resume() via qcom_ice_resume().
2. ufshcd_recover_pm_error() checks only power.runtime_error and skips
   consumer queue recovery, leaving waking I/O blocked in
   __bio_queue_enter() until another request touches the same queue.

Patch 1 tracks POST_CHANGE variant suspend state in
hba->is_vops_suspended so ufshcd_vops_resume() pairs 1:1 with
ufshcd_vops_suspend() across error recovery. Patch 2 wakes consumer
request queues when ufs_device_wlun is runtime-suspended in
ufshcd_recover_pm_error() so consumer pm_request_resume() resumes
ufs_device_wlun via the runtime PM device link.

This series applies on top of two prerequisite patches so repeated
post-failure W-LUN runtime resumes keep wlun q->pm_only and
clk_gating.active_reqs balanced:
- Message-ID: <20261104112744.1630864-1-stanleyjhu@google.com>
  ("[PATCH v2] block: Keep RPM_ACTIVE in blk_pre_runtime_resume() when
  already active")
- Message-ID: <20261029040002.2100002-1-stanleyjhu@google.com>
  ("scsi: ufs: core: Keep clock-gating hold on failed W-LUN runtime
  resume")

Tested: QEMU ARM64 UFS in SDB and MCQ modes with injected W-LUN
runtime-resume START STOP UNIT and fatal UIC error recovery, verifying
immediate consumer wake, 1:1 variant suspend/resume pairing, and idle
clock gating.

Stanley Jhu (2):
  scsi: ufs: core: Skip redundant variant resume after error recovery
    power-up
  scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery

 drivers/ufs/core/ufshcd-priv.h | 21 ++++++++++++++++-----
 drivers/ufs/core/ufshcd.c      |  4 +++-
 include/ufs/ufshcd.h           |  2 ++
 3 files changed, 21 insertions(+), 6 deletions(-)

-- 
2.56.0.385.gac671c819a-goog


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

* [PATCH 1/2] scsi: ufs: core: Skip redundant variant resume after error recovery power-up
  2026-11-05  7:15 [PATCH 0/2] scsi: ufs: core: Fix post-reset runtime PM recovery after eef1d74bf88f Stanley Jhu
@ 2026-11-05  7:15 ` Stanley Jhu
  2026-11-05  7:42   ` sashiko-bot
  2026-11-05  7:15 ` [PATCH 2/2] scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery Stanley Jhu
  1 sibling, 1 reply; 7+ messages in thread
From: Stanley Jhu @ 2026-11-05  7:15 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Brian Norris, Can Guo,
	Peter Wang, John Garry, Rafael J . Wysocki, Praveen Talari,
	linux-scsi, linux-pm, linux-kernel, stable, Stanley Jhu

Since commit eef1d74bf88f ("PM: runtime: Only set runtime_error on
suspend callback failures"), a failed W-LUN runtime resume leaves
ufs_device_wlun in RPM_SUSPENDED with power.runtime_error == 0. If
error recovery then powers up the host, ufshcd_vops_resume() is called
twice in a row without an intervening ufshcd_vops_suspend():

1. __ufshcd_wl_suspend() runs ufshcd_vops_suspend() at POST_CHANGE, and
   a failed W-LUN runtime resume rolls back through vendor_suspend: to
   keep the variant suspended while ufs_device_wlun stays RPM_SUSPENDED.
2. ufshcd_err_handling_prepare() sees ufs_device_wlun suspended and
   calls ufshcd_vops_resume(), while ufshcd_recover_pm_error() leaves
   ufs_device_wlun in RPM_SUSPENDED.
3. The next ufs_device_wlun runtime resume calls ufshcd_vops_resume()
   again with no ufshcd_vops_suspend() in between, leaking core_clk and
   iface_clk enable counts in ufs_qcom_resume() via qcom_ice_resume().

Track POST_CHANGE variant suspend state in hba->is_vops_suspended and
skip ufshcd_vops_resume() when already resumed so variant suspend and
resume callbacks remain paired 1:1 across error recovery.

Fixes: eef1d74bf88f ("PM: runtime: Only set runtime_error on suspend callback failures")
Cc: stable@vger.kernel.org # v7.3+
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd-priv.h | 21 ++++++++++++++++-----
 include/ufs/ufshcd.h           |  2 ++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index e55c2a02c1f5..9e4d8174a0f0 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -236,20 +236,31 @@ 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)
+				      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->is_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->is_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->is_vops_suspended = false;
 
-	return 0;
+	return ret;
 }
 
 static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba)
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index be62165b785e..cdcaccf9201b 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1038,6 +1038,7 @@ 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
+ * @is_vops_suspended: tracks whether variant POST_CHANGE suspend is active
  * @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 +1227,7 @@ struct ufs_hba {
 	struct devfreq *devfreq;
 	struct ufs_clk_scaling clk_scaling;
 	bool is_sys_suspended;
+	bool is_vops_suspended;
 
 	enum bkops_status urgent_bkops_lvl;
 	bool is_urgent_bkops_lvl_checked;
-- 
2.56.0.385.gac671c819a-goog


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

* [PATCH 2/2] scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery
  2026-11-05  7:15 [PATCH 0/2] scsi: ufs: core: Fix post-reset runtime PM recovery after eef1d74bf88f Stanley Jhu
  2026-11-05  7:15 ` [PATCH 1/2] scsi: ufs: core: Skip redundant variant resume after error recovery power-up Stanley Jhu
@ 2026-11-05  7:15 ` Stanley Jhu
  2026-11-05  7:43   ` sashiko-bot
  1 sibling, 1 reply; 7+ messages in thread
From: Stanley Jhu @ 2026-11-05  7:15 UTC (permalink / raw)
  To: Martin K . Petersen, James E . J . Bottomley
  Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Brian Norris, Can Guo,
	Peter Wang, John Garry, Rafael J . Wysocki, Praveen Talari,
	linux-scsi, linux-pm, linux-kernel, stable, Stanley Jhu

Since commit eef1d74bf88f ("PM: runtime: Only set runtime_error on
suspend callback failures"), a failed W-LUN runtime resume leaves
ufs_device_wlun in RPM_SUSPENDED with power.runtime_error == 0. When
error recovery resets and restores the host, ufshcd_recover_pm_error()
misses consumer queue recovery and leaves waking I/O stuck:

1. Consumer I/O on a runtime-suspended logical unit blocks in
   __bio_queue_enter() with pm_only set, queues pm_request_resume(), and
   resumes supplier ufs_device_wlun, whose START STOP UNIT fails.
2. Consumer runtime resume aborts with pm_only still set while
   ufshcd_err_handler() resets and restores the host and link, then
   calls ufshcd_recover_pm_error().
3. ufshcd_recover_pm_error() checks only power.runtime_error and skips
   consumer queue recovery, leaving waking I/O blocked in
   __bio_queue_enter() until another request touches the same queue.

Set resume_sdev_queues when ufs_device_wlun is runtime-suspended in
ufshcd_recover_pm_error() so consumer pm_request_resume() resumes
ufs_device_wlun via the runtime PM device link and clears consumer
pm_only through sdev_runtime_resume().

Fixes: eef1d74bf88f ("PM: runtime: Only set runtime_error on suspend callback failures")
Cc: stable@vger.kernel.org # v7.3+
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
Prerequisite patches (for mainline and stable backports):
- Message-ID: <20261104112744.1630864-1-stanleyjhu@google.com>
  ("block: Keep RPM_ACTIVE in blk_pre_runtime_resume() when already
  active")
- Message-ID: <20261029040002.2100002-1-stanleyjhu@google.com>
  ("scsi: ufs: core: Keep clock-gating hold on failed W-LUN runtime
  resume")

 drivers/ufs/core/ufshcd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 4f0301d2a5d4..880a14a2c5a4 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6953,7 +6953,9 @@ static void ufshcd_recover_pm_error(struct ufs_hba *hba)
 		resume_sdev_queues = true;
 	}
 
-	if (hba->ufs_device_wlun->sdev_gendev.power.runtime_error) {
+	if (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev)) {
+		resume_sdev_queues = true;
+	} else if (hba->ufs_device_wlun->sdev_gendev.power.runtime_error) {
 		/*
 		 * starget, parent of wlun, might be suspended if wlun resume failed.
 		 * Make sure parent is resumed before set child (wlun) active.
-- 
2.56.0.385.gac671c819a-goog


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

* Re: [PATCH 1/2] scsi: ufs: core: Skip redundant variant resume after error recovery power-up
  2026-11-05  7:15 ` [PATCH 1/2] scsi: ufs: core: Skip redundant variant resume after error recovery power-up Stanley Jhu
@ 2026-11-05  7:42   ` sashiko-bot
  2026-11-05 14:43     ` Stanley Jhu
  0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-11-05  7:42 UTC (permalink / raw)
  To: Stanley Jhu; +Cc: linux-scsi

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ufs: core: W-LUN left RPM_SUSPENDED after error recovery reset
--

commit 2e7bb517d17be41f390aa16033a12c2fb8953c6f
Author: Stanley Jhu <stanleyjhu@google.com>

scsi: ufs: core: Skip redundant variant resume after error recovery power-up

This commit tracks POST_CHANGE variant suspend state in
hba->is_vops_suspended so ufshcd_vops_resume() is skipped if the variant is
already resumed when ufs_device_wlun remains RPM_SUSPENDED across error
recovery.

> diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
> index e55c2a02c1f50..9e4d8174a0f08 100644
> --- a/drivers/ufs/core/ufshcd-priv.h
> +++ b/drivers/ufs/core/ufshcd-priv.h
> @@ -236,20 +236,31 @@ 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)
> +				      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->is_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->is_vops_suspended)
> +		return 0;

[Severity: High]
Does skipping ufshcd_vops_resume() here leave ufs_device_wlun out of sync
with the actual hardware and clock-gating state when error recovery resets
and restores the host after a failed W-LUN runtime resume?

When __ufshcd_wl_suspend() suspends the W-LUN, it calls ufshcd_hold(hba)
(incrementing hba->clk_gating.active_reqs) and sets
hba->clk_gating.is_suspended = true. If a subsequent __ufshcd_wl_resume()
fails (for example, in ufshcd_set_dev_pwr_mode()), it rolls back to
vendor_suspend: and exits through out:, which clears is_suspended and drops
the clock-gating hold without a matching ufshcd_hold(hba) on future resume
attempts:

drivers/ufs/core/ufshcd.c:__ufshcd_wl_resume() {
	...
vendor_suspend:
	ufshcd_vops_suspend(hba, pm_op, POST_CHANGE);
out:
	if (ret)
		ufshcd_update_evt_hist(hba, UFS_EVT_WL_RES_ERR, (u32)ret);
	hba->clk_gating.is_suspended = false;
	ufshcd_release(hba);
	hba->pm_op_in_progress = false;
	return ret;
}

Because commit eef1d74bf88f ("PM: runtime: Only set runtime_error on
suspend callback failures") leaves ufs_device_wlun->sdev_gendev in
RPM_SUSPENDED with power.runtime_error == 0, every subsequent call to
__ufshcd_wl_resume() without an intervening __ufshcd_wl_suspend() (first
from ufshcd_err_handling_prepare() -> ufshcd_rpm_get_sync(), and again on
the next runtime resume after recovery) calls ufshcd_release(hba) without
a matching ufshcd_hold(hba), underflowing hba->clk_gating.active_reqs.

Furthermore, during ufshcd_err_handler(), ufshcd_err_handling_prepare()
powers on the regulators, clocks, and variant, and ufshcd_reset_and_restore()
restores the controller, setting hba->uic_link_state = UIC_LINK_ACTIVE_STATE
and hba->curr_dev_pwr_mode = UFS_ACTIVE_PWR_MODE. However,
ufshcd_recover_pm_error() only marks ufs_device_wlun active if
power.runtime_error is set:

drivers/ufs/core/ufshcd.c:ufshcd_recover_pm_error() {
	...
	if (hba->ufs_device_wlun->sdev_gendev.power.runtime_error) {
		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;
	}
	...
}

With power.runtime_error == 0, ufs_device_wlun, starget->dev, and
shost->shost_gendev remain in RPM_SUSPENDED (and consumer SCSI device
queues are not resumed here, though a later commit wakes consumer queues).
When ufshcd_err_handling_unprepare() then finishes:

drivers/ufs/core/ufshcd.c:ufshcd_err_handling_unprepare() {
	...
	if (ufshcd_is_clkscaling_supported(hba))
		ufshcd_clk_scaling_suspend(hba, false);
	ufshcd_rpm_put(hba);
	pm_runtime_put(hba->dev);
}

ufshcd_rpm_put(hba) will not run __ufshcd_wl_suspend() because
ufs_device_wlun is already RPM_SUSPENDED, and pm_runtime_put(hba->dev) can
runtime suspend hba->dev via ufshcd_suspend() while the UFS device is in
UFS_ACTIVE_PWR_MODE, the UniPro link is in UIC_LINK_ACTIVE_STATE, devfreq
clock scaling is active, and the variant is resumed.

Should ufshcd_recover_pm_error() instead transition ufs_device_wlun back to
RPM_ACTIVE when ufshcd_reset_and_restore() has powered up the device and
link, rather than leaving ufs_device_wlun in RPM_SUSPENDED with the hardware
and variant resumed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105071518.1458428-1-stanleyjhu@google.com?part=1

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

* Re: [PATCH 2/2] scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery
  2026-11-05  7:15 ` [PATCH 2/2] scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery Stanley Jhu
@ 2026-11-05  7:43   ` sashiko-bot
  2026-11-05 14:43     ` Stanley Jhu
  0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-11-05  7:43 UTC (permalink / raw)
  To: Stanley Jhu; +Cc: linux-scsi

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: ufs: core: premature HBA runtime suspend in ufshcd_recover_pm_error()
- [High] scsi: ufs: core: missed queue wake in ufshcd_recover_pm_error() after prepare
--

commit ea7f240ee0c25a5e5c4a46de9b52655deaa5da4e
Author: Stanley Jhu <stanleyjhu@google.com>

scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery

This commit updates ufshcd_recover_pm_error() to set resume_sdev_queues
when ufs_device_wlun is runtime-suspended, so consumer queues blocked in
__bio_queue_enter() after a failed W-LUN runtime resume are woken via
pm_request_resume().

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index fd52deb72d21c..0701ec408736d 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6953,7 +6953,9 @@ static void ufshcd_recover_pm_error(struct ufs_hba *hba)
>  		resume_sdev_queues = true;
>  	}
>  
> -	if (hba->ufs_device_wlun->sdev_gendev.power.runtime_error) {
> +	if (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev)) {
> +		resume_sdev_queues = true;

[Severity: High]
Can leaving ufs_device_wlun in RPM_SUSPENDED here cause hba->dev to
prematurely runtime-suspend before the asynchronous consumer resume work runs?

When W-LUN runtime resume fails, rpm_resume() leaves ufs_device_wlun in
RPM_SUSPENDED with power.runtime_error == 0 and drops the reference on its
parent starget->dev, allowing starget->dev and shost->shost_gendev to remain
RPM_SUSPENDED with hba->dev->power.child_count == 0.

In ufshcd_err_handler(), ufshcd_err_handling_prepare() takes
pm_runtime_get_sync(hba->dev), powers on regulators/clocks/IRQs, and calls
ufshcd_vops_resume(), and then ufshcd_reset_and_restore() resets the host and
restores hba->curr_dev_pwr_mode = UFS_ACTIVE_PWR_MODE and
hba->uic_link_state = UIC_LINK_ACTIVE_STATE.

Unlike the power.runtime_error branch below:

drivers/ufs/core/ufshcd.c:ufshcd_recover_pm_error() {
	...
	} else if (hba->ufs_device_wlun->sdev_gendev.power.runtime_error) {
		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;
	}
}

which synchronously resumes starget->dev and sets ufs_device_wlun active (so
starget->dev, shost->shost_gendev, and hba->dev all have power.child_count ==
1), this new branch only sets resume_sdev_queues = true and queues
asynchronous pm_request_resume(q->dev) work on pm_wq (or queues nothing if no
consumer LUN has q->rpm_status in RPM_SUSPENDED or RPM_SUSPENDING).

Immediately after ufshcd_recover_pm_error() returns,
ufshcd_err_handling_unprepare() runs:

drivers/ufs/core/ufshcd.c:ufshcd_err_handling_unprepare() {
	...
	if (ufshcd_is_clkscaling_supported(hba))
		ufshcd_clk_scaling_suspend(hba, false);
	ufshcd_rpm_put(hba);
	pm_runtime_put(hba->dev);
}

Because ufs_device_wlun, starget->dev, and shost->shost_gendev are still
RPM_SUSPENDED, pm_runtime_put(hba->dev) drops hba->dev->power.usage_count to 0
while hba->dev->power.child_count is 0. Wouldn't that allow
ufshcd_runtime_suspend() -> ufshcd_suspend() to disable host IRQs, gate clocks
to CLKS_OFF, and put vreg and vdd_hba into LPM while the UFS device and UniPro
link are still in UFS_ACTIVE_PWR_MODE and UIC_LINK_ACTIVE_STATE (without
__ufshcd_wl_suspend() or ufshcd_vops_suspend() having run), and then cause a
subsequent __ufshcd_wl_resume() to skip controller/link reinitialization
because both ufshcd_is_link_hibern8(hba) and ufshcd_is_link_off(hba) are
false?

> +	} else if (hba->ufs_device_wlun->sdev_gendev.power.runtime_error) {

[Severity: High]
What happens if ufshcd_rpm_get_sync(hba) in ufshcd_err_handling_prepare()
already transitioned ufs_device_wlun to RPM_ACTIVE before
ufshcd_recover_pm_error() is reached?

When a consumer-initiated runtime resume of ufs_device_wlun fails, commit
eef1d74bf88f leaves ufs_device_wlun in RPM_SUSPENDED with
power.runtime_error == 0, while the consumer queue remains blocked in
__bio_queue_enter() with q->pm_only set and q->rpm_status == RPM_SUSPENDED.

When ufshcd_err_handler() subsequently runs, it calls
ufshcd_err_handling_prepare() before ufshcd_set_eh_in_progress():

drivers/ufs/core/ufshcd.c:ufshcd_err_handling_prepare() {
	pm_runtime_get_sync(hba->dev);
	ufshcd_rpm_get_sync(hba);
	if (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev) ||
	...
}

Because power.runtime_error is 0, ufshcd_rpm_get_sync(hba) invokes
__ufshcd_wl_resume() a second time instead of failing immediately with
-EINVAL. That second __ufshcd_wl_resume() call can succeed and transition
ufs_device_wlun to RPM_ACTIVE: for example, when hba->ufshcd_state is
UFSHCD_STATE_EH_SCHEDULED_NON_FATAL (which ufshcd_queuecommand() allows before
ufshcd_set_eh_in_progress() and which later triggers needs_reset = true in
ufshcd_err_handler()), or when __ufshcd_wl_resume() recovers the link via
ufshcd_link_recovery() or ufshcd_reset_and_restore():

drivers/ufs/core/ufshcd.c:__ufshcd_wl_resume() {
	...
			ret = ufshcd_link_recovery(hba);
			if (ret)
				goto vendor_suspend;
		}
	} else if (ufshcd_is_link_off(hba)) {
		ret = ufshcd_reset_and_restore(hba);
	...
}

Once ufshcd_rpm_get_sync(hba) in ufshcd_err_handling_prepare() sets
ufs_device_wlun to RPM_ACTIVE, both
pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev) and
hba->ufs_device_wlun->sdev_gendev.power.runtime_error will be false in
ufshcd_recover_pm_error() (or ufshcd_err_handling_should_stop() will skip the
do_reset path altogether if __ufshcd_wl_resume() already ran
ufshcd_reset_and_restore()). Wouldn't that leave resume_sdev_queues false and
leave the consumer queue stuck in __bio_queue_enter()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105071518.1458428-1-stanleyjhu@google.com?part=2

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

* Re: [PATCH 1/2] scsi: ufs: core: Skip redundant variant resume after error recovery power-up
  2026-11-05  7:42   ` sashiko-bot
@ 2026-11-05 14:43     ` Stanley Jhu
  0 siblings, 0 replies; 7+ messages in thread
From: Stanley Jhu @ 2026-11-05 14:43 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Martin K . Petersen, James E . J . Bottomley, Alim Akhtar,
	Bart Van Assche, Bean Huo, Brian Norris, Can Guo, Peter Wang,
	John Garry, Rafael J . Wysocki, Praveen Talari, linux-scsi,
	linux-pm, linux-kernel, stable, Stanley Jhu

On Thu, Nov 05, 2026 at 07:42:29AM +0000, sashiko-bot@kernel.org wrote:
> If a subsequent __ufshcd_wl_resume()
> fails (for example, in ufshcd_set_dev_pwr_mode()), it rolls back to
> vendor_suspend: and exits through out:, which clears is_suspended and drops
> the clock-gating hold without a matching ufshcd_hold(hba) on future resume
> attempts:

This review was run without the prerequisite listed in the cover
letter, "scsi: ufs: core: Keep clock-gating hold on failed W-LUN
runtime resume". With it applied, a failed runtime resume keeps
is_suspended and the hold, so active_reqs does not underflow.

> However,
> ufshcd_recover_pm_error() only marks ufs_device_wlun active if
> power.runtime_error is set:

Agreed. This will be fixed in v2.

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

* Re: [PATCH 2/2] scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery
  2026-11-05  7:43   ` sashiko-bot
@ 2026-11-05 14:43     ` Stanley Jhu
  0 siblings, 0 replies; 7+ messages in thread
From: Stanley Jhu @ 2026-11-05 14:43 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Martin K . Petersen, James E . J . Bottomley, Alim Akhtar,
	Bart Van Assche, Bean Huo, Brian Norris, Can Guo, Peter Wang,
	John Garry, Rafael J . Wysocki, Praveen Talari, linux-scsi,
	linux-pm, linux-kernel, stable, Stanley Jhu

On Thu, Nov 05, 2026 at 07:43:05AM +0000, sashiko-bot@kernel.org wrote:
> Can leaving ufs_device_wlun in RPM_SUSPENDED here cause hba->dev to
> prematurely runtime-suspend before the asynchronous consumer resume work runs?

Agreed. This will be fixed in v2.

> Wouldn't that leave resume_sdev_queues false and
> leave the consumer queue stuck in __bio_queue_enter()?

Agreed. This will also be fixed in v2.

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

end of thread, other threads:[~2026-11-05 14:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-05  7:15 [PATCH 0/2] scsi: ufs: core: Fix post-reset runtime PM recovery after eef1d74bf88f Stanley Jhu
2026-11-05  7:15 ` [PATCH 1/2] scsi: ufs: core: Skip redundant variant resume after error recovery power-up Stanley Jhu
2026-11-05  7:42   ` sashiko-bot
2026-11-05 14:43     ` Stanley Jhu
2026-11-05  7:15 ` [PATCH 2/2] scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery Stanley Jhu
2026-11-05  7:43   ` sashiko-bot
2026-11-05 14:43     ` Stanley Jhu

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