All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@kernel.org>
To: Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Russell King <linux@armlinux.org.uk>,
	Jeffrey Hugo <jhugo@codeaurora.org>, Chen-Yu Tsai <wens@csie.org>,
	Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Subject: [PATCH v3 1/7] clkdev: Hold clocks_mutex while iterating clocks list
Date: Thu,  4 Apr 2019 14:53:38 -0700	[thread overview]
Message-ID: <20190404215344.6330-2-sboyd@kernel.org> (raw)
In-Reply-To: <20190404215344.6330-1-sboyd@kernel.org>

We recently introduced a change to support devm clk lookups. That change
introduced a code-path that used clk_find() without holding the
'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
list and so we need to prevent the list from being modified while
iterating over it by holding the mutex. Similarly, we don't need to hold
the 'clocks_mutex' besides when we're dereferencing the clk_lookup
pointer we find or while we're modifying/iterating the 'clocks' list.

Let's ensure that we have the mutex held while iterating the list and
fix the callers of clk_find() to only hold the mutex as long as they
need to. This should fix this race and also nicely move clk creation
outside of the 'clocks_mutex'.

Fixes: 3eee6c7d119c ("clkdev: add managed clkdev lookup registration")
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clkdev.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 8c4435c53f09..db82eee8e209 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -36,7 +36,7 @@ static DEFINE_MUTEX(clocks_mutex);
  * Then we take the most specific entry - with the following
  * order of precedence: dev+con > dev only > con only.
  */
-static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
+static struct clk_lookup *__clk_find(const char *dev_id, const char *con_id)
 {
 	struct clk_lookup *p, *cl = NULL;
 	int match, best_found = 0, best_possible = 0;
@@ -46,6 +46,8 @@ static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
 	if (con_id)
 		best_possible += 1;
 
+	lockdep_assert_held(&clocks_mutex);
+
 	list_for_each_entry(p, &clocks, node) {
 		match = 0;
 		if (p->dev_id) {
@@ -70,25 +72,37 @@ static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
 	return cl;
 }
 
-static struct clk *__clk_get_sys(struct device *dev, const char *dev_id,
-				 const char *con_id)
+static struct clk_hw *clk_find_hw(const char *dev_id, const char *con_id)
 {
 	struct clk_lookup *cl;
-	struct clk *clk = NULL;
+	struct clk_hw *hw = ERR_PTR(-ENOENT);
 
 	mutex_lock(&clocks_mutex);
+	cl = __clk_find(dev_id, con_id);
+	if (cl)
+		hw = cl->clk_hw;
+	mutex_unlock(&clocks_mutex);
 
-	cl = clk_find(dev_id, con_id);
-	if (!cl)
-		goto out;
+	return hw;
+}
 
-	clk = clk_hw_create_clk(dev, cl->clk_hw, dev_id, con_id);
-	if (IS_ERR(clk))
-		cl = NULL;
-out:
+static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
+{
+	struct clk_lookup *cl;
+
+	mutex_lock(&clocks_mutex);
+	cl = __clk_find(dev_id, con_id);
 	mutex_unlock(&clocks_mutex);
 
-	return cl ? clk : ERR_PTR(-ENOENT);
+	return cl;
+}
+
+static struct clk *__clk_get_sys(struct device *dev, const char *dev_id,
+				 const char *con_id)
+{
+	struct clk_hw *hw = clk_find_hw(dev_id, con_id);
+
+	return clk_hw_create_clk(dev, hw, dev_id, con_id);
 }
 
 struct clk *clk_get_sys(const char *dev_id, const char *con_id)
-- 
Sent by a computer through tubes


  reply	other threads:[~2019-04-04 21:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-04 21:53 [PATCH v3 0/7] Rewrite clk parent handling Stephen Boyd
2019-04-04 21:53 ` Stephen Boyd [this message]
2019-04-05  6:51   ` [PATCH v3 1/7] clkdev: Hold clocks_mutex while iterating clocks list Vaittinen, Matti
2019-04-05 20:37     ` Stephen Boyd
2019-04-08 10:49       ` Matti Vaittinen
2019-04-08 17:00         ` Stephen Boyd
2019-04-08 22:21           ` Russell King - ARM Linux admin
2019-04-09  5:31             ` Vaittinen, Matti
2019-04-09  8:57               ` Russell King - ARM Linux admin
2019-04-10 16:45                 ` Stephen Boyd
2019-04-04 21:53 ` [PATCH v3 2/7] clk: Prepare for clk registration API that uses DT nodes Stephen Boyd
2019-04-04 21:53 ` [PATCH v3 3/7] clk: Add of_clk_hw_register() API for early clk drivers Stephen Boyd
2019-04-08 21:46   ` Jeffrey Hugo
2019-04-10 16:49     ` Stephen Boyd
2019-04-10 16:53     ` Stephen Boyd
2019-04-10 19:39       ` Jeffrey Hugo
2019-04-10 21:45         ` Stephen Boyd
2019-04-10 22:18           ` Jeffrey Hugo
2019-04-11 17:32             ` Jeffrey Hugo
2019-04-04 21:53 ` [PATCH v3 4/7] clk: Allow parents to be specified without string names Stephen Boyd
2019-04-04 21:53 ` [PATCH v3 5/7] clk: Look for parents with clkdev based clk_lookups Stephen Boyd
2019-04-04 21:53 ` [PATCH v3 6/7] clk: Allow parents to be specified via clkspec index Stephen Boyd
2019-04-04 21:53 ` [PATCH v3 7/7] clk: fixed-factor: Let clk framework find parent Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190404215344.6330-2-sboyd@kernel.org \
    --to=sboyd@kernel.org \
    --cc=jbrunet@baylibre.com \
    --cc=jhugo@codeaurora.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=matti.vaittinen@fi.rohmeurope.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=mturquette@baylibre.com \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.