Messages in this thread | | | Date | Sun, 18 Dec 2016 15:01:22 +0100 | From | Lukas Wunner <> | Subject | Re: [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
| |