2021-12-17 19:16:07

by Rafael J. Wysocki

Subject: [PATCH] PM: runtime: Simplify locking in pm_runtime_put_suppliers()

From: Rafael J. Wysocki <[email protected]>

Notice that pm_runtime_put_suppliers() cannot be called with
disabled interrupts, because it may sleep (due to the device
links read locking in the non-SRCU case), and so it can use
spin_lock_irq() and spin_unlock_irq() for the locking.

Update the function accordingly and while at it move the "put"
local variable in it into the inner block where it is used.

This change is not expected to have any visible functional impact.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/runtime.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/base/power/runtime.c
===================================================================
--- linux-pm.orig/drivers/base/power/runtime.c
+++ linux-pm/drivers/base/power/runtime.c
@@ -1748,8 +1748,6 @@ 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();
@@ -1757,11 +1755,17 @@ void pm_runtime_put_suppliers(struct dev
list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
device_links_read_lock_held())
if (link->supplier_preactivated) {
+ bool put;
+
link->supplier_preactivated = false;
- spin_lock_irqsave(&dev->power.lock, flags);
+
+ spin_lock_irq(&dev->power.lock);
+
put = pm_runtime_status_suspended(dev) &&
refcount_dec_not_one(&link->rpm_active);
- spin_unlock_irqrestore(&dev->power.lock, flags);
+
+ spin_unlock_irq(&dev->power.lock);
+
if (put)
pm_runtime_put(link->supplier);
}

2021-12-21 11:47:26

by Ulf Hansson

Subject: Re: [PATCH] PM: runtime: Simplify locking in pm_runtime_put_suppliers()

On Fri, 17 Dec 2021 at 20:16, Rafael J. Wysocki <[email protected]> wrote:
>
> From: Rafael J. Wysocki <[email protected]>
>
> Notice that pm_runtime_put_suppliers() cannot be called with
> disabled interrupts, because it may sleep (due to the device
> links read locking in the non-SRCU case), and so it can use
> spin_lock_irq() and spin_unlock_irq() for the locking.
>
> Update the function accordingly and while at it move the "put"
> local variable in it into the inner block where it is used.
>
> This change is not expected to have any visible functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe

> ---
> drivers/base/power/runtime.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> Index: linux-pm/drivers/base/power/runtime.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/runtime.c
> +++ linux-pm/drivers/base/power/runtime.c
> @@ -1748,8 +1748,6 @@ 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();
> @@ -1757,11 +1755,17 @@ void pm_runtime_put_suppliers(struct dev
> list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
> device_links_read_lock_held())
> if (link->supplier_preactivated) {
> + bool put;
> +
> link->supplier_preactivated = false;
> - spin_lock_irqsave(&dev->power.lock, flags);
> +
> + spin_lock_irq(&dev->power.lock);
> +
> put = pm_runtime_status_suspended(dev) &&
> refcount_dec_not_one(&link->rpm_active);
> - spin_unlock_irqrestore(&dev->power.lock, flags);
> +
> + spin_unlock_irq(&dev->power.lock);
> +
> if (put)
> pm_runtime_put(link->supplier);
> }
>
>
>