diff options
author | Ludvig Pärsson <ludvig.parsson@axis.com> | 2025-03-05 19:05:04 +0300 |
---|---|---|
committer | Mark Brown <broonie@kernel.org> | 2025-03-05 19:35:11 +0300 |
commit | 1c81a8c78ae653f3a21cde0f37a91f1b22b7d2fb (patch) | |
tree | 85ab7291686abdf6f0f7f1505d49e8077e773458 | |
parent | 7eb172143d5508b4da468ed59ee857c6e5e01da6 (diff) | |
download | linux-1c81a8c78ae653f3a21cde0f37a91f1b22b7d2fb.tar.xz |
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.
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 <ludvig.parsson@axis.com>
Link: https://patch.msgid.link/20250305-regulator_lockdep_fix-v1-1-ab938b12e790@axis.com
Signed-off-by: Mark Brown <broonie@kernel.org>
-rw-r--r-- | drivers/regulator/core.c | 76 |
1 files changed, 43 insertions, 33 deletions
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 4ddf0efead68..c993ab48f415 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1830,12 +1830,49 @@ static const struct file_operations constraint_flags_fops = { #define REG_STR_SIZE 64 +static void link_and_create_debugfs(struct regulator *regulator, struct regulator_dev *rdev, + struct device *dev) +{ + int err = 0; + + if (dev) { + regulator->dev = dev; + + /* Add a link to the device sysfs entry */ + err = sysfs_create_link_nowarn(&rdev->dev.kobj, &dev->kobj, + regulator->supply_name); + if (err) { + rdev_dbg(rdev, "could not add device link %s: %pe\n", + dev->kobj.name, ERR_PTR(err)); + /* non-fatal */ + } + } + + if (err != -EEXIST) { + regulator->debugfs = debugfs_create_dir(regulator->supply_name, rdev->debugfs); + if (IS_ERR(regulator->debugfs)) { + rdev_dbg(rdev, "Failed to create debugfs directory\n"); + regulator->debugfs = NULL; + } + } + + if (regulator->debugfs) { + debugfs_create_u32("uA_load", 0444, regulator->debugfs, + ®ulator->uA_load); + debugfs_create_u32("min_uV", 0444, regulator->debugfs, + ®ulator->voltage[PM_SUSPEND_ON].min_uV); + debugfs_create_u32("max_uV", 0444, regulator->debugfs, + ®ulator->voltage[PM_SUSPEND_ON].max_uV); + debugfs_create_file("constraint_flags", 0444, regulator->debugfs, + regulator, &constraint_flags_fops); + } +} + static struct regulator *create_regulator(struct regulator_dev *rdev, struct device *dev, const char *supply_name) { struct regulator *regulator; - int err = 0; lockdep_assert_held_once(&rdev->mutex.base); @@ -1868,38 +1905,6 @@ static struct regulator *create_regulator(struct regulator_dev *rdev, list_add(®ulator->list, &rdev->consumer_list); - if (dev) { - regulator->dev = dev; - - /* Add a link to the device sysfs entry */ - err = sysfs_create_link_nowarn(&rdev->dev.kobj, &dev->kobj, - supply_name); - if (err) { - rdev_dbg(rdev, "could not add device link %s: %pe\n", - dev->kobj.name, ERR_PTR(err)); - /* non-fatal */ - } - } - - if (err != -EEXIST) { - regulator->debugfs = debugfs_create_dir(supply_name, rdev->debugfs); - if (IS_ERR(regulator->debugfs)) { - rdev_dbg(rdev, "Failed to create debugfs directory\n"); - regulator->debugfs = NULL; - } - } - - if (regulator->debugfs) { - debugfs_create_u32("uA_load", 0444, regulator->debugfs, - ®ulator->uA_load); - debugfs_create_u32("min_uV", 0444, regulator->debugfs, - ®ulator->voltage[PM_SUSPEND_ON].min_uV); - debugfs_create_u32("max_uV", 0444, regulator->debugfs, - ®ulator->voltage[PM_SUSPEND_ON].max_uV); - debugfs_create_file("constraint_flags", 0444, regulator->debugfs, - regulator, &constraint_flags_fops); - } - /* * Check now if the regulator is an always on regulator - if * 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) regulator_unlock_two(rdev, r, &ww_ctx); + /* rdev->supply was created in set_supply() */ + link_and_create_debugfs(rdev->supply, r, &rdev->dev); + /* * 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 @@ -2271,6 +2279,8 @@ struct regulator *_regulator_get_common(struct regulator_dev *rdev, struct devic return regulator; } + link_and_create_debugfs(regulator, rdev, dev); + rdev->open_count++; if (get_type == EXCLUSIVE_GET) { rdev->exclusive = 1; |