lkml.org 
[lkml]   [2016]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v6 4/5] PM / runtime: Use device links
Hi Rafael,

spotted what looks like a bug in the device links runtime PM code:

When resuming a device, __rpm_callback() calls rpm_get_suppliers(dev):

> + retval = rpm_get_suppliers(dev);
> + if (retval)
> + goto fail;


This will walk the list of suppliers and call pm_runtime_get_sync()
for each of them:

> +static int rpm_get_suppliers(struct device *dev)
> +{
> + struct device_link *link;
> +
> + list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) {
> + int retval;
[...]
> + retval = pm_runtime_get_sync(link->supplier);
> + if (retval < 0) {
> + pm_runtime_put_noidle(link->supplier);
> + return retval;


If pm_runtime_get_sync() failed, e.g. because runtime PM is disabled
on a supplier, the function will put the reference of the failed
supplier and return.

Back in __rpm_callback() we jump to the fail mark, where we call
rpm_put_suppliers().

> + fail:
> + rpm_put_suppliers(dev);
> +
> + device_links_read_unlock(idx);


This walks the list of suppliers and releases a ref for each of them:

> +static void rpm_put_suppliers(struct device *dev)
> +{
> + struct device_link *link;
> +
> + list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
> + if (link->rpm_active &&
> + READ_ONCE(link->status) != DL_STATE_SUPPLIER_UNBIND) {
> + pm_runtime_put(link->supplier);
> + link->rpm_active = false;
> + }
> +}


This looks wrong: We've already put a ref on the failed supplier, so here
we're putting another one. And if there are further suppliers in the list
following the failed one, we'll decrement their refcount even though we've
never incremented it.

Thanks,

Lukas

\
 
 \ /
  Last update: 2016-12-18 15:02    [W:0.144 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site