mirror of
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2025-09-04 20:19:47 +08:00
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 commitcba6cfdc7c
("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:cba6cfdc7c
("regulator: core: Avoid lockdep reports when resolving supplies") Fixes:eaa7995c52
("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>
This commit is contained in:
parent
7eb172143d
commit
1c81a8c78a
@ -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;
|
||||
|
Loading…
Reference in New Issue
Block a user