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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx # v7.3+
> Reviewed-by: Bart Van Assche <bvanassche@xxxxxxx>
> Signed-off-by: Stanley Jhu <stanleyjhu@xxxxxxxxxx>

LGTM

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

> ---
> 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@xxxxxxxxxx
>
>  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
>