Re: [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 3 Nov 2026 16:32:44 -0800
Brian Norris <briannorris@chromium.org> wrote:

> Hi Jonathan,
> 
> On Tue, Nov 03, 2026 at 11:17:19PM +0000, Jonathan Cameron wrote:
> > On Tue,  3 Nov 2026 14:14:41 -0800
> > Brian Norris <briannorris@chromium.org> wrote:
> >   
> > > When a child device suspends, it does not update the last_busy timestamp
> > > for its parent. If that parent configured autosuspend and didn't
> > > otherwise maintain its last_busy timestamp, it may now be immediately
> > > eligible to suspend. This is probably not expected -- the parent should
> > > wait for its autosuspend delay before suspending.  
> > 
> > So reading this again - I wonder if this text is describing the
> > expected behaviour.  
> 
> Agreed, this might give the wrong impression, depending on the
> assumptions the reader makes about autosuspend delay. (I think I had a
> particular problem scenario in mind, but didn't translate that properly
> here.)
> 
> > If the child is also using autosupend then, in extreme case, that time
> > might be longer than the autosuspend time of the parent.
> > If so it seems odd the parent still waits.  
> 
> Right, that seems wrong, and indeed that's not what this patch does.
> 
> > So most expected to me
> > would be if the parent were to suspend it's autosuspend time after
> > the parent or child was last used for something that indicate a
> > desire not to suspend (that shouldn't include actions the child
> > did whilst suspending).  
> 
> (Not sure I'm parsing this sentence quite right, but...)
> 
> The point is that last_busy should propagate (rather than, say, the
> expiration time). So each device still gets its own-delay-based
> autosuspend timer expiration. That may mean the parent waits longer than
> the child, but it may not, if the child's delay was longer than the
> parent's.
> 
> > Maybe there isn't a sensible way to deal with this and what you have here
> > is the best we can do.  Note that this is just commenting on this
> > statement in the commit description.  From what follows I think
> > this is just a commit description thing rather than functional.  
> 
> If I'm understanding what you're saying correctly: maybe it would be an
> improvement here to say:
> 
>   If that parent configured an autosuspend delay longer than its child's
>   and didn't otherwise maintain its last_busy timestamp, it may now be
>   immediately eligible to suspend.
> 
> I think that would more accurately describe the target problem, even if
> it doesn't cover all cases (such as when the parent delay is shorter).

Agreed. That covers the main case nicely.

> 
> > > The effect of this behavior is that a parent device may suspend sooner
> > > than its autosuspend delay, simply because its usage was accounted by
> > > its children, and not by direct references to the parent device.  
> > 
> > Sooner is indeed bad.
> >   
> > > 
> > > This was noticed in several cases, and some have implemented
> > > workarounds, such as in commit c537d3457542 ("iio: adc: stm32-adc: fix
> > > runtime autosuspend delay when slow polling"). At the same time, Ulf
> > > suggested these problems "should be solved in the runtime PM core" [1].
> > > 
> > > Instead of working around the problem in drivers, we propagate last_busy
> > > timestamps from a dependent device to its dependencies any time it may
> > > allow a dependency to suspend -- i.e., when releasing a refcount for its
> > > parent or suppliers. We take care to only propagate the timestamp if it
> > > is larger than the existing busy timestamp.  
> > 
> > So this sounds fine but I don't think aligns with that first paragraph.
> >   
> > > 
> > > Note that this works best if the dependent device is using autosuspend
> > > (and therefore updates its last_busy timestamps appropriately), but even
> > > for a non-autosuspend child, this is still somewhat useful --
> > > non-autosuspend devices still automatically update their last_busy every
> > > time they resume.  
> > 
> > Can we set the last busy on the suspend of child devices that aren't
> > suspending as a result of autosuspend timeout but for other reasons?
> > (may be some fiddly corners!)  
> 
> Do you have concrete use cases in mind, or do you bring this up simply
> because of my note?

Just your note ;)
> 
> For one, I feel like for devices with power.use_autosuspend==true, it's
> reasonable to expect drivers to use the appropriate helpers to update
> last_busy at the appropriate times, and so any "corner cases" are from
> misuse.
> 
> But if you mean non-use_autosuspend devices: I suppose it'd be possible
> but I haven't reasoned through the implementation very far yet. It
> almost seems like we'd need to plumb pm_runtime_mark_last_busy() into
> *all* the put() helpers, including the non-autosuspend ones
> (pm_runtime_put(), pm_runtime_put_sync()). I'm not quite sure if that's
> desirable. But I could try to look a bit more, if you think there are
> important use cases that would need this.
If it is hard not a problem to leave it as is and call out the limitation
as you have done.  Someone who needs it can figure it out!


Jonathan

> 
> Brian
> 
> > > Link: https://lore.kernel.org/all/CAPDyKFp=KTf8=zGBSzPYqhjnZpY8xwvjCeM1e-WTKT1QLSxaDA@mail.gmail.com/ [1]
> > > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > > ---
> > > Cc: Ulf Hansson <ulfh@kernel.org>
> > > 
> > > Changes in v2:
> > >  * Update comments to explain when/why rpm_propagate_last_busy()
> > >    performs the update
> > > 
> > >  drivers/base/power/runtime.c | 23 ++++++++++++++++++++++-
> > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> > > index 2a117d6a776f..dd306cbabcd3 100644
> > > --- a/drivers/base/power/runtime.c
> > > +++ b/drivers/base/power/runtime.c
> > > @@ -318,6 +318,22 @@ static int rpm_get_suppliers(struct device *dev)
> > >  	return 0;
> > >  }
> > >  
> > > +/*
> > > + * Propagate last_busy timestamp from one device to another, typically when a
> > > + * device (@dev) releases a reference count on a dependency (@target) and might
> > > + * allow it to (auto)suspend. This can, for example, prevent overactive suspend
> > > + * when a dependency's usage is primarily driven by one of its dependents.
> > > + */
> > > +static void rpm_propagate_last_busy(struct device *dev, struct device *target)
> > > +{
> > > +	s64 busy = atomic64_read(&dev->power.last_busy);
> > > +	s64 target_busy = atomic64_read(&target->power.last_busy);
> > > +
> > > +	while (target_busy < busy)
> > > +		if (atomic64_try_cmpxchg(&target->power.last_busy, &target_busy, busy))
> > > +			return;
> > > +}
> > > +
> > >  /**
> > >   * pm_runtime_release_supplier - Drop references to device link's supplier.
> > >   * @link: Target device link.
> > > @@ -348,6 +364,7 @@ static void __rpm_put_suppliers(struct device *dev, bool try_to_suspend)
> > >  		if (!device_link_test(link, DL_FLAG_PM_RUNTIME))
> > >  			continue;
> > >  
> > > +		rpm_propagate_last_busy(dev, link->supplier);
> > >  		pm_runtime_release_supplier(link);
> > >  		if (try_to_suspend)
> > >  			pm_request_idle(link->supplier);
> > > @@ -710,6 +727,8 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> > >  
> > >  	if (dev->parent) {
> > >  		parent = dev->parent;
> > > +		if (!parent->power.ignore_children)
> > > +			rpm_propagate_last_busy(dev, parent);
> > >  		atomic_add_unless(&parent->power.child_count, -1, 0);
> > >  	}
> > >  	wake_up_all(&dev->power.wait_queue);
> > > @@ -1368,8 +1387,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
> > >  		goto out_set;
> > >  
> > >  	if (status == RPM_SUSPENDED) {
> > > -		atomic_add_unless(&parent->power.child_count, -1, 0);
> > >  		notify_parent = !parent->power.ignore_children;
> > > +		if (notify_parent)
> > > +			rpm_propagate_last_busy(dev, parent);
> > > +		atomic_add_unless(&parent->power.child_count, -1, 0);
> > >  	} else {
> > >  		spin_lock_nested(&parent->power.lock, SINGLE_DEPTH_NESTING);
> > >    
> >   
>