linux-kernel - Re: [PATCH v1] PM: runtime: Take active children into account in pm_runtime_get_if_in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aHZVQaY6E9R_YWKs@kekkonen.localdomain>
Date: Tue, 15 Jul 2025 13:18:57 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>,
	Linux PM <linux-pm@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>, Alex Elder <elder@...aro.org>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH v1] PM: runtime: Take active children into account in
 pm_runtime_get_if_in_use()

Hi Rafael,

On Tue, Jul 15, 2025 at 02:46:21PM +0200, Rafael J. Wysocki wrote:
> Hi Sakari,
> 
> On Tue, Jul 15, 2025 at 9:28 AM Sakari Ailus
> <sakari.ailus@...ux.intel.com> wrote:
> >
> > Hi Rafael,
> >
> > Thanks for the patch.
> >
> > On Wed, Jul 09, 2025 at 12:41:45PM +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > >
> > > For all practical purposes, there is no difference between the situation
> > > in which a given device is not ignoring children and its active child
> > > count is nonzero and the situation in which its runtime PM usage counter
> > > is nonzero.  However, pm_runtime_get_if_in_use() will only increment the
> > > device's usage counter and return 1 in the latter case.
> > >
> > > For consistency, make it do so in the former case either by adjusting
> > > pm_runtime_get_conditional() and update the related kerneldoc comments
> > > accordingly.
> > >
> > > Fixes: c0ef3df8dbae ("PM: runtime: Simplify pm_runtime_get_if_active() usage")
> >
> > I guess this should be:
> >
> > Fixes: c111566bea7c ("PM: runtime: Add pm_runtime_get_if_active()")
> 
> Technically yes, but that would require specific backport changes for
> older "stable" series.

The lines in pm_runtime_get_conditional() were added by c111566bea7cc, it
looks like the actual fix would be backportable without changes whereas the
documentation would require them. So splitting the patch should allow
backporting the fix only while leaving the documentation as-is. I don't
really have an opinion either way. It's perhaps unlikely the issue causes
actual problems but you never know...

> 
> > Should this also be cc'd to stable?
> 
> Possibly.
> 
> > Reviewed-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> 
> Thank you!
> 
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > > ---
> > >  drivers/base/power/runtime.c |   27 ++++++++++++++++++---------
> > >  1 file changed, 18 insertions(+), 9 deletions(-)
> > >
> > > --- a/drivers/base/power/runtime.c
> > > +++ b/drivers/base/power/runtime.c
> > > @@ -1203,10 +1203,12 @@
> > >   *
> > >   * Return -EINVAL if runtime PM is disabled for @dev.
> > >   *
> > > - * Otherwise, if the runtime PM status of @dev is %RPM_ACTIVE and either
> > > - * @ign_usage_count is %true or the runtime PM usage counter of @dev is not
> > > - * zero, increment the usage counter of @dev and return 1. Otherwise, return 0
> > > - * without changing the usage counter.
> > > + * Otherwise, if its runtime PM status is %RPM_ACTIVE and (1) @ign_usage_count
> > > + * is set, or (2) @dev is not ignoring children and its active child count is
> > > + * nonero, or (3) the runtime PM usage counter of @dev is not zero, increment
> > > + * the usage counter of @dev and return 1.
> > > + *
> > > + * Otherwise, return 0 without changing the usage counter.
> > >   *
> > >   * If @ign_usage_count is %true, this function can be used to prevent suspending
> > >   * the device when its runtime PM status is %RPM_ACTIVE.
> > > @@ -1228,7 +1230,8 @@
> > >               retval = -EINVAL;
> > >       } else if (dev->power.runtime_status != RPM_ACTIVE) {
> > >               retval = 0;
> > > -     } else if (ign_usage_count) {
> > > +     } else if (ign_usage_count || (!dev->power.ignore_children &&
> > > +                atomic_read(&dev->power.child_count) > 0)) {
> > >               retval = 1;
> > >               atomic_inc(&dev->power.usage_count);
> > >       } else {
> > > @@ -1261,10 +1264,16 @@
> > >   * @dev: Target device.
> > >   *
> > >   * Increment the runtime PM usage counter of @dev if its runtime PM status is
> > > - * %RPM_ACTIVE and its runtime PM usage counter is greater than 0, in which case
> > > - * it returns 1. If the device is in a different state or its usage_count is 0,
> > > - * 0 is returned. -EINVAL is returned if runtime PM is disabled for the device,
> > > - * in which case also the usage_count will remain unmodified.
> > > + * %RPM_ACTIVE and its runtime PM usage counter is greater than 0 or it is not
> > > + * ignoring children and its active child count is nonzero.  1 is returned in
> > > + * this case.
> > > + *
> > > + * If @dev is in a different state or it is not in use (that is, its usage
> > > + * counter is 0, or it is ignoring children, or its active child count is 0),
> > > + * 0 is returned.
> > > + *
> > > + * -EINVAL is returned if runtime PM is disabled for the device, in which case
> > > + * also the usage counter of @dev is not updated.
> > >   */
> > >  int pm_runtime_get_if_in_use(struct device *dev)
> > >  {
> > >
> > >
> >

-- 
Kind regards,

Sakari Ailus