lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20231205-genpd-perf-order-v1-1-6597cc69a729@gerhold.net>
Date:   Tue, 05 Dec 2023 10:34:05 +0100
From:   Stephan Gerhold <stephan@...hold.net>
To:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Kevin Hilman <khilman@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>
Cc:     Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Stephan Gerhold <stephan@...hold.net>
Subject: [PATCH] PM: domains: Scale down parent performance states in
 reverse order

Power domains might have parent domains assigned that are automatically
managed by the PM domain core. In particular, parent domains are
automatically powered on/off and setting performance states on child
domains is translated to parent domains (e.g. using an OPP table from
the device tree).

Currently parent performance states are always adjusted before the
performance state of the child domain is changed.

However, typically a parent/child relationship between two power
domains with performance states models the requirement to keep the
parent domain at a performance state equal or higher to the child
domain. When scaling down there is a brief moment where the parent
domain will end up having a lower performance state than required by
the child domain.

To avoid this, we need to differentiate between scaling up/down and
adjust the order of operations:

 - When scaling up, parent domains should be adjusted before the child
   domain. In case of an error, the rollback happens in reverse order.

 - When scaling down, parent domains should be adjusted after the child
   domain, in reverse order, just as if we would rollback scaling up.
   In case of an error, the rollback happens in normal order (just as
   if we would normally scale up).

Implement this by moving the existing functionality of
_genpd_set_performance_state() to two separate functions that are
called in the proper iteration order.

Signed-off-by: Stephan Gerhold <stephan@...hold.net>
---
Related discussion: https://lore.kernel.org/linux-pm/ZWXgFNKgm9QaFuzx@gerhold.net/
---
 drivers/base/power/domain.c | 124 +++++++++++++++++++++++++++-----------------
 1 file changed, 77 insertions(+), 47 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index da1777e39eaa..830dfef2c880 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -313,72 +313,102 @@ static int genpd_xlate_performance_state(struct generic_pm_domain *genpd,
 }
 
 static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
-					unsigned int state, int depth)
+					unsigned int state, int depth);
+
+static void _genpd_rollback_parent_state(struct gpd_link *link, int depth)
 {
-	struct generic_pm_domain *parent;
-	struct gpd_link *link;
-	int parent_state, ret;
+	struct generic_pm_domain *parent = link->parent;
+	int parent_state;
 
-	if (state == genpd->performance_state)
-		return 0;
+	genpd_lock_nested(parent, depth + 1);
 
-	/* Propagate to parents of genpd */
-	list_for_each_entry(link, &genpd->child_links, child_node) {
-		parent = link->parent;
+	parent_state = link->prev_performance_state;
+	link->performance_state = parent_state;
 
-		/* Find parent's performance state */
-		ret = genpd_xlate_performance_state(genpd, parent, state);
-		if (unlikely(ret < 0))
-			goto err;
+	parent_state = _genpd_reeval_performance_state(parent, parent_state);
+	if (_genpd_set_performance_state(parent, parent_state, depth + 1)) {
+		pr_err("%s: Failed to roll back to %d performance state\n",
+		       parent->name, parent_state);
+	}
 
-		parent_state = ret;
+	genpd_unlock(parent);
+}
 
-		genpd_lock_nested(parent, depth + 1);
+static int _genpd_set_parent_state(struct generic_pm_domain *genpd,
+				   struct gpd_link *link,
+				   unsigned int state, int depth)
+{
+	struct generic_pm_domain *parent = link->parent;
+	int parent_state, ret;
 
-		link->prev_performance_state = link->performance_state;
-		link->performance_state = parent_state;
-		parent_state = _genpd_reeval_performance_state(parent,
-						parent_state);
-		ret = _genpd_set_performance_state(parent, parent_state, depth + 1);
-		if (ret)
-			link->performance_state = link->prev_performance_state;
+	/* Find parent's performance state */
+	ret = genpd_xlate_performance_state(genpd, parent, state);
+	if (unlikely(ret < 0))
+		return ret;
 
-		genpd_unlock(parent);
+	parent_state = ret;
 
-		if (ret)
-			goto err;
-	}
+	genpd_lock_nested(parent, depth + 1);
 
-	if (genpd->set_performance_state) {
-		ret = genpd->set_performance_state(genpd, state);
-		if (ret)
-			goto err;
-	}
+	link->prev_performance_state = link->performance_state;
+	link->performance_state = parent_state;
 
-	genpd->performance_state = state;
-	return 0;
+	parent_state = _genpd_reeval_performance_state(parent, parent_state);
+	ret = _genpd_set_performance_state(parent, parent_state, depth + 1);
+	if (ret)
+		link->performance_state = link->prev_performance_state;
 
-err:
-	/* Encountered an error, lets rollback */
-	list_for_each_entry_continue_reverse(link, &genpd->child_links,
-					     child_node) {
-		parent = link->parent;
+	genpd_unlock(parent);
 
-		genpd_lock_nested(parent, depth + 1);
+	return ret;
+}
+
+static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
+					unsigned int state, int depth)
+{
+	struct gpd_link *link = NULL;
+	int ret;
+
+	if (state == genpd->performance_state)
+		return 0;
 
-		parent_state = link->prev_performance_state;
-		link->performance_state = parent_state;
+	/* When scaling up, propagate to parents first in normal order */
+	if (state > genpd->performance_state) {
+		list_for_each_entry(link, &genpd->child_links, child_node) {
+			ret = _genpd_set_parent_state(genpd, link, state, depth);
+			if (ret)
+				goto rollback_parents_up;
+		}
+	}
 
-		parent_state = _genpd_reeval_performance_state(parent,
-						parent_state);
-		if (_genpd_set_performance_state(parent, parent_state, depth + 1)) {
-			pr_err("%s: Failed to roll back to %d performance state\n",
-			       parent->name, parent_state);
+	if (genpd->set_performance_state) {
+		ret = genpd->set_performance_state(genpd, state);
+		if (ret) {
+			if (link)
+				goto rollback_parents_up;
+			return ret;
 		}
+	}
 
-		genpd_unlock(parent);
+	/* When scaling down, propagate to parents last in reverse order */
+	if (state < genpd->performance_state) {
+		list_for_each_entry_reverse(link, &genpd->child_links, child_node) {
+			ret = _genpd_set_parent_state(genpd, link, state, depth);
+			if (ret)
+				goto rollback_parents_down;
+		}
 	}
 
+	genpd->performance_state = state;
+	return 0;
+
+rollback_parents_up:
+	list_for_each_entry_continue_reverse(link, &genpd->child_links, child_node)
+		_genpd_rollback_parent_state(link, depth);
+	return ret;
+rollback_parents_down:
+	list_for_each_entry_continue(link, &genpd->child_links, child_node)
+		_genpd_rollback_parent_state(link, depth);
 	return ret;
 }
 

---
base-commit: 0f5f12ac05f36f117e793656c3f560625e927f1b
change-id: 20231205-genpd-perf-order-bf33029c25ac

Best regards,
-- 
Stephan Gerhold <stephan@...hold.net>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