linux-kernel - [PATCH] regulator: core: Make regulator_lock

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>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230413173359.1.I1ae92b25689bd6579952e6d458b79f5f8054a0c9@changeid>
Date:   Thu, 13 Apr 2023 17:34:17 -0700
From:   Douglas Anderson <dianders@...omium.org>
To:     Mark Brown <broonie@...nel.org>
Cc:     Stephen Boyd <swboyd@...omium.org>,
        Douglas Anderson <dianders@...omium.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        linux-kernel@...r.kernel.org
Subject: [PATCH] regulator: core: Make regulator_lock_two() logic easier to follow

The regulator_lock_two() function could be made clearer in the case of
lock contention by having a local variable for each of the held and
contended locks. Let's do that. At the same time, let's use the swap()
function instead of open coding it.

This change is expected to be a no-op and simply improves code
clarity.

Suggested-by: Stephen Boyd <swboyd@...omium.org>
Link: https://lore.kernel.org/r/CAE-0n53Eb1BeDPmjBycXUaQAF4ppiAM6UDWje_jiB9GAmR8MMw@mail.gmail.com
Signed-off-by: Douglas Anderson <dianders@...omium.org>
---

 drivers/regulator/core.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 08726bc0da9d..dc741ac156c3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -219,7 +219,7 @@ static void regulator_lock_two(struct regulator_dev *rdev1,
 			       struct regulator_dev *rdev2,
 			       struct ww_acquire_ctx *ww_ctx)
 {
-	struct regulator_dev *tmp;
+	struct regulator_dev *held, *contended;
 	int ret;
 
 	ww_acquire_init(ww_ctx, &regulator_ww_class);
@@ -233,25 +233,18 @@ static void regulator_lock_two(struct regulator_dev *rdev1,
 		goto exit;
 	}
 
+	held = rdev1;
+	contended = rdev2;
 	while (true) {
-		/*
-		 * Start of loop: rdev1 was locked and rdev2 was contended.
-		 * Need to unlock rdev1, slowly lock rdev2, then try rdev1
-		 * again.
-		 */
-		regulator_unlock(rdev1);
-
-		ww_mutex_lock_slow(&rdev2->mutex, ww_ctx);
-		rdev2->ref_cnt++;
-		rdev2->mutex_owner = current;
-		ret = regulator_lock_nested(rdev1, ww_ctx);
-
-		if (ret == -EDEADLOCK) {
-			/* More contention; swap which needs to be slow */
-			tmp = rdev1;
-			rdev1 = rdev2;
-			rdev2 = tmp;
-		} else {
+		regulator_unlock(held);
+
+		ww_mutex_lock_slow(&contended->mutex, ww_ctx);
+		contended->ref_cnt++;
+		contended->mutex_owner = current;
+		swap(held, contended);
+		ret = regulator_lock_nested(contended, ww_ctx);
+
+		if (ret != -EDEADLOCK) {
 			WARN_ON(ret);
 			break;
 		}
-- 
2.40.0.634.g4ca3ef3211-goog