[PATCH] opp: Reduce the size of critical section in _opp_kref_release()

From: Viresh Kumar
Date: Thu Nov 19 2020 - 01:06:23 EST


There is a lot of stuff here which can be done outside of the
opp_table->lock, do that. This helps avoiding a circular dependency
lockdeps around debugfs.

Reported-by: Rob Clark <robdclark@xxxxxxxxx>
Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
---
drivers/opp/core.c | 94 +++++++++++++++++++++++-----------------------
1 file changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 9d145bb99a59..4268eb359915 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1251,9 +1251,14 @@ void _opp_free(struct dev_pm_opp *opp)
kfree(opp);
}

-static void _opp_kref_release(struct dev_pm_opp *opp,
- struct opp_table *opp_table)
+static void _opp_kref_release(struct kref *kref)
{
+ struct dev_pm_opp *opp = container_of(kref, struct dev_pm_opp, kref);
+ struct opp_table *opp_table = opp->opp_table;
+
+ list_del(&opp->node);
+ mutex_unlock(&opp_table->lock);
+
/*
* Notify the changes in the availability of the operable
* frequency/voltage list.
@@ -1261,27 +1266,9 @@ static void _opp_kref_release(struct dev_pm_opp *opp,
blocking_notifier_call_chain(&opp_table->head, OPP_EVENT_REMOVE, opp);
_of_opp_free_required_opps(opp_table, opp);
opp_debug_remove_one(opp);
- list_del(&opp->node);
kfree(opp);
}

-static void _opp_kref_release_unlocked(struct kref *kref)
-{
- struct dev_pm_opp *opp = container_of(kref, struct dev_pm_opp, kref);
- struct opp_table *opp_table = opp->opp_table;
-
- _opp_kref_release(opp, opp_table);
-}
-
-static void _opp_kref_release_locked(struct kref *kref)
-{
- struct dev_pm_opp *opp = container_of(kref, struct dev_pm_opp, kref);
- struct opp_table *opp_table = opp->opp_table;
-
- _opp_kref_release(opp, opp_table);
- mutex_unlock(&opp_table->lock);
-}
-
void dev_pm_opp_get(struct dev_pm_opp *opp)
{
kref_get(&opp->kref);
@@ -1289,16 +1276,10 @@ void dev_pm_opp_get(struct dev_pm_opp *opp)

void dev_pm_opp_put(struct dev_pm_opp *opp)
{
- kref_put_mutex(&opp->kref, _opp_kref_release_locked,
- &opp->opp_table->lock);
+ kref_put_mutex(&opp->kref, _opp_kref_release, &opp->opp_table->lock);
}
EXPORT_SYMBOL_GPL(dev_pm_opp_put);

-static void dev_pm_opp_put_unlocked(struct dev_pm_opp *opp)
-{
- kref_put(&opp->kref, _opp_kref_release_unlocked);
-}
-
/**
* dev_pm_opp_remove() - Remove an OPP from OPP table
* @dev: device for which we do this operation
@@ -1342,30 +1323,49 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_remove);

+static struct dev_pm_opp *_opp_get_next(struct opp_table *opp_table,
+ bool dynamic)
+{
+ struct dev_pm_opp *opp = NULL, *temp;
+
+ mutex_lock(&opp_table->lock);
+ list_for_each_entry(temp, &opp_table->opp_list, node) {
+ if (dynamic == temp->dynamic) {
+ opp = temp;
+ break;
+ }
+ }
+
+ mutex_unlock(&opp_table->lock);
+ return opp;
+}
+
bool _opp_remove_all_static(struct opp_table *opp_table)
{
- struct dev_pm_opp *opp, *tmp;
- bool ret = true;
+ struct dev_pm_opp *opp;

mutex_lock(&opp_table->lock);

if (!opp_table->parsed_static_opps) {
- ret = false;
- goto unlock;
+ mutex_unlock(&opp_table->lock);
+ return false;
}

- if (--opp_table->parsed_static_opps)
- goto unlock;
-
- list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) {
- if (!opp->dynamic)
- dev_pm_opp_put_unlocked(opp);
+ if (--opp_table->parsed_static_opps) {
+ mutex_unlock(&opp_table->lock);
+ return true;
}

-unlock:
mutex_unlock(&opp_table->lock);

- return ret;
+ /*
+ * 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);
+
+ return true;
}

/**
@@ -1377,21 +1377,21 @@ 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, *temp;
+ struct dev_pm_opp *opp;
int count = 0;

opp_table = _find_opp_table(dev);
if (IS_ERR(opp_table))
return;

- mutex_lock(&opp_table->lock);
- list_for_each_entry_safe(opp, temp, &opp_table->opp_list, node) {
- if (opp->dynamic) {
- dev_pm_opp_put_unlocked(opp);
- count++;
- }
+ /*
+ * 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++;
}
- mutex_unlock(&opp_table->lock);

/* Drop the references taken by dev_pm_opp_add() */
while (count--)
--
2.25.0.rc1.19.g042ed3e048af