* Re: [PATCH] PM: runtime: Update runtime_pm.rst for non-fatal resume errors
2026-10-29 7:11 [PATCH] PM: runtime: Update runtime_pm.rst for non-fatal resume errors Stanley Jhu
@ 2026-11-03 19:47 ` Brian Norris
0 siblings, 0 replies; 2+ messages in thread
From: Brian Norris @ 2026-11-03 19:47 UTC (permalink / raw)
To: Stanley Jhu
Cc: Rafael J . Wysocki, Len Brown, Pavel Machek, Praveen Talari,
Jonathan Corbet, Mark Brown, Alan Stern, Jens Axboe,
Bart Van Assche, linux-pm, linux-doc, linux-block, linux-scsi,
linux-kernel
Hi Stanley,
On Thu, Oct 29, 2026 at 03:11:45PM +0800, Stanley Jhu wrote:
> Since commit eef1d74bf88f ("PM: runtime: Only set runtime_error on
> suspend callback failures"), rpm_callback() no longer sets
> dev->power.runtime_error when ->runtime_resume() returns an error.
> However, Documentation/power/runtime_pm.rst still states that the PM
> core treats any ->runtime_resume() error as a fatal device error and
> refuses to run runtime PM helper functions until the device status is
> manually reset.
>
> Update the resume callback semantics bullet to state that on error the
> PM core resets the runtime PM status to 'suspended', returns the error
> code from synchronous resume helpers, and leaves power.runtime_error
> unset so a subsequent resume request can invoke ->runtime_resume()
> again. Also drop ->runtime_resume() from the ->runtime_idle()
> comparison of callbacks whose negative return codes are treated as fatal
> device errors.
>
> Fixes: eef1d74bf88f ("PM: runtime: Only set runtime_error on suspend callback failures")
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
> ---
> This updates Documentation/power/runtime_pm.rst as discussed in the v3
> thread for commit eef1d74bf88f:
> https://lore.kernel.org/r/CAJZ5v0ga8qqYg79+hQVtO8T=FOCj8XLmTz3HpEKoOykV_K-HSQ@mail.gmail.com
>
> Two .runtime_resume() paths assumed the old latching behavior:
> - blk_pre_runtime_resume() (q->pm_only underflow):
> https://lore.kernel.org/r/20261029040001.2100001-1-stanleyjhu@google.com
> - __ufshcd_wl_resume() in UFS, where after a failed resume the retried
> resume releases the clock-gating hold a second time:
> https://lore.kernel.org/r/20261029040002.2100002-1-stanleyjhu@google.com
Those need a close eye, as the code involved is a bit subtle. I tried to
review, but I'm not really an expert in those subsystems. (I'm also not
quite sure the second one was 100% a regression, vs further exposing an
existing underlying problem.)
> Should other .runtime_resume() callbacks that undo suspend-time state
> on failure, assuming no retry, be audited?
Sure, that seems reasonable (although difficult), especially considering
you seem to have identified real problems here already. I don't know
that you'll get a formal sign-up from anyone here for that work, but
you're free to do it yourself :)
> Documentation/power/runtime_pm.rst | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
> index 40a5c23d5259..04dc2a538352 100644
> --- a/Documentation/power/runtime_pm.rst
> +++ b/Documentation/power/runtime_pm.rst
> @@ -222,11 +222,12 @@ what to do to handle the device).
> I/O operations as needed. The runtime PM status of the device is then
> 'active'.
>
> - * If the resume callback returns an error code, the PM core regards this as a
> - fatal error and will refuse to run the helper functions described in
> - `Section 4`_ for the device, until its status is directly set to either
> - 'active', or 'suspended' (by means of special helper functions provided by
> - the PM core for this purpose).
> + * If the resume callback returns an error code, the PM core sets the
> + device's runtime PM status back to 'suspended' and returns the error
> + code from synchronous resume helpers.
This part seems good.
> As with a suspend callback
> + returning -EBUSY or -EAGAIN, this does not set power.runtime_error,
I think this is a weird comparison to make, for two reasons:
1) we don't talk about power.runtime_error anywhere else in this doc.
That's probably a good thing, since it's not really something any
consumers should need to inspect in most cases. Instead, the
runtime_suspend() section you're referring to mentions "remains
'active'" (i.e., can be retried later) or "fatal error" and "will
refuse to run [...] until [...]" (i.e., not retriable).
2) -EBUSY and -EAGAIN aren't relevant here at all. All error codes are
now treated the same for runtime_resume(). (I know you aren't
claiming they matter in runtime_resume(); but the wording seems a bit
cloudy to me.)
I think it would be better to flip the comparison to the other
direction, and deemphasize power.runtime_error. For example:
"Unlike with the suspend callback, failures in the resume callback are
never considered fatal, [...]"
> so
> + the helper functions described in `Section 4`_ are not blocked and a
> + subsequent resume request can run the resume callback again.
Looks good to me.
> The idle callback (a subsystem-level one, if present, or the driver one) is
> executed by the PM core whenever the device appears to be idle, which is
> @@ -250,9 +251,9 @@ call to pm_runtime_autosuspend().
> To prevent this suspension (for example, if the callback routine has scheduled
> a delayed suspend or determined the device cannot be idle), the routine must
> return a non-zero value (typically -EBUSY or -EAGAIN). Unlike
> -->runtime_suspend() and ->runtime_resume(), the PM core does not treat negative
> -return codes from ->runtime_idle() as a fatal device error; any non-zero value
> -simply stops the PM core from suspending the device.
> +->runtime_suspend(), the PM core does not treat negative return codes from
> +->runtime_idle() as a fatal device error, and any non-zero value simply stops
> +the PM core from suspending the device.
Looks good. Sorry, I think I missed this part when updating this
language recently in commit aedc51762a0d ("PM: runtime: Clarify
->runtime_idle() callback return value handling").
Brian
>
> Core Guarantees and Synchronization Rules
> -----------------------------------------
> --
> 2.56.0.313.g2479322e68-goog
>
^ permalink raw reply [flat|nested] 2+ messages in thread