regulator: core: Make regulator_lock_two() logic easier to follow
Commit Message
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@chromium.org>
Link: https://lore.kernel.org/r/CAE-0n53Eb1BeDPmjBycXUaQAF4ppiAM6UDWje_jiB9GAmR8MMw@mail.gmail.com
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/regulator/core.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
Comments
On Thu, 13 Apr 2023 17:34:17 -0700, Douglas Anderson wrote:
> 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.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/1] regulator: core: Make regulator_lock_two() logic easier to follow
commit: 37473397b852f556d8b9428ccbfd51886037842d
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
@@ -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, ®ulator_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;
}