mirror of https://lore.kernel.org/lkml/
 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; 5+ 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] 5+ 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
       [not found]   ` <sashiko-outbox-198692@kernel.org>
  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; 5+ 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	[flat|nested] 5+ 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
       [not found]   ` <sashiko-outbox-198693@kernel.org>
  1 sibling, 1 reply; 5+ 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	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] scsi: ufs: core: Skip redundant variant resume after error recovery power-up
       [not found]   ` <sashiko-outbox-198692@kernel.org>
@ 2026-11-05 14:43     ` Stanley Jhu
  0 siblings, 0 replies; 5+ 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] 5+ messages in thread

* Re: [PATCH 2/2] scsi: ufs: core: Wake consumer queues on suspended W-LUN recovery
       [not found]   ` <sashiko-outbox-198693@kernel.org>
@ 2026-11-05 14:43     ` Stanley Jhu
  0 siblings, 0 replies; 5+ 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] 5+ messages in thread

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

Thread overview: 5+ 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
     [not found]   ` <sashiko-outbox-198692@kernel.org>
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
     [not found]   ` <sashiko-outbox-198693@kernel.org>
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

all inboxes | Powered by JetHome®