All of lore.kernel.org
 help / color / mirror / Atom feed
From: Beata Michalska <beata.michalska@arm.com>
To: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org
Cc: vireshk@kernel.org, nm@ti.com, sboyd@kernel.org, beata.michalska@arm.com
Subject: [PATCH] opp: Invalidate current opp when draining the opp list
Date: Thu,  4 Mar 2021 15:07:34 +0000	[thread overview]
Message-ID: <1614870454-18709-1-git-send-email-beata.michalska@arm.com> (raw)

The current_opp when set, grabs additional reference on the opp,
which is then supposed to be dropped upon releasing the opp table.
Still both dev_pm_opp_remove_table and dev_pm_opp_remove_all_dynamic
will completely drain the OPPs list, including dropping the additional
reference on current_opp. This may lead to an attempt to access
memory that has already been released. Make sure that while draining
the list (in both dynamic and static cases) the current_opp gets
actually invalidated.

Fixes: 81c4d8a3c414 ("opp: Keep track of currently programmed OPP")

Signed-off-by: Beata Michalska <beata.michalska@arm.com>
---
 drivers/opp/core.c | 49 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index c268938..10e65c4 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1502,10 +1502,39 @@ static struct dev_pm_opp *_opp_get_next(struct opp_table *opp_table,
 	return opp;
 }
 
-bool _opp_remove_all_static(struct opp_table *opp_table)
+static int __opp_drain_list(struct opp_table *opp_table, bool dynamic)
 {
 	struct dev_pm_opp *opp;
+	int count = 0;
+
+	/*
+	 * Can't remove the OPP from under the lock, debugfs removal needs to
+	 * happen lock less to avoid circular dependency issues.
+	 */
+	while ((opp = _opp_get_next(opp_table, dynamic))) {
+		/*
+		 * The current_opp has extra hold on the ref count,
+		 * still the draining here will result in all of them
+		 * being dropped completely, so make
+		 * sure no one will try to access the current_opp
+		 * afterwords
+		 */
+		if (opp_table->current_opp == opp &&
+		    !(kref_read(&opp->kref) - 1))
+			opp_table->current_opp = NULL;
+
+		dev_pm_opp_put(opp);
+		/*
+		 * Note: the count here will reflect number of references
+		 * dropped, not the number of opps in the list
+		 */
+		++count;
+	}
+	return count;
+}
 
+bool _opp_remove_all_static(struct opp_table *opp_table)
+{
 	mutex_lock(&opp_table->lock);
 
 	if (!opp_table->parsed_static_opps) {
@@ -1520,13 +1549,7 @@ bool _opp_remove_all_static(struct opp_table *opp_table)
 
 	mutex_unlock(&opp_table->lock);
 
-	/*
-	 * Can't remove the OPP from under the lock, debugfs removal needs to
-	 * happen lock less to avoid circular dependency issues.
-	 */
-	while ((opp = _opp_get_next(opp_table, false)))
-		dev_pm_opp_put(opp);
-
+	__opp_drain_list(opp_table, false);
 	return true;
 }
 
@@ -1539,21 +1562,13 @@ bool _opp_remove_all_static(struct opp_table *opp_table)
 void dev_pm_opp_remove_all_dynamic(struct device *dev)
 {
 	struct opp_table *opp_table;
-	struct dev_pm_opp *opp;
 	int count = 0;
 
 	opp_table = _find_opp_table(dev);
 	if (IS_ERR(opp_table))
 		return;
 
-	/*
-	 * Can't remove the OPP from under the lock, debugfs removal needs to
-	 * happen lock less to avoid circular dependency issues.
-	 */
-	while ((opp = _opp_get_next(opp_table, true))) {
-		dev_pm_opp_put(opp);
-		count++;
-	}
+	count = __opp_drain_list(opp_table, true);
 
 	/* Drop the references taken by dev_pm_opp_add() */
 	while (count--)
-- 
2.7.4


             reply	other threads:[~2021-03-04 15:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-04 15:07 Beata Michalska [this message]
2021-03-04 17:27 ` [PATCH] opp: Invalidate current opp when draining the opp list Lukasz Luba
2021-03-05  4:24 ` Viresh Kumar
2021-03-05 13:55   ` Beata Michalska
2021-03-08 11:50     ` Viresh Kumar
2021-03-08 18:14       ` Beata Michalska
2021-03-09  4:31         ` Viresh Kumar
2021-03-09 12:14           ` Beata Michalska
2021-03-10  8:47             ` Viresh Kumar
2021-03-10 23:03               ` Beata Michalska
2021-03-12  3:49                 ` Viresh Kumar

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=1614870454-18709-1-git-send-email-beata.michalska@arm.com \
    --to=beata.michalska@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=sboyd@kernel.org \
    --cc=vireshk@kernel.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.