Skip to content

Commit e581cf5

Browse files
bebarinobebarino
committed
clk: Get runtime PM before walking tree during disable_unused
Doug reported [1] the following hung task: INFO: task swapper/0:1 blocked for more than 122 seconds. Not tainted 5.15.149-21875-gf795ebc40eb8 #1 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:swapper/0 state:D stack: 0 pid: 1 ppid: 0 flags:0x00000008 Call trace: __switch_to+0xf4/0x1f4 __schedule+0x418/0xb80 schedule+0x5c/0x10c rpm_resume+0xe0/0x52c rpm_resume+0x178/0x52c __pm_runtime_resume+0x58/0x98 clk_pm_runtime_get+0x30/0xb0 clk_disable_unused_subtree+0x58/0x208 clk_disable_unused_subtree+0x38/0x208 clk_disable_unused_subtree+0x38/0x208 clk_disable_unused_subtree+0x38/0x208 clk_disable_unused_subtree+0x38/0x208 clk_disable_unused+0x4c/0xe4 do_one_initcall+0xcc/0x2d8 do_initcall_level+0xa4/0x148 do_initcalls+0x5c/0x9c do_basic_setup+0x24/0x30 kernel_init_freeable+0xec/0x164 kernel_init+0x28/0x120 ret_from_fork+0x10/0x20 INFO: task kworker/u16:0:9 blocked for more than 122 seconds. Not tainted 5.15.149-21875-gf795ebc40eb8 #1 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:kworker/u16:0 state:D stack: 0 pid: 9 ppid: 2 flags:0x00000008 Workqueue: events_unbound deferred_probe_work_func Call trace: __switch_to+0xf4/0x1f4 __schedule+0x418/0xb80 schedule+0x5c/0x10c schedule_preempt_disabled+0x2c/0x48 __mutex_lock+0x238/0x488 __mutex_lock_slowpath+0x1c/0x28 mutex_lock+0x50/0x74 clk_prepare_lock+0x7c/0x9c clk_core_prepare_lock+0x20/0x44 clk_prepare+0x24/0x30 clk_bulk_prepare+0x40/0xb0 mdss_runtime_resume+0x54/0x1c8 pm_generic_runtime_resume+0x30/0x44 __genpd_runtime_resume+0x68/0x7c genpd_runtime_resume+0x108/0x1f4 __rpm_callback+0x84/0x144 rpm_callback+0x30/0x88 rpm_resume+0x1f4/0x52c rpm_resume+0x178/0x52c __pm_runtime_resume+0x58/0x98 __device_attach+0xe0/0x170 device_initial_probe+0x1c/0x28 bus_probe_device+0x3c/0x9c device_add+0x644/0x814 mipi_dsi_device_register_full+0xe4/0x170 devm_mipi_dsi_device_register_full+0x28/0x70 ti_sn_bridge_probe+0x1dc/0x2c0 auxiliary_bus_probe+0x4c/0x94 really_probe+0xcc/0x2c8 __driver_probe_device+0xa8/0x130 driver_probe_device+0x48/0x110 __device_attach_driver+0xa4/0xcc bus_for_each_drv+0x8c/0xd8 __device_attach+0xf8/0x170 device_initial_probe+0x1c/0x28 bus_probe_device+0x3c/0x9c deferred_probe_work_func+0x9c/0xd8 process_one_work+0x148/0x518 worker_thread+0x138/0x350 kthread+0x138/0x1e0 ret_from_fork+0x10/0x20 The first thread is walking the clk tree and calling clk_pm_runtime_get() to power on devices required to read the clk hardware via struct clk_ops::is_enabled(). This thread holds the clk prepare_lock, and is trying to runtime PM resume a device, when it finds that the device is in the process of resuming so the thread schedule()s away waiting for the device to finish resuming before continuing. The second thread is runtime PM resuming the same device, but the runtime resume callback is calling clk_prepare(), trying to grab the prepare_lock waiting on the first thread. This is a classic ABBA deadlock. To properly fix the deadlock, we must never runtime PM resume or suspend a device with the clk prepare_lock held. Actually doing that is near impossible today because the global prepare_lock would have to be dropped in the middle of the tree, the device runtime PM resumed/suspended, and then the prepare_lock grabbed again to ensure consistency of the clk tree topology. If anything changes with the clk tree in the meantime, we've lost and will need to start the operation all over again. Luckily, most of the time we're simply incrementing or decrementing the runtime PM count on an active device, so we don't have the chance to schedule away with the prepare_lock held. Let's fix this immediate problem that can be triggered more easily by simply booting on Qualcomm sc7180. Introduce a list of clk_core structures that have been registered, or are in the process of being registered, that require runtime PM to operate. Iterate this list and call clk_pm_runtime_get() on each of them without holding the prepare_lock during clk_disable_unused(). This way we can be certain that the runtime PM state of the devices will be active and resumed so we can't schedule away while walking the clk tree with the prepare_lock held. Similarly, call clk_pm_runtime_put() without the prepare_lock held to properly drop the runtime PM reference. We remove the calls to clk_pm_runtime_{get,put}() in this path because they're superfluous now that we know the devices are runtime resumed. Reported-by: Douglas Anderson <[email protected]> Closes: https://lore.kernel.org/all/20220922084322.RFC.2.I375b6b9e0a0a5348962f004beb3dafee6a12dfbb@changeid/ [1] Closes: https://issuetracker.google.com/328070191 Cc: Marek Szyprowski <[email protected]> Cc: Ulf Hansson <[email protected]> Cc: Krzysztof Kozlowski <[email protected]> Fixes: 9a34b45 ("clk: Add support for runtime PM") Signed-off-by: Stephen Boyd <[email protected]> Link: https://lore.kernel.org/r/[email protected] Reviewed-by: Douglas Anderson <[email protected]>
1 parent 9d05ae5 commit e581cf5

