Hello Mark,
On 1/21/21 12:30 PM, Marek Szyprowski wrote:
Hi Mark,
On 21.01.2021 16:44, Mark Brown wrote:
On Thu, Jan 21, 2021 at 10:41:59AM +0100, Marek Szyprowski wrote:
On 18.01.2021 21:49, Mark Brown wrote:
Does this help (completely untested):
Sadly nope. I get same warning:
Try this instead:
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 3ae5ccd9277d..31503776dbd7 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1823,17 +1823,6 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
if (rdev->supply)
return 0;
- /*
- * Recheck rdev->supply with rdev->mutex lock held to avoid a race
- * between rdev->supply null check and setting rdev->supply in
- * set_supply() from concurrent tasks.
- */
- regulator_lock(rdev);
-
- /* Supply just resolved by a concurrent task? */
- if (rdev->supply)
- goto out;
-
r = regulator_dev_lookup(dev, rdev->supply_name);
if (IS_ERR(r)) {
ret = PTR_ERR(r);
@@ -1885,12 +1874,29 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
goto out;
}
+ /*
+ * Recheck rdev->supply with rdev->mutex lock held to avoid a race
+ * between rdev->supply null check and setting rdev->supply in
+ * set_supply() from concurrent tasks.
+ */
+ regulator_lock(rdev);
+
+ /* Supply just resolved by a concurrent task? */
+ if (rdev->supply) {
+ regulator_unlock(rdev);
+ put_device(&r->dev);
+ return ret;
+ }
+
ret = set_supply(rdev, r);
if (ret < 0) {
+ regulator_unlock(rdev);
put_device(&r->dev);
- goto out;
+ return ret;
}
+ regulator_unlock(rdev);
+
/*
* In set_machine_constraints() we may have turned this regulator on
* but we couldn't propagate to the supply if it hadn't been resolved
@@ -1901,12 +1907,11 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
if (ret < 0) {
_regulator_put(rdev->supply);
rdev->supply = NULL;
- goto out;
+ goto out_rdev_lock;
drivers/regulator/core.c:1910:4: error: label ‘out_rdev_lock’ used but
not defined
}
}
out:
- regulator_unlock(rdev);
return ret;
}
It looks that it finally fixes the locking issue, with the above goto
removed completely to fix build. Feel free to add:
Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
Thank you for making this fix. I'm sorry that I missed the potential
deadlock issue resulting from the regulator_enable() call inside
regulator_resolve_supply() with rdev->mutex locked. Your fix avoids
deadlock while still ensuring that the there isn't a set supply race
condition.
Take care,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
