Skip to content

Commit 1c81a8c

Browse files
ludvigpabroonie
authored andcommitted
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 cba6cfd ("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. Fixes: cba6cfd ("regulator: core: Avoid lockdep reports when resolving supplies") Fixes: eaa7995 ("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]>
1 parent 7eb1721 commit 1c81a8c

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
@@ -1830,12 +1830,49 @@ static const struct file_operations constraint_flags_fops = {
18301830

18311831
#define REG_STR_SIZE 64
18321832

1833+
static void link_and_create_debugfs(struct regulator *regulator, struct regulator_dev *rdev,
1834+
struct device *dev)
1835+
{
1836+
int err = 0;
1837+
1838+
if (dev) {
1839+
regulator->dev = dev;
1840+
1841+
/* Add a link to the device sysfs entry */
1842+
err = sysfs_create_link_nowarn(&rdev->dev.kobj, &dev->kobj,
1843+
regulator->supply_name);
1844+
if (err) {
1845+
rdev_dbg(rdev, "could not add device link %s: %pe\n",
1846+
dev->kobj.name, ERR_PTR(err));
1847+
/* non-fatal */
1848+
}
1849+
}
1850+
1851+
if (err != -EEXIST) {
1852+
regulator->debugfs = debugfs_create_dir(regulator->supply_name, rdev->debugfs);
1853+
if (IS_ERR(regulator->debugfs)) {
1854+
rdev_dbg(rdev, "Failed to create debugfs directory\n");
1855+
regulator->debugfs = NULL;
1856+
}
1857+
}
1858+
1859+
if (regulator->debugfs) {
1860+
debugfs_create_u32("uA_load", 0444, regulator->debugfs,
1861+
&regulator->uA_load);
1862+
debugfs_create_u32("min_uV", 0444, regulator->debugfs,
1863+
&regulator->voltage[PM_SUSPEND_ON].min_uV);
1864+
debugfs_create_u32("max_uV", 0444, regulator->debugfs,
1865+
&regulator->voltage[PM_SUSPEND_ON].max_uV);
1866+
debugfs_create_file("constraint_flags", 0444, regulator->debugfs,
1867+
regulator, &constraint_flags_fops);
1868+
}
1869+
}
1870+
18331871
static struct regulator *create_regulator(struct regulator_dev *rdev,
18341872
struct device *dev,
18351873
const char *supply_name)
18361874
{
18371875
struct regulator *regulator;
1838-
int err = 0;
18391876

18401877
lockdep_assert_held_once(&rdev->mutex.base);
18411878

@@ -1868,38 +1905,6 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
18681905

18691906
list_add(&regulator->list, &rdev->consumer_list);
18701907

1871-
if (dev) {
1872-
regulator->dev = dev;
1873-
1874-
/* Add a link to the device sysfs entry */
1875-
err = sysfs_create_link_nowarn(&rdev->dev.kobj, &dev->kobj,
1876-
supply_name);
1877-
if (err) {
1878-
rdev_dbg(rdev, "could not add device link %s: %pe\n",
1879-
dev->kobj.name, ERR_PTR(err));
1880-
/* non-fatal */
1881-
}
1882-
}
1883-
1884-
if (err != -EEXIST) {
1885-
regulator->debugfs = debugfs_create_dir(supply_name, rdev->debugfs);
1886-
if (IS_ERR(regulator->debugfs)) {
1887-
rdev_dbg(rdev, "Failed to create debugfs directory\n");
1888-
regulator->debugfs = NULL;
1889-
}
1890-
}
1891-
1892-
if (regulator->debugfs) {
1893-
debugfs_create_u32("uA_load", 0444, regulator->debugfs,
1894-
&regulator->uA_load);
1895-
debugfs_create_u32("min_uV", 0444, regulator->debugfs,
1896-
&regulator->voltage[PM_SUSPEND_ON].min_uV);
1897-
debugfs_create_u32("max_uV", 0444, regulator->debugfs,
1898-
&regulator->voltage[PM_SUSPEND_ON].max_uV);
1899-
debugfs_create_file("constraint_flags", 0444, regulator->debugfs,
1900-
regulator, &constraint_flags_fops);
1901-
}
1902-
19031908
/*
19041909
* Check now if the regulator is an always on regulator - if
19051910
* it is then we don't need to do nearly so much work for
@@ -2133,6 +2138,9 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
21332138

21342139
regulator_unlock_two(rdev, r, &ww_ctx);
21352140

2141+
/* rdev->supply was created in set_supply() */
2142+
link_and_create_debugfs(rdev->supply, r, &rdev->dev);
2143+
21362144
/*
21372145
* In set_machine_constraints() we may have turned this regulator on
21382146
* but we couldn't propagate to the supply if it hadn't been resolved
@@ -2271,6 +2279,8 @@ struct regulator *_regulator_get_common(struct regulator_dev *rdev, struct devic
22712279
return regulator;
22722280
}
22732281

2282+
link_and_create_debugfs(regulator, rdev, dev);
2283+
22742284
rdev->open_count++;
22752285
if (get_type == EXCLUSIVE_GET) {
22762286
rdev->exclusive = 1;

0 commit comments

Comments
 (0)