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>

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 4ddf0efead682fd006657cdad1dc335f08f1da3e..c993ab48f4153965706567b3f497d461c46f535b 100644 (file)
--- 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,
+                                  &regulator->uA_load);
+               debugfs_create_u32("min_uV", 0444, regulator->debugfs,
+                                  &regulator->voltage[PM_SUSPEND_ON].min_uV);
+               debugfs_create_u32("max_uV", 0444, regulator->debugfs,
+                                  &regulator->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(&regulator->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,
-                                  &regulator->uA_load);
-               debugfs_create_u32("min_uV", 0444, regulator->debugfs,
-                                  &regulator->voltage[PM_SUSPEND_ON].min_uV);
-               debugfs_create_u32("max_uV", 0444, regulator->debugfs,
-                                  &regulator->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;