lkml.org 
[lkml]   [2021]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 32/56] PM: runtime: Fix race getting/putting suppliers at probe
    Date
    From: Adrian Hunter <adrian.hunter@intel.com>

    commit 9dfacc54a8661bc8be6e08cffee59596ec59f263 upstream.

    pm_runtime_put_suppliers() must not decrement rpm_active unless the
    consumer is suspended. That is because, otherwise, it could suspend
    suppliers for an active consumer.

    That can happen as follows:

    static int driver_probe_device(struct device_driver *drv, struct device *dev)
    {
    int ret = 0;

    if (!device_is_registered(dev))
    return -ENODEV;

    dev->can_match = true;
    pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
    drv->bus->name, __func__, dev_name(dev), drv->name);

    pm_runtime_get_suppliers(dev);
    if (dev->parent)
    pm_runtime_get_sync(dev->parent);

    At this point, dev can runtime suspend so rpm_put_suppliers() can run,
    rpm_active becomes 1 (the lowest value).

    pm_runtime_barrier(dev);
    if (initcall_debug)
    ret = really_probe_debug(dev, drv);
    else
    ret = really_probe(dev, drv);

    Probe callback can have runtime resumed dev, and then runtime put
    so dev is awaiting autosuspend, but rpm_active is 2.

    pm_request_idle(dev);

    if (dev->parent)
    pm_runtime_put(dev->parent);

    pm_runtime_put_suppliers(dev);

    Now pm_runtime_put_suppliers() will put the supplier
    i.e. rpm_active 2 -> 1, but consumer can still be active.

    return ret;
    }

    Fix by checking the runtime status. For any status other than
    RPM_SUSPENDED, rpm_active can be considered to be "owned" by
    rpm_[get/put]_suppliers() and pm_runtime_put_suppliers() need do nothing.

    Reported-by: Asutosh Das <asutoshd@codeaurora.org>
    Fixes: 4c06c4e6cf63 ("driver core: Fix possible supplier PM-usage counter imbalance")
    Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
    Cc: 5.1+ <stable@vger.kernel.org> # 5.1+
    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/base/power/runtime.c | 8 +++++++-
    1 file changed, 7 insertions(+), 1 deletion(-)

    --- a/drivers/base/power/runtime.c
    +++ b/drivers/base/power/runtime.c
    @@ -1586,6 +1586,8 @@ void pm_runtime_get_suppliers(struct dev
    void pm_runtime_put_suppliers(struct device *dev)
    {
    struct device_link *link;
    + unsigned long flags;
    + bool put;
    int idx;

    idx = device_links_read_lock();
    @@ -1593,7 +1595,11 @@ void pm_runtime_put_suppliers(struct dev
    list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
    if (link->supplier_preactivated) {
    link->supplier_preactivated = false;
    - if (refcount_dec_not_one(&link->rpm_active))
    + spin_lock_irqsave(&dev->power.lock, flags);
    + put = pm_runtime_status_suspended(dev) &&
    + refcount_dec_not_one(&link->rpm_active);
    + spin_unlock_irqrestore(&dev->power.lock, flags);
    + if (put)
    pm_runtime_put(link->supplier);
    }


    \
     
     \ /
      Last update: 2021-04-05 11:10    [W:2.140 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site