File tree

1 file changed

+105
-12
lines changed

1 file changed

+105
-12
lines changed

‎drivers/clk/clk.c

Lines changed: 105 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ static HLIST_HEAD(clk_root_list);
3737
static HLIST_HEAD(clk_orphan_list);
3838
static LIST_HEAD(clk_notifier_list);
3939

40+
/* List of registered clks that use runtime PM */
41+
static HLIST_HEAD(clk_rpm_list);
42+
static DEFINE_MUTEX(clk_rpm_list_lock);
43+
4044
static const struct hlist_head *all_lists[] = {
4145
&clk_root_list,
4246
&clk_orphan_list,
@@ -59,6 +63,7 @@ struct clk_core {
5963
struct clk_hw *hw;
6064
struct module *owner;
6165
struct device *dev;
66+
struct hlist_node rpm_node;
6267
struct device_node *of_node;
6368
struct clk_core *parent;
6469
struct clk_parent_map *parents;
@@ -122,6 +127,89 @@ static void clk_pm_runtime_put(struct clk_core *core)
122127
pm_runtime_put_sync(core->dev);
123128
}
124129

130+
/**
131+
* clk_pm_runtime_get_all() - Runtime "get" all clk provider devices
132+
*
133+
* Call clk_pm_runtime_get() on all runtime PM enabled clks in the clk tree so
134+
* that disabling unused clks avoids a deadlock where a device is runtime PM
135+
* resuming/suspending and the runtime PM callback is trying to grab the
136+
* prepare_lock for something like clk_prepare_enable() while
137+
* clk_disable_unused_subtree() holds the prepare_lock and is trying to runtime
138+
* PM resume/suspend the device as well.
139+
*
140+
* Context: Acquires the 'clk_rpm_list_lock' and returns with the lock held on
141+
* success. Otherwise the lock is released on failure.
142+
*
143+
* Return: 0 on success, negative errno otherwise.
144+
*/
145+
static int clk_pm_runtime_get_all(void)
146+
{
147+
int ret;
148+
struct clk_core *core, *failed;
149+
150+
/*
151+
* Grab the list lock to prevent any new clks from being registered
152+
* or unregistered until clk_pm_runtime_put_all().
153+
*/
154+
mutex_lock(&clk_rpm_list_lock);
155+
156+
/*
157+
* Runtime PM "get" all the devices that are needed for the clks
158+
* currently registered. Do this without holding the prepare_lock, to
159+
* avoid the deadlock.
160+
*/
161+
hlist_for_each_entry(core, &clk_rpm_list, rpm_node) {
162+
ret = clk_pm_runtime_get(core);
163+
if (ret) {
164+
failed = core;
165+
pr_err("clk: Failed to runtime PM get '%s' for clk '%s'\n",
166+
dev_name(failed->dev), failed->name);
167+
goto err;
168+
}
169+
}
170+
171+
return 0;
172+
173+
err:
174+
hlist_for_each_entry(core, &clk_rpm_list, rpm_node) {
175+
if (core == failed)
176+
break;
177+
178+
clk_pm_runtime_put(core);
179+
}
180+
mutex_unlock(&clk_rpm_list_lock);
181+
182+
return ret;
183+
}
184+
185+
/**
186+
* clk_pm_runtime_put_all() - Runtime "put" all clk provider devices
187+
*
188+
* Put the runtime PM references taken in clk_pm_runtime_get_all() and release
189+
* the 'clk_rpm_list_lock'.
190+
*/
191+
static void clk_pm_runtime_put_all(void)
192+
{
193+
struct clk_core *core;
194+
195+
hlist_for_each_entry(core, &clk_rpm_list, rpm_node)
196+
clk_pm_runtime_put(core);
197+
mutex_unlock(&clk_rpm_list_lock);
198+
}
199+
200+
static void clk_pm_runtime_init(struct clk_core *core)
201+
{
202+
struct device *dev = core->dev;
203+
204+
if (dev && pm_runtime_enabled(dev)) {
205+
core->rpm_enabled = true;
206+
207+
mutex_lock(&clk_rpm_list_lock);
208+
hlist_add_head(&core->rpm_node, &clk_rpm_list);
209+
mutex_unlock(&clk_rpm_list_lock);
210+
}
211+
}
212+
125213
/*** locking ***/
126214
static void clk_prepare_lock(void)
127215
{
@@ -1381,9 +1469,6 @@ static void __init clk_unprepare_unused_subtree(struct clk_core *core)
13811469
if (core->flags & CLK_IGNORE_UNUSED)
13821470
return;
13831471

1384-
if (clk_pm_runtime_get(core))
1385-
return;
1386-
13871472
if (clk_core_is_prepared(core)) {
13881473
trace_clk_unprepare(core);
13891474
if (core->ops->unprepare_unused)
@@ -1392,8 +1477,6 @@ static void __init clk_unprepare_unused_subtree(struct clk_core *core)
13921477
core->ops->unprepare(core->hw);
13931478
trace_clk_unprepare_complete(core);
13941479
}
1395-
1396-
clk_pm_runtime_put(core);
13971480
}
13981481

13991482
static void __init clk_disable_unused_subtree(struct clk_core *core)
@@ -1409,9 +1492,6 @@ static void __init clk_disable_unused_subtree(struct clk_core *core)
14091492
if (core->flags & CLK_OPS_PARENT_ENABLE)
14101493
clk_core_prepare_enable(core->parent);
14111494

1412-
if (clk_pm_runtime_get(core))
1413-
goto unprepare_out;
1414-
14151495
flags = clk_enable_lock();
14161496

14171497
if (core->enable_count)
@@ -1436,8 +1516,6 @@ static void __init clk_disable_unused_subtree(struct clk_core *core)
14361516

14371517
unlock_out:
14381518
clk_enable_unlock(flags);
1439-
clk_pm_runtime_put(core);
1440-
unprepare_out:
14411519
if (core->flags & CLK_OPS_PARENT_ENABLE)
14421520
clk_core_disable_unprepare(core->parent);
14431521
}
@@ -1453,6 +1531,7 @@ __setup("clk_ignore_unused", clk_ignore_unused_setup);
14531531
static int __init clk_disable_unused(void)
14541532
{
14551533
struct clk_core *core;
1534+
int ret;
14561535

14571536
if (clk_ignore_unused) {
14581537
pr_warn("clk: Not disabling unused clocks\n");
@@ -1461,6 +1540,13 @@ static int __init clk_disable_unused(void)
14611540

14621541
pr_info("clk: Disabling unused clocks\n");
14631542

1543+
ret = clk_pm_runtime_get_all();
1544+
if (ret)
1545+
return ret;
1546+
/*
1547+
* Grab the prepare lock to keep the clk topology stable while iterating
1548+
* over clks.
1549+
*/
14641550
clk_prepare_lock();
14651551

14661552
hlist_for_each_entry(core, &clk_root_list, child_node)
@@ -1477,6 +1563,8 @@ static int __init clk_disable_unused(void)
14771563

14781564
clk_prepare_unlock();
14791565

1566+
clk_pm_runtime_put_all();
1567+
14801568
return 0;
14811569
}
14821570
late_initcall_sync(clk_disable_unused);
@@ -4214,6 +4302,12 @@ static void __clk_release(struct kref *ref)
42144302
{
42154303
struct clk_core *core = container_of(ref, struct clk_core, ref);
42164304

4305+
if (core->rpm_enabled) {
4306+
mutex_lock(&clk_rpm_list_lock);
4307+
hlist_del(&core->rpm_node);
4308+
mutex_unlock(&clk_rpm_list_lock);
4309+
}
4310+
42174311
clk_core_free_parent_map(core);
42184312
kfree_const(core->name);
42194313
kfree(core);
@@ -4253,9 +4347,8 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
42534347
}
42544348
core->ops = init->ops;
42554349

4256-
if (dev && pm_runtime_enabled(dev))
4257-
core->rpm_enabled = true;
42584350
core->dev = dev;
4351+
clk_pm_runtime_init(core);
42594352
core->of_node = np;
42604353
if (dev && dev->driver)
42614354
core->owner = dev->driver->owner;

0 commit comments

Comments
 (0)