[PATCH v2] block: Keep RPM_ACTIVE in blk_pre_runtime_resume() when already active

[PATCH v2] 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.

Tested on QEMU ARM64 UFS in SDB and MCQ modes by injecting
runtime-resume START STOP UNIT failures on the UFS device well-known
LUN, confirming zero pm_only underflows and balanced runtime PM cycles.

Fixes: eef1d74bf88f ("PM: runtime: Only set runtime_error on suspend callback failures")
Cc: stable@vger.kernel.org # v7.3+
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
Differences from v1:
- Move SUSPENDED -> RESUMING explicitly in blk_pre_runtime_resume() and
  clear pm_only only for RPM_RESUMING in blk_post_runtime_resume()
- Describe the QEMU UFS test coverage in the commit message
- Link to v1: https://lore.kernel.org/r/20261029040001.2100001-1-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.385.gac671c819a-goog

Re: [PATCH v2] block: Keep RPM_ACTIVE in blk_pre_runtime_resume() when already active

[Rafael J. Wysocki (Intel)]

On Wed, Nov 4, 2026 at 12:27 PM Stanley Jhu <stanleyjhu@google.com> wrote:
>
> 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.
>
> Tested on QEMU ARM64 UFS in SDB and MCQ modes by injecting
> runtime-resume START STOP UNIT failures on the UFS device well-known
> LUN, confirming zero pm_only underflows and balanced runtime PM cycles.
>
> Fixes: eef1d74bf88f ("PM: runtime: Only set runtime_error on suspend callback failures")
> Cc: stable@vger.kernel.org # v7.3+
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>

LGTM

Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

> ---
> Differences from v1:
> - Move SUSPENDED -> RESUMING explicitly in blk_pre_runtime_resume() and
>   clear pm_only only for RPM_RESUMING in blk_post_runtime_resume()
> - Describe the QEMU UFS test coverage in the commit message
> - Link to v1: https://lore.kernel.org/r/20261029040001.2100001-1-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.385.gac671c819a-goog
>

Re: [PATCH v2] block: Keep RPM_ACTIVE in blk_pre_runtime_resume() when already active

[Bart Van Assche]

On 11/4/26 3:27 AM, Stanley Jhu wrote:
> Differences from v1:
> - Move SUSPENDED -> RESUMING explicitly in blk_pre_runtime_resume() and
>    clear pm_only only for RPM_RESUMING in blk_post_runtime_resume()

Hi Stanley,

Thank you for having addressed my concerns. There is an existing
issues that you may want to address with a future patch: Queue ACTIVE,
device suspended: after a failed resume, the queue is ACTIVE with
pm_only == 0 while the device is still RPM_SUSPENDED. So
blk_pm_resume_queue() never asks for a resume, and normal I/O goes to
a device the PM core thinks is suspended. A retry only happens if
something else calls pm_runtime_get*(). Commit 6e1fcab00a23 introduced
this behavior on purpose.

Thanks,

Bart.