2024-04-23 12:00:05

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 5.4 6/8] clk: Don't hold prepare_lock when calling kref_put()

From: Stephen Boyd <[email protected]>

[ Upstream commit 6f63af7511e7058f3fa4ad5b8102210741c9f947 ]

We don't need to hold the prepare_lock when dropping a ref on a struct
clk_core. The release function is only freeing memory and any code with
a pointer reference has already unlinked anything pointing to the
clk_core. This reduces the holding area of the prepare_lock a bit.

Note that we also don't call free_clk() with the prepare_lock held.
There isn't any reason to do that.

Reviewed-by: Douglas Anderson <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/clk/clk.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 67051ca60920a..9dddf5f555ab5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3971,7 +3971,8 @@ void clk_unregister(struct clk *clk)
if (clk->core->ops == &clk_nodrv_ops) {
pr_err("%s: unregistered clock: %s\n", __func__,
clk->core->name);
- goto unlock;
+ clk_prepare_unlock();
+ return;
}
/*
* Assign empty clock ops for consumers that might still hold
@@ -4002,11 +4003,10 @@ void clk_unregister(struct clk *clk)
if (clk->core->protect_count)
pr_warn("%s: unregistering protected clock: %s\n",
__func__, clk->core->name);
+ clk_prepare_unlock();

kref_put(&clk->core->ref, __clk_release);
free_clk(clk);
-unlock:
- clk_prepare_unlock();
}
EXPORT_SYMBOL_GPL(clk_unregister);

@@ -4168,13 +4168,11 @@ void __clk_put(struct clk *clk)
clk->max_rate < clk->core->req_rate)
clk_core_set_rate_nolock(clk->core, clk->core->req_rate);

- owner = clk->core->owner;
- kref_put(&clk->core->ref, __clk_release);
-
clk_prepare_unlock();

+ owner = clk->core->owner;
+ kref_put(&clk->core->ref, __clk_release);
module_put(owner);
-
free_clk(clk);
}

--
2.43.0



2024-04-23 19:26:19

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH AUTOSEL 5.4 6/8] clk: Don't hold prepare_lock when calling kref_put()

Quoting Sasha Levin (2024-04-23 04:03:01)
> From: Stephen Boyd <[email protected]>
>
> [ Upstream commit 6f63af7511e7058f3fa4ad5b8102210741c9f947 ]
>
> We don't need to hold the prepare_lock when dropping a ref on a struct
> clk_core. The release function is only freeing memory and any code with
> a pointer reference has already unlinked anything pointing to the
> clk_core. This reduces the holding area of the prepare_lock a bit.
>
> Note that we also don't call free_clk() with the prepare_lock held.
> There isn't any reason to do that.

You'll want the patch before this, 8358a76cfb47 ("clk: Remove
prepare_lock hold assertion in __clk_release()"), to avoid lockdep
warnings. And it looks like the problem was reported on v5.15.y so all
5 patches from the series would need a backport.

8358a76cfb47 clk: Remove prepare_lock hold assertion in __clk_release()
6f63af7511e7 clk: Don't hold prepare_lock when calling kref_put()
9d05ae531c2c clk: Initialize struct clk_core kref earlier
e581cf5d2162 clk: Get runtime PM before walking tree during disable_unused
9d1e795f754d clk: Get runtime PM before walking tree for clk_summary