interconnect: avoid memory allocation when 'icc_bw_lock' is held

The 'icc_bw_lock' mutex is introduced in commit af42269c3523
("interconnect: Fix locking for runpm vs reclaim") in order to decouple
serialization of bw aggregation from codepaths that require memory
allocation.

However commit d30f83d278a9 ("interconnect: core: Add dynamic id
allocation support") added a devm_kasprintf() call into a path protected
by the 'icc_bw_lock' which causes the following lockdep warning on
machines like the Lenovo ThinkPad X13s:

    ======================================================
    WARNING: possible circular locking dependency detected
    6.16.0-rc3 #15 Not tainted
    ------------------------------------------------------
    (udev-worker)/342 is trying to acquire lock:
    ffffb973f7ec4638 (fs_reclaim){+.+.}-{0:0}, at: __kmalloc_node_track_caller_noprof+0xa0/0x3e0

    but task is already holding lock:
    ffffb973f7f7f0e8 (icc_bw_lock){+.+.}-{4:4}, at: icc_node_add+0x44/0x154

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -> #1 (icc_bw_lock){+.+.}-{4:4}:
           icc_init+0x48/0x108
           do_one_initcall+0x64/0x30c
           kernel_init_freeable+0x27c/0x500
           kernel_init+0x20/0x1d8
           ret_from_fork+0x10/0x20

    -> #0 (fs_reclaim){+.+.}-{0:0}:
           __lock_acquire+0x136c/0x2114
           lock_acquire+0x1c8/0x354
           fs_reclaim_acquire+0x74/0xa8
           __kmalloc_node_track_caller_noprof+0xa0/0x3e0
           devm_kmalloc+0x54/0x124
           devm_kvasprintf+0x74/0xd4
           devm_kasprintf+0x58/0x80
           icc_node_add+0xb4/0x154
           qcom_osm_l3_probe+0x20c/0x314 [icc_osm_l3]
           platform_probe+0x68/0xd8
           really_probe+0xc0/0x38c
           __driver_probe_device+0x7c/0x160
           driver_probe_device+0x40/0x110
           __driver_attach+0xfc/0x208
           bus_for_each_dev+0x74/0xd0
           driver_attach+0x24/0x30
           bus_add_driver+0x110/0x234
           driver_register+0x60/0x128
           __platform_driver_register+0x24/0x30
           osm_l3_driver_init+0x20/0x1000 [icc_osm_l3]
           do_one_initcall+0x64/0x30c
           do_init_module+0x58/0x23c
           load_module+0x1df8/0x1f70
           init_module_from_file+0x88/0xc4
           idempotent_init_module+0x188/0x280
           __arm64_sys_finit_module+0x6c/0xd8
           invoke_syscall+0x48/0x110
           el0_svc_common.constprop.0+0xc0/0xe0
           do_el0_svc+0x1c/0x28
           el0_svc+0x4c/0x158
           el0t_64_sync_handler+0xc8/0xcc
           el0t_64_sync+0x198/0x19c

    other info that might help us debug this:

     Possible unsafe locking scenario:

           CPU0                    CPU1
           ----                    ----
      lock(icc_bw_lock);
                                   lock(fs_reclaim);
                                   lock(icc_bw_lock);
      lock(fs_reclaim);

     *** DEADLOCK ***

The icc_node_add() functions is not designed to fail, and as such it
should not do any memory allocation. In order to avoid this, add a new
helper function for the name generation to be called by drivers which
are using the new dynamic id feature.

Fixes: d30f83d278a9 ("interconnect: core: Add dynamic id allocation support")
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
Link: https://lore.kernel.org/r/20250625-icc-bw-lockdep-v3-1-2b8f8b8987c4@gmail.com
Co-developed-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Link: https://lore.kernel.org/r/20250627075854.26943-1-johan+linaro@kernel.org
Signed-off-by: Georgi Djakov <djakov@kernel.org>

diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 3a41b2717edd995d0fb864bbea005a827a33c87a..3ebf37ddfc18a3b4f86e97b251749fefe2822ce1 100644 (file)
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -909,10 +909,35 @@ void icc_node_destroy(int id)
                return;
 
        kfree(node->links);
+       if (node->id >= ICC_DYN_ID_START)
+               kfree(node->name);
        kfree(node);
 }
 EXPORT_SYMBOL_GPL(icc_node_destroy);
 
