| Message ID | 1836398.tdWV9SEqCh@kreacher |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] PM: runtime: Avoid device usage count underflows | expand |
On Wed, 6 Apr 2022 at 21:03, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > A PM-runtime device usage count underflow is potentially critical, > because it may cause a device to be suspended when it is expected to > be operational. I get the point. Although, perhaps we should also state that it's a programming problem that we would like to catch and warn about? > > For this reason, (1) make rpm_check_suspend_allowed() return an error > when the device usage count is negative to prevent devices from being > suspended in that case, (2) introduce rpm_drop_usage_count() that will > detect device usage count underflows, warn about them and fix them up, > and (3) use it to drop the usage count in a few places instead of > atomic_dec_and_test(). > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/base/power/runtime.c | 44 ++++++++++++++++++++++++++++++++++++------- > 1 file changed, 37 insertions(+), 7 deletions(-) > > Index: linux-pm/drivers/base/power/runtime.c > =================================================================== > --- linux-pm.orig/drivers/base/power/runtime.c > +++ linux-pm/drivers/base/power/runtime.c > @@ -263,7 +263,7 @@ static int rpm_check_suspend_allowed(str > retval = -EINVAL; > else if (dev->power.disable_depth > 0) > retval = -EACCES; > - else if (atomic_read(&dev->power.usage_count) > 0) > + else if (atomic_read(&dev->power.usage_count)) > retval = -EAGAIN; > else if (!dev->power.ignore_children && > atomic_read(&dev->power.child_count)) > @@ -1039,13 +1039,33 @@ int pm_schedule_suspend(struct device *d > } > EXPORT_SYMBOL_GPL(pm_schedule_suspend); > > +static int rpm_drop_usage_count(struct device *dev) > +{ > + int ret; > + > + ret = atomic_sub_return(1, &dev->power.usage_count); > + if (ret >= 0) > + return ret; > + > + /* > + * Because rpm_resume() does not check the usage counter, it will resume > + * the device even if the usage counter is 0 or negative, so it is > + * sufficient to increment the usage counter here to reverse the change > + * made above. > + */ > + atomic_inc(&dev->power.usage_count); Rather than this two-step process, couldn't we just do an "atomic_add_unless(&dev->power.usage_count, -1, 0)" - and check the return value? > + dev_warn(dev, "Runtime PM usage count underflow!\n"); > + return -EINVAL; > +} > + [...] Kind regards Uffe
On Fri, Apr 8, 2022 at 4:05 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Wed, 6 Apr 2022 at 21:03, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > A PM-runtime device usage count underflow is potentially critical, > > because it may cause a device to be suspended when it is expected to > > be operational. > > I get the point. Although, perhaps we should also state that it's a > programming problem that we would like to catch and warn about? OK, I can add that to the changelog. > > > > For this reason, (1) make rpm_check_suspend_allowed() return an error > > when the device usage count is negative to prevent devices from being > > suspended in that case, (2) introduce rpm_drop_usage_count() that will > > detect device usage count underflows, warn about them and fix them up, > > and (3) use it to drop the usage count in a few places instead of > > atomic_dec_and_test(). > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > drivers/base/power/runtime.c | 44 ++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 37 insertions(+), 7 deletions(-) > > > > Index: linux-pm/drivers/base/power/runtime.c > > =================================================================== > > --- linux-pm.orig/drivers/base/power/runtime.c > > +++ linux-pm/drivers/base/power/runtime.c > > @@ -263,7 +263,7 @@ static int rpm_check_suspend_allowed(str > > retval = -EINVAL; > > else if (dev->power.disable_depth > 0) > > retval = -EACCES; > > - else if (atomic_read(&dev->power.usage_count) > 0) > > + else if (atomic_read(&dev->power.usage_count)) > > retval = -EAGAIN; > > else if (!dev->power.ignore_children && > > atomic_read(&dev->power.child_count)) > > @@ -1039,13 +1039,33 @@ int pm_schedule_suspend(struct device *d > > } > > EXPORT_SYMBOL_GPL(pm_schedule_suspend); > > > > +static int rpm_drop_usage_count(struct device *dev) > > +{ > > + int ret; > > + > > + ret = atomic_sub_return(1, &dev->power.usage_count); > > + if (ret >= 0) > > + return ret; > > + > > + /* > > + * Because rpm_resume() does not check the usage counter, it will resume > > + * the device even if the usage counter is 0 or negative, so it is > > + * sufficient to increment the usage counter here to reverse the change > > + * made above. > > + */ > > + atomic_inc(&dev->power.usage_count); > > Rather than this two-step process, couldn't we just do an > "atomic_add_unless(&dev->power.usage_count, -1, 0)" - and check the > return value? No, we couldn't, because atomic_add_unless() returns a bool and we need to know the new counter value (and in particular whether or not it is 0). I thought that it would be better to do the extra access in the failing case only. > > + dev_warn(dev, "Runtime PM usage count underflow!\n"); > > + return -EINVAL; > > +} > > + > > [...]
On Fri, 8 Apr 2022 at 19:05, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Fri, Apr 8, 2022 at 4:05 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > On Wed, 6 Apr 2022 at 21:03, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > A PM-runtime device usage count underflow is potentially critical, > > > because it may cause a device to be suspended when it is expected to > > > be operational. > > > > I get the point. Although, perhaps we should also state that it's a > > programming problem that we would like to catch and warn about? > > OK, I can add that to the changelog. > > > > > > > For this reason, (1) make rpm_check_suspend_allowed() return an error > > > when the device usage count is negative to prevent devices from being > > > suspended in that case, (2) introduce rpm_drop_usage_count() that will > > > detect device usage count underflows, warn about them and fix them up, > > > and (3) use it to drop the usage count in a few places instead of > > > atomic_dec_and_test(). > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > --- > > > drivers/base/power/runtime.c | 44 ++++++++++++++++++++++++++++++++++++------- > > > 1 file changed, 37 insertions(+), 7 deletions(-) > > > > > > Index: linux-pm/drivers/base/power/runtime.c > > > =================================================================== > > > --- linux-pm.orig/drivers/base/power/runtime.c > > > +++ linux-pm/drivers/base/power/runtime.c > > > @@ -263,7 +263,7 @@ static int rpm_check_suspend_allowed(str > > > retval = -EINVAL; > > > else if (dev->power.disable_depth > 0) > > > retval = -EACCES; > > > - else if (atomic_read(&dev->power.usage_count) > 0) > > > + else if (atomic_read(&dev->power.usage_count)) > > > retval = -EAGAIN; > > > else if (!dev->power.ignore_children && > > > atomic_read(&dev->power.child_count)) > > > @@ -1039,13 +1039,33 @@ int pm_schedule_suspend(struct device *d > > > } > > > EXPORT_SYMBOL_GPL(pm_schedule_suspend); > > > > > > +static int rpm_drop_usage_count(struct device *dev) > > > +{ > > > + int ret; > > > + > > > + ret = atomic_sub_return(1, &dev->power.usage_count); > > > + if (ret >= 0) > > > + return ret; > > > + > > > + /* > > > + * Because rpm_resume() does not check the usage counter, it will resume > > > + * the device even if the usage counter is 0 or negative, so it is > > > + * sufficient to increment the usage counter here to reverse the change > > > + * made above. > > > + */ > > > + atomic_inc(&dev->power.usage_count); > > > > Rather than this two-step process, couldn't we just do an > > "atomic_add_unless(&dev->power.usage_count, -1, 0)" - and check the > > return value? > > No, we couldn't, because atomic_add_unless() returns a bool and we > need to know the new counter value (and in particular whether or not > it is 0). atomic_add_unless(&dev->power.usage_count, -1, 0) would return true as long as the counter value is greater than 0. If the counter has become 0, atomic_add_unless() would return false and not continue to decrease the value below zero. Isn't this exactly what we want? > > I thought that it would be better to do the extra access in the > failing case only. > > > > + dev_warn(dev, "Runtime PM usage count underflow!\n"); > > > + return -EINVAL; > > > +} > > > + > > > > [...] Kind regards Uffe
On Mon, Apr 11, 2022 at 12:36 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Fri, 8 Apr 2022 at 19:05, Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Fri, Apr 8, 2022 at 4:05 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > > > On Wed, 6 Apr 2022 at 21:03, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > > > > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > A PM-runtime device usage count underflow is potentially critical, > > > > because it may cause a device to be suspended when it is expected to > > > > be operational. > > > > > > I get the point. Although, perhaps we should also state that it's a > > > programming problem that we would like to catch and warn about? > > > > OK, I can add that to the changelog. > > > > > > > > > > For this reason, (1) make rpm_check_suspend_allowed() return an error > > > > when the device usage count is negative to prevent devices from being > > > > suspended in that case, (2) introduce rpm_drop_usage_count() that will > > > > detect device usage count underflows, warn about them and fix them up, > > > > and (3) use it to drop the usage count in a few places instead of > > > > atomic_dec_and_test(). > > > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > --- > > > > drivers/base/power/runtime.c | 44 ++++++++++++++++++++++++++++++++++++------- > > > > 1 file changed, 37 insertions(+), 7 deletions(-) > > > > > > > > Index: linux-pm/drivers/base/power/runtime.c > > > > =================================================================== > > > > --- linux-pm.orig/drivers/base/power/runtime.c > > > > +++ linux-pm/drivers/base/power/runtime.c > > > > @@ -263,7 +263,7 @@ static int rpm_check_suspend_allowed(str > > > > retval = -EINVAL; > > > > else if (dev->power.disable_depth > 0) > > > > retval = -EACCES; > > > > - else if (atomic_read(&dev->power.usage_count) > 0) > > > > + else if (atomic_read(&dev->power.usage_count)) > > > > retval = -EAGAIN; > > > > else if (!dev->power.ignore_children && > > > > atomic_read(&dev->power.child_count)) > > > > @@ -1039,13 +1039,33 @@ int pm_schedule_suspend(struct device *d > > > > } > > > > EXPORT_SYMBOL_GPL(pm_schedule_suspend); > > > > > > > > +static int rpm_drop_usage_count(struct device *dev) > > > > +{ > > > > + int ret; > > > > + > > > > + ret = atomic_sub_return(1, &dev->power.usage_count); > > > > + if (ret >= 0) > > > > + return ret; > > > > + > > > > + /* > > > > + * Because rpm_resume() does not check the usage counter, it will resume > > > > + * the device even if the usage counter is 0 or negative, so it is > > > > + * sufficient to increment the usage counter here to reverse the change > > > > + * made above. > > > > + */ > > > > + atomic_inc(&dev->power.usage_count); > > > > > > Rather than this two-step process, couldn't we just do an > > > "atomic_add_unless(&dev->power.usage_count, -1, 0)" - and check the > > > return value? > > > > No, we couldn't, because atomic_add_unless() returns a bool and we > > need to know the new counter value (and in particular whether or not > > it is 0). > > atomic_add_unless(&dev->power.usage_count, -1, 0) would return true as > long as the counter value is greater than 0. Yes, and it in particular, when the current value of the counter is 1 before the operation IIUC. So after the operation it is 0 and true will be returned, won't it? But that's exactly the case we want to catch. > If the counter has become 0, atomic_add_unless() would return false > and not continue to decrease the value below zero. Isn't this exactly > what we want? Not really. We want to detect transitions from 0 to 1 which is the case when the device can be suspended.
On Mon, 11 Apr 2022 at 13:29, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Mon, Apr 11, 2022 at 12:36 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > On Fri, 8 Apr 2022 at 19:05, Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > > > On Fri, Apr 8, 2022 at 4:05 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > > > > > On Wed, 6 Apr 2022 at 21:03, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > > > > > > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > > > A PM-runtime device usage count underflow is potentially critical, > > > > > because it may cause a device to be suspended when it is expected to > > > > > be operational. > > > > > > > > I get the point. Although, perhaps we should also state that it's a > > > > programming problem that we would like to catch and warn about? > > > > > > OK, I can add that to the changelog. > > > > > > > > > > > > > For this reason, (1) make rpm_check_suspend_allowed() return an error > > > > > when the device usage count is negative to prevent devices from being > > > > > suspended in that case, (2) introduce rpm_drop_usage_count() that will > > > > > detect device usage count underflows, warn about them and fix them up, > > > > > and (3) use it to drop the usage count in a few places instead of > > > > > atomic_dec_and_test(). > > > > > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > --- > > > > > drivers/base/power/runtime.c | 44 ++++++++++++++++++++++++++++++++++++------- > > > > > 1 file changed, 37 insertions(+), 7 deletions(-) > > > > > > > > > > Index: linux-pm/drivers/base/power/runtime.c > > > > > =================================================================== > > > > > --- linux-pm.orig/drivers/base/power/runtime.c > > > > > +++ linux-pm/drivers/base/power/runtime.c > > > > > @@ -263,7 +263,7 @@ static int rpm_check_suspend_allowed(str > > > > > retval = -EINVAL; > > > > > else if (dev->power.disable_depth > 0) > > > > > retval = -EACCES; > > > > > - else if (atomic_read(&dev->power.usage_count) > 0) > > > > > + else if (atomic_read(&dev->power.usage_count)) > > > > > retval = -EAGAIN; > > > > > else if (!dev->power.ignore_children && > > > > > atomic_read(&dev->power.child_count)) > > > > > @@ -1039,13 +1039,33 @@ int pm_schedule_suspend(struct device *d > > > > > } > > > > > EXPORT_SYMBOL_GPL(pm_schedule_suspend); > > > > > > > > > > +static int rpm_drop_usage_count(struct device *dev) > > > > > +{ > > > > > + int ret; > > > > > + > > > > > + ret = atomic_sub_return(1, &dev->power.usage_count); > > > > > + if (ret >= 0) > > > > > + return ret; > > > > > + > > > > > + /* > > > > > + * Because rpm_resume() does not check the usage counter, it will resume > > > > > + * the device even if the usage counter is 0 or negative, so it is > > > > > + * sufficient to increment the usage counter here to reverse the change > > > > > + * made above. > > > > > + */ > > > > > + atomic_inc(&dev->power.usage_count); > > > > > > > > Rather than this two-step process, couldn't we just do an > > > > "atomic_add_unless(&dev->power.usage_count, -1, 0)" - and check the > > > > return value? > > > > > > No, we couldn't, because atomic_add_unless() returns a bool and we > > > need to know the new counter value (and in particular whether or not > > > it is 0). > > > > atomic_add_unless(&dev->power.usage_count, -1, 0) would return true as > > long as the counter value is greater than 0. > > Yes, and it in particular, when the current value of the counter is 1 > before the operation IIUC. > > So after the operation it is 0 and true will be returned, won't it? > But that's exactly the case we want to catch. > > > If the counter has become 0, atomic_add_unless() would return false > > and not continue to decrease the value below zero. Isn't this exactly > > what we want? > > Not really. > > We want to detect transitions from 0 to 1 which is the case when the > device can be suspended. I assume you mean from 1 to 0. In any case, I see what you mean by now, sorry for the noise. Then feel free to add: Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe
On Wed, Apr 6, 2022 at 11:49 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > A PM-runtime device usage count underflow is potentially critical, > because it may cause a device to be suspended when it is expected to > be operational. > > For this reason, (1) make rpm_check_suspend_allowed() return an error > when the device usage count is negative to prevent devices from being > suspended in that case, (2) introduce rpm_drop_usage_count() that will > detect device usage count underflows, warn about them and fix them up, > and (3) use it to drop the usage count in a few places instead of > atomic_dec_and_test(). ... > + retval = rpm_drop_usage_count(dev); > + if (retval > 0) { > trace_rpm_usage_rcuidle(dev, rpmflags); > return 0; > + } else if (retval < 0) { > + return retval; > } Can be written in a form if (retval < 0) return retval; if (retval > 0) { trace_rpm_usage_rcuidle(dev, rpmflags); return 0; } ... > + if (retval > 0) { > trace_rpm_usage_rcuidle(dev, rpmflags); > return 0; > + } else if (retval < 0) { > + return retval; > } Ditto.
On Mon, Apr 11, 2022 at 5:09 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Wed, Apr 6, 2022 at 11:49 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > A PM-runtime device usage count underflow is potentially critical, > > because it may cause a device to be suspended when it is expected to > > be operational. > > > > For this reason, (1) make rpm_check_suspend_allowed() return an error > > when the device usage count is negative to prevent devices from being > > suspended in that case, (2) introduce rpm_drop_usage_count() that will > > detect device usage count underflows, warn about them and fix them up, > > and (3) use it to drop the usage count in a few places instead of > > atomic_dec_and_test(). > > ... > > > + retval = rpm_drop_usage_count(dev); > > + if (retval > 0) { > > trace_rpm_usage_rcuidle(dev, rpmflags); > > return 0; > > + } else if (retval < 0) { > > + return retval; > > } > > Can be written in a form > > if (retval < 0) > return retval; > if (retval > 0) { > trace_rpm_usage_rcuidle(dev, rpmflags); > return 0; > } > I know. And why would it be better?
On Mon, Apr 11, 2022 at 6:17 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > On Mon, Apr 11, 2022 at 5:09 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Wed, Apr 6, 2022 at 11:49 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: ... > > > + retval = rpm_drop_usage_count(dev); > > > + if (retval > 0) { > > > trace_rpm_usage_rcuidle(dev, rpmflags); > > > return 0; > > > + } else if (retval < 0) { > > > + return retval; > > > } > > > > Can be written in a form > > > > if (retval < 0) > > return retval; > > if (retval > 0) { > > trace_rpm_usage_rcuidle(dev, rpmflags); > > return 0; > > } > > > > I know. > > And why would it be better? Depends on the perception: a) less characters to parse (no 'else'); b) checking for errors first, which seems more or less standard pattern.
On Mon, Apr 11, 2022 at 6:53 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Apr 11, 2022 at 6:17 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Mon, Apr 11, 2022 at 5:09 PM Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > > > On Wed, Apr 6, 2022 at 11:49 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > ... > > > > > + retval = rpm_drop_usage_count(dev); > > > > + if (retval > 0) { > > > > trace_rpm_usage_rcuidle(dev, rpmflags); > > > > return 0; > > > > + } else if (retval < 0) { > > > > + return retval; > > > > } > > > > > > Can be written in a form > > > > > > if (retval < 0) > > > return retval; > > > if (retval > 0) { > > > trace_rpm_usage_rcuidle(dev, rpmflags); > > > return 0; > > > } > > > > > > > I know. > > > > And why would it be better? > > Depends on the perception: Well, exactly. > a) less characters to parse (no 'else'); But to me, with the "else" it is clear that the conditionals are related to each other which is not so clear otherwise at first sight. YMMV > b) checking for errors first, which seems more or less standard pattern. So the checks can be reversed no problem, but this is such a minor point ,,,
Index: linux-pm/drivers/base/power/runtime.c =================================================================== --- linux-pm.orig/drivers/base/power/runtime.c +++ linux-pm/drivers/base/power/runtime.c @@ -263,7 +263,7 @@ static int rpm_check_suspend_allowed(str retval = -EINVAL; else if (dev->power.disable_depth > 0) retval = -EACCES; - else if (atomic_read(&dev->power.usage_count) > 0) + else if (atomic_read(&dev->power.usage_count)) retval = -EAGAIN; else if (!dev->power.ignore_children && atomic_read(&dev->power.child_count)) @@ -1039,13 +1039,33 @@ int pm_schedule_suspend(struct device *d } EXPORT_SYMBOL_GPL(pm_schedule_suspend); +static int rpm_drop_usage_count(struct device *dev) +{ + int ret; + + ret = atomic_sub_return(1, &dev->power.usage_count); + if (ret >= 0) + return ret; + + /* + * Because rpm_resume() does not check the usage counter, it will resume + * the device even if the usage counter is 0 or negative, so it is + * sufficient to increment the usage counter here to reverse the change + * made above. + */ + atomic_inc(&dev->power.usage_count); + dev_warn(dev, "Runtime PM usage count underflow!\n"); + return -EINVAL; +} + /** * __pm_runtime_idle - Entry point for runtime idle operations. * @dev: Device to send idle notification for. * @rpmflags: Flag bits. * * If the RPM_GET_PUT flag is set, decrement the device's usage count and - * return immediately if it is larger than zero. Then carry out an idle + * return immediately if it is larger than zero (if it becomes negative, log a + * warning, increment it, and return an error). Then carry out an idle * notification, either synchronous or asynchronous. * * This routine may be called in atomic context if the RPM_ASYNC flag is set, @@ -1057,9 +1077,12 @@ int __pm_runtime_idle(struct device *dev int retval; if (rpmflags & RPM_GET_PUT) { - if (!atomic_dec_and_test(&dev->power.usage_count)) { + retval = rpm_drop_usage_count(dev); + if (retval > 0) { trace_rpm_usage_rcuidle(dev, rpmflags); return 0; + } else if (retval < 0) { + return retval; } } @@ -1079,7 +1102,8 @@ EXPORT_SYMBOL_GPL(__pm_runtime_idle); * @rpmflags: Flag bits. * * If the RPM_GET_PUT flag is set, decrement the device's usage count and - * return immediately if it is larger than zero. Then carry out a suspend, + * return immediately if it is larger than zero (if it becomes negative, log a + * warning, increment it, and return an error). Then carry out a suspend, * either synchronous or asynchronous. * * This routine may be called in atomic context if the RPM_ASYNC flag is set, @@ -1091,9 +1115,12 @@ int __pm_runtime_suspend(struct device * int retval; if (rpmflags & RPM_GET_PUT) { - if (!atomic_dec_and_test(&dev->power.usage_count)) { + retval = rpm_drop_usage_count(dev); + if (retval > 0) { trace_rpm_usage_rcuidle(dev, rpmflags); return 0; + } else if (retval < 0) { + return retval; } } @@ -1527,14 +1554,17 @@ EXPORT_SYMBOL_GPL(pm_runtime_forbid); */ void pm_runtime_allow(struct device *dev) { + int ret; + spin_lock_irq(&dev->power.lock); if (dev->power.runtime_auto) goto out; dev->power.runtime_auto = true; - if (atomic_dec_and_test(&dev->power.usage_count)) + ret = rpm_drop_usage_count(dev); + if (ret == 0) rpm_idle(dev, RPM_AUTO | RPM_ASYNC); - else + else if (ret > 0) trace_rpm_usage_rcuidle(dev, RPM_AUTO | RPM_ASYNC); out: