Skip to contentSkip to conversation

clk: Really fix deadlock with mmap_sem

PATCH In review Stephen Boyd 4 participants 6 messages Dec 12, 2014 – Dec 16, 2014 Common clk

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 clk: Really fix deadlock with mmap_sem

Commit 6314b6796e3c (clk: Don't hold prepare_lock across debugfs creation, 2014-09-04) forgot to update one place where we hold the prepare_lock while creating debugfs directories. This means we still have the chance of a deadlock that the commit was trying to fix. Actually fix it by moving the debugfs creation outside the prepare_lock.

Fixes 6314b6796e3c "clk: Don't hold prepare_lock across debugfs creation"
---
 drivers/clk/clk.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/clk/clk.c +5 -2
index 44cdc47a6cc5..c9430653ddc9 100644
@@ -240,12 +240,13 @@ static const struct file_operations clk_dump_fops = {
 	.release	= single_release,
 };
 
-/* caller must hold prepare_lock */
 static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry)
 {
 	struct dentry *d;
 	int ret = -ENOMEM;
 
+	lockdep_assert_held(clk_debug_lock);
+
 	if (!clk || !pdentry) {
 		ret = -EINVAL;
 		goto out;
@@ -1944,7 +1945,6 @@ int __clk_init(struct device *dev, struct clk *clk)
 	else
 		clk->rate = 0;
 
-	clk_debug_register(clk);
 	/*
 	 * walk the list of orphan clocks and reparent any that are children of
 	 * this clock
@@ -1979,6 +1979,9 @@ int __clk_init(struct device *dev, struct clk *clk)
 out:
 	clk_prepare_unlock();
 
+	if (!ret)
+		clk_debug_register(clk);
+
 	return ret;
 }
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw
Quoted 8 lines · Stephen Boyd

Commit 6314b6796e3c (clk: Don't hold prepare_lock across debugfs creation, 2014-09-04) forgot to update one place where we hold the prepare_lock while creating debugfs directories. This means we still have the chance of a deadlock that the commit was trying to fix. Actually fix it by moving the debugfs creation outside the prepare_lock.

Please use "Russell King <[email protected]>" rather than this address, thanks.

Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
#2 lore raw

Quoting Russell King - ARM Linux (2014-12-12 15:05:43)

Quoted 8 lines · Stephen Boyd
Quoted 7 lines

Commit 6314b6796e3c (clk: Don't hold prepare_lock across debugfs creation, 2014-09-04) forgot to update one place where we hold the prepare_lock while creating debugfs directories. This means we still have the chance of a deadlock that the commit was trying to fix. Actually fix it by moving the debugfs creation outside the prepare_lock.

Quoted 4 lines
Quoted 1 line

Please use "Russell King <[email protected]>" rather than this address, thanks.

Applied to clk-next and fixed up the email address locally.

Regards,
Mike

Quoted 4 lines
Signature · 3 lines
-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
#3 lore raw
Quoted 9 lines · Mike Turquette

Quoting Russell King - ARM Linux (2014-12-12 15:05:43)

Quoted 8 lines · Stephen Boyd
Quoted 7 lines

Commit 6314b6796e3c (clk: Don't hold prepare_lock across debugfs creation, 2014-09-04) forgot to update one place where we hold the prepare_lock while creating debugfs directories. This means we still have the chance of a deadlock that the commit was trying to fix. Actually fix it by moving the debugfs creation outside the prepare_lock.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

Please use "Russell King <[email protected]>" rather than this address, thanks.

Applied to clk-next and fixed up the email address locally.

Please make sure it gets into stable kernels too as v3.18 suffers from this bug, thanks.

Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
#4 lore raw

PATCH 1/1 clk: Really fix deadlock with mmap_sem

Quoting Russell King - ARM Linux (2014-12-12 15:26:26)

Quoted 12 lines · Mike Turquette
Quoted 11 lines

Quoting Russell King - ARM Linux (2014-12-12 15:05:43)

Quoted 10 lines · Stephen Boyd
Quoted 8 lines

Commit 6314b6796e3c (clk: Don't hold prepare_lock across debugfs creation, 2014-09-04) forgot to update one place where we hold the prepare_lock while creating debugfs directories. This means we still have the chance of a deadlock that the commit was trying to fix. Actually fix it by moving the debugfs creation outside the prepare_lock.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Please use "Russell King <[email protected]>" rather than this address, thanks.

Applied to clk-next and fixed up the email address locally.

Please make sure it gets into stable kernels too as v3.18 suffers from this bug, thanks.

Final version of this in my tree removes lockdep_assert (not mentioned in commitlog and had a bug in it), adds Cc: stable and Thomas's reviewed-by.

Regards,
Mike

Quoted 4 lines
Signature · 3 lines
-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
Reviewed-by on v1 1/1
#5 lore raw
In reply to Stephen Boyd

drivers/clk/clk.c static const struct file_operations clk_dump_fops = { · line 248

Quoted 12 lines · Stephen Boyd

Commit 6314b6796e3c (clk: Don't hold prepare_lock across debugfs creation, 2014-09-04) forgot to update one place where we hold the prepare_lock while creating debugfs directories. This means we still have the chance of a deadlock that the commit was trying to fix. Actually fix it by moving the debugfs creation outside the prepare_lock.

Fixes 6314b6796e3c "clk: Don't hold prepare_lock across debugfs creation"
drivers/clk/clk.c +1 -0
+	lockdep_assert_held(clk_debug_lock);

That change is not mentioned in the changelog and seems to be unrelated to the issue at hand.

Other than that: