Skip to content

Commit 55f3f18

Browse files
ludvigpaTreehugger Robot
authored andcommitted
UPSTREAM: regulator: core: Fix deadlock in create_regulator()
Currently, we are unnecessarily holding a regulator_ww_class_mutex lock when creating debugfs entries for a newly created regulator. This was brought up as a concern in the discussion in commit cba6cfdc7c3f ("regulator: core: Avoid lockdep reports when resolving supplies"). This causes the following lockdep splat after executing `ls /sys/kernel/debug` on my platform: ====================================================== WARNING: possible circular locking dependency detected 5.15.167-axis9-devel #1 Tainted: G O ------------------------------------------------------ ls/2146 is trying to acquire lock: ffffff803a562918 (&mm->mmap_lock){++++}-{3:3}, at: __might_fault+0x40/0x88 but task is already holding lock: ffffff80014497f8 (&sb->s_type->i_mutex_key#3){++++}-{3:3}, at: iterate_dir+0x50/0x1f4 which lock already depends on the new lock. [...] Chain exists of: &mm->mmap_lock --> regulator_ww_class_mutex --> &sb->s_type->i_mutex_key#3 Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&sb->s_type->i_mutex_key#3); lock(regulator_ww_class_mutex); lock(&sb->s_type->i_mutex_key#3); lock(&mm->mmap_lock); *** DEADLOCK *** This lock dependency still exists on the latest kernel and using a newer non-tainted kernel would still cause this problem. Fix by moving sysfs symlinking and creation of debugfs entries to after the release of the regulator lock. Bug: 254441685 Fixes: cba6cfdc7c3f ("regulator: core: Avoid lockdep reports when resolving supplies") Fixes: eaa7995c529b ("regulator: core: avoid regulator_resolve_supply() race condition") Signed-off-by: Ludvig Pärsson <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Mark Brown <[email protected]> (cherry picked from commit 1c81a8c78ae653f3a21cde0f37a91f1b22b7d2fb) Signed-off-by: Lee Jones <[email protected]> Change-Id: I1339abdfda460221e7e3fae3dae465b76f972394
1 parent be7fc88 commit 55f3f18

File tree

1 file changed

+43
-33
lines changed

1 file changed

+43
-33
lines changed

drivers/regulator/core.c

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,12 +1698,49 @@ static const struct file_operations constraint_flags_fops = {
16981698

16991699
#define REG_STR_SIZE 64
17001700

1701+
static void link_and_create_debugfs(struct regulator *regulator, struct regulator_dev *rdev,
1702+
struct device *dev)
1703+
{
1704+
int err = 0;
1705+
1706+
if (dev) {
1707+
regulator->dev = dev;
1708+
1709+
/* Add a link to the device sysfs entry */
1710+
err = sysfs_create_link_nowarn(&rdev->dev.kobj, &dev->kobj,
1711+
regulator->supply_name);
1712+
if (err) {
1713+
rdev_dbg(rdev, "could not add device link %s: %pe\n",
1714+
dev->kobj.name, ERR_PTR(err));
1715+
/* non-fatal */
1716+
}
1717+
}
1718+
1719+
if (err != -EEXIST) {
1720+
regulator->debugfs = debugfs_create_dir(regulator->supply_name, rdev->debugfs);
1721+
if (IS_ERR(regulator->debugfs)) {
1722+
rdev_dbg(rdev, "Failed to create debugfs directory\n");
1723+
regulator->debugfs = NULL;
1724+
}
1725+
}
1726+
1727+
if (regulator->debugfs) {
1728+
debugfs_create_u32("uA_load", 0444, regulator->debugfs,
1729+
&regulator->uA_load);
1730+
debugfs_create_u32("min_uV", 0444, regulator->debugfs,
1731+
&regulator->voltage[PM_SUSPEND_ON].min_uV);
1732+
debugfs_create_u32("max_uV", 0444, regulator->debugfs,
1733+
&regulator->voltage[PM_SUSPEND_ON].max_uV);
1734+
debugfs_create_file("constraint_flags", 0444, regulator->debugfs,
1735+
regulator, &constraint_flags_fops);
1736+
}
1737+
}
1738+
17011739
static struct regulator *create_regulator(struct regulator_dev *rdev,
17021740
struct device *dev,
17031741
const char *supply_name)
17041742
{
17051743
struct regulator *regulator;
1706-
int err = 0;
17071744

17081745
lockdep_assert_held_once(&rdev->mutex.base);
17091746

@@ -1736,38 +1773,6 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
17361773

17371774
list_add(&regulator->list, &rdev->consumer_list);
17381775

1739-
if (dev) {
1740-
regulator->dev = dev;
1741-
1742-
/* Add a link to the device sysfs entry */
1743-
err = sysfs_create_link_nowarn(&rdev->dev.kobj, &dev->kobj,
1744-
supply_name);
1745-
if (err) {
1746-
rdev_dbg(rdev, "could not add device link %s: %pe\n",
1747-
dev->kobj.name, ERR_PTR(err));
1748-
/* non-fatal */
1749-
}
1750-
}
1751-
1752-
if (err != -EEXIST) {
1753-
regulator->debugfs = debugfs_create_dir(supply_name, rdev->debugfs);
1754-
if (IS_ERR(regulator->debugfs)) {
1755-
rdev_dbg(rdev, "Failed to create debugfs directory\n");
1756-
regulator->debugfs = NULL;
1757-
}
1758-
}
1759-
1760-
if (regulator->debugfs) {
1761-
debugfs_create_u32("uA_load", 0444, regulator->debugfs,
1762-
&regulator->uA_load);
1763-
debugfs_create_u32("min_uV", 0444, regulator->debugfs,
1764-
&regulator->voltage[PM_SUSPEND_ON].min_uV);
1765-
debugfs_create_u32("max_uV", 0444, regulator->debugfs,
1766-
&regulator->voltage[PM_SUSPEND_ON].max_uV);
1767-
debugfs_create_file("constraint_flags", 0444, regulator->debugfs,
1768-
regulator, &constraint_flags_fops);
1769-
}
1770-
17711776
/*
17721777
* Check now if the regulator is an always on regulator - if
17731778
* it is then we don't need to do nearly so much work for
@@ -1996,6 +2001,9 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
19962001

19972002
regulator_unlock_two(rdev, r, &ww_ctx);
19982003

2004+
/* rdev->supply was created in set_supply() */
2005+
link_and_create_debugfs(rdev->supply, r, &rdev->dev);
2006+
19992007
/*
20002008
* In set_machine_constraints() we may have turned this regulator on
20012009
* but we couldn't propagate to the supply if it hadn't been resolved
@@ -2119,6 +2127,8 @@ struct regulator *_regulator_get(struct device *dev, const char *id,
21192127
return regulator;
21202128
}
21212129

2130+
link_and_create_debugfs(regulator, rdev, dev);
2131+
21222132
rdev->open_count++;
21232133
if (get_type == EXCLUSIVE_GET) {
21242134
rdev->exclusive = 1;

0 commit comments

Comments
 (0)