+/**
+ * icc_node_set_name() - set node name
+ * @node: node
+ * @provider: node provider
+ * @name: node name
+ *
+ * Return: 0 on success, or -ENOMEM on allocation failure
+ */
+int icc_node_set_name(struct icc_node *node, const struct icc_provider *provider, const char *name)
+{
+       if (node->id >= ICC_DYN_ID_START) {
+               node->name = kasprintf(GFP_KERNEL, "%s@%s", name,
+                                      dev_name(provider->dev));
+               if (!node->name)
+                       return -ENOMEM;
+       } else {
+               node->name = name;
+       }
+
+       return 0;
+}
+EXPORT_SYMBOL_GPL(icc_node_set_name);
+
 /**
  * icc_link_nodes() - create link between two nodes
  * @src_node: source node
@@ -1041,10 +1066,6 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider)
        node->avg_bw = node->init_avg;
        node->peak_bw = node->init_peak;
 
-       if (node->id >= ICC_DYN_ID_START)
-               node->name = devm_kasprintf(provider->dev, GFP_KERNEL, "%s@%s",
-                                           node->name, dev_name(provider->dev));
-
        if (node->avg_bw || node->peak_bw) {
                if (provider->pre_aggregate)
                        provider->pre_aggregate(node);

diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c
index 41bfc6e7ee1d53d34b919dd8afa97698bc69d79c..001404e91041597eab7f251606873182e52e360c 100644 (file)
--- a/drivers/interconnect/qcom/icc-rpmh.c
+++ b/drivers/interconnect/qcom/icc-rpmh.c
@@ -293,7 +293,12 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
                        goto err_remove_nodes;
                }
 
-               node->name = qn->name;
+               ret = icc_node_set_name(node, provider, qn->name);
+               if (ret) {
+                       icc_node_destroy(node->id);
+                       goto err_remove_nodes;
+               }
+
                node->data = qn;
                icc_node_add(node, provider);
 

diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
index baecbf2533f76cbf92bb2451979c4db57f8e4a2b..b33f00da1880de509b79400e05f0f0d0ac68aca3 100644 (file)
--- a/drivers/interconnect/qcom/osm-l3.c
+++ b/drivers/interconnect/qcom/osm-l3.c
@@ -236,7 +236,12 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
                        goto err;
                }
 
-               node->name = qnodes[i]->name;
+               ret = icc_node_set_name(node, provider, qnodes[i]->name);
+               if (ret) {
+                       icc_node_destroy(node->id);
+                       goto err;
+               }
+
                /* Cast away const and add it back in qcom_osm_l3_set() */
                node->data = (void *)qnodes[i];
                icc_node_add(node, provider);

diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
index 55cfebc658e6831659885adde2e97251efd13f77..8a2f652a05ff7d0746096fb8b81dc0b9557c9aae 100644 (file)
--- a/include/linux/interconnect-provider.h
+++ b/include/linux/interconnect-provider.h
@@ -119,6 +119,7 @@ int icc_std_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
 struct icc_node *icc_node_create_dyn(void);
 struct icc_node *icc_node_create(int id);
 void icc_node_destroy(int id);
+int icc_node_set_name(struct icc_node *node, const struct icc_provider *provider, const char *name);
 int icc_link_nodes(struct icc_node *src_node, struct icc_node **dst_node);
 int icc_link_create(struct icc_node *node, const int dst_id);
 void icc_node_add(struct icc_node *node, struct icc_provider *provider);
@@ -152,6 +153,12 @@ static inline void icc_node_destroy(int id)
 {
 }
 
+static inline int icc_node_set_name(struct icc_node *node, const struct icc_provider *provider,
+                                   const char *name)
+{
+       return -EOPNOTSUPP;
+}
+
 static inline int icc_link_nodes(struct icc_node *src_node, struct icc_node **dst_node)
 {
        return -EOPNOTSUPP;