Re: [RFC PATCH v3 5/8] pm: runtime: Introduce pm_runtime_get_if_not_suspended()

[Pranjal Shrivastava <praan@google.com>]

On Wed, Jul 09, 2025 at 08:44:06AM +0200, Rafael J. Wysocki wrote:
> On Mon, Jun 16, 2025 at 10:32 PM Pranjal Shrivastava <praan@google.com> wrote:
> >
> > The existing opportunistic helpers like pm_runtime_get_if_active() and
> > pm_runtime_get_if_in_use(), are too strict for certain use cases. They
> > fail if the device is in a transient state like RPM_SUSPENDING, which
> > can lead to drivers making incorrect assumptions about the dev's state.
> >
> > These helpers don't suffice for cases where one wishes to elide HW clean
> > up like queue flushes or TLB invalidations if the device is powered off.
> > It is wasteful to wake up the device in cases where the resume callback
> > resets the device well OR if the HW resets in a clean state. Thus, if a
>ppy to hear your feedback or any alternative ideas you might have > device is powered off, it is preferred to elide any clean-up HW ops like
> > queue flushes / TLB invalidations when the device will resume afresh.
> >
> > Consider the following sequence of operations:
> >
> > 1. The device is in `RPM_SUSPENDING` state
> > 2. The driver calls pm_runtime_get_if_active/in_use
> > 3. Depending on these API, the driver elides a HW clean-up op like:
> >
> > if (pm_runtime_get_if_in_active(dev))
> >         invalidate_tlb(dev);
> > else
> >         // Skip flush, assuming device will fully suspend
> >
> > 4. Now, another rpm dev-linked device wakes up, causing the device's
> >    state to bounce from RPM_SUSPENDING to RPM_ACTIVE without invoking
> >    any rpm callbacks, preventing them from resetting the dev correctly.
> 
> This never happens.
> 
> If the status is RPM_SUSPENDING, the runtime suspend callback will be invoked.

Ack, I believe we're talking about the check[1] in rpm_resume here:

	if (dev->power.runtime_status == RPM_RESUMING ||
	    dev->power.runtime_status == RPM_SUSPENDING) {

	    [...]
	    /* Wait for the operation carried out in parallel with us. */
	    [...]
	    finish_wait(&dev->power.wait_queue, &wait);
	    goto repeat;

However, my worry is about the following situation:

  rpm_suspend			   rpm_resume

  rpm_status = RPM_SUSPENDING 	
  				   if (RPM_SUSPENDING)
				   prepare_to wait(...)

  // suspend fails
  retval = rpm_callback(...);
  goto fail;
  [...]
fail:
  rpm_status = RPM_ACTIVE;
  				   finish_wait(...);
				   goto repeat;
				   repeat:
				   if (rpm_status == RPM_ACTIVE) {
					retval = 1;
					goto out;
				   }
				out:
				   [ ... ] // put_parent if one

				   // we return without resume cb();
				   return retval;

Now, if we rely on APIs based on pm_runtime_get_conditional(), which
might return 0 if (dev->power.runtime_status != RPM_ACTIVE), we might
end up eliding some TLB invalidations in the period where the status was
RPM_SUSPENDING till it's back to RPM_ACTIVE due to a failed suspend.
This can cause some severe address-aliasing/ghost hit issues since the
TLB still has some stale entries..

Instead, we'd like to ensure that the status IS suspended to make a
decision to elide the TLBI or not.

What are your thoughts on this? I'm open to approaching it differently.
Happy to hear your feedback or any alternative ideas you might have

Thanks
Praan

[1] https://elixir.bootlin.com/linux/v6.16-rc5/source/drivers/base/power/runtime.c#L808