This series is to fix and refactor CoreSight device registration and unregistration, it can be divided into three small parts:
Patches 01-03: Three fixes for memory leak, device reference and mutex protection. Patches 04-05: Move connection cleanup operations into coresight_remove_conns(). Patches 06-08: Refactor error handling in coresight_register().
This series is verified on Juno board with kmemleak detector.
For kmemleak verifying:
echo clear > /sys/kernel/debug/kmemleak insmod coresight modules rmmod coresight modules echo scan > /sys/kernel/debug/kmemleak
The result shows no memory leak during a cycle of device registration and unregistration.
--- Changes in v3: - Updated patch 01 to use coesight core layer for device index list (Suzuki). - Link to v2: https://lore.kernel.org/r/20260126-arm_coresight_refactor_dev_register-v2-0-...
Changes in v2: - Refined the commit log in patch 06 (Suzuki). - Unified to call coresight_unregister() for error handling (Suzuki). - Refactor connection and sysfs group release. - Link to v1: https://lore.kernel.org/linux-arm-kernel/20250512154108.23920-1-leo.yan@arm....
To: Suzuki K Poulose suzuki.poulose@arm.com To: Mike Leach mike.leach@arm.com To: James Clark james.clark@linaro.org To: Alexander Shishkin alexander.shishkin@linux.intel.com To: Greg Kroah-Hartman gregkh@linuxfoundation.org To: Mathieu Poirier mathieu.poirier@linaro.org To: Mao Jinlong quic_jinlmao@quicinc.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Leo Yan leo.yan@arm.com
--- Leo Yan (8): coresight: Fix memory leak in coresight_alloc_device_name() coresight: Get parent device reference after sink ID map allocation coresight: Protect unregistration with mutex coresight: Refactor output connection sysfs link cleanup coresight: Refactor sysfs connection group cleanup coresight: Move sink validation into etm_perf_add_symlink_sink() coresight: Do not mix success path with failure handling coresight: Unify error handling in coresight_register()
drivers/hwtracing/coresight/coresight-catu.c | 4 +- drivers/hwtracing/coresight/coresight-core.c | 230 +++++++++++++-------- drivers/hwtracing/coresight/coresight-ctcu-core.c | 4 +- drivers/hwtracing/coresight/coresight-cti-core.c | 19 +- drivers/hwtracing/coresight/coresight-dummy.c | 7 +- drivers/hwtracing/coresight/coresight-etb10.c | 4 +- drivers/hwtracing/coresight/coresight-etm-perf.c | 5 +- drivers/hwtracing/coresight/coresight-funnel.c | 4 +- drivers/hwtracing/coresight/coresight-platform.c | 2 +- drivers/hwtracing/coresight/coresight-priv.h | 3 +- drivers/hwtracing/coresight/coresight-replicator.c | 4 +- drivers/hwtracing/coresight/coresight-stm.c | 4 +- drivers/hwtracing/coresight/coresight-tmc-core.c | 12 +- drivers/hwtracing/coresight/coresight-tnoc.c | 4 +- drivers/hwtracing/coresight/coresight-tpda.c | 4 +- drivers/hwtracing/coresight/coresight-tpdm.c | 4 +- drivers/hwtracing/coresight/coresight-tpiu.c | 4 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 4 +- include/linux/coresight.h | 14 +- 19 files changed, 175 insertions(+), 161 deletions(-) --- base-commit: eebe8dbd8630f51cf70b1f68a440cd3d7f7a914d change-id: 20260120-arm_coresight_refactor_dev_register-f16c069db41d
Best regards,
The memory leak detector reports:
echo clear > /sys/kernel/debug/kmemleak modprobe coresight_funnel rmmod coresight_funnel
# Scan memory leak and report it echo scan > /sys/kernel/debug/kmemleak cat /sys/kernel/debug/kmemleak unreferenced object 0xffff0008020c7200 (size 64): comm "modprobe", pid 410, jiffies 4295333721 hex dump (first 32 bytes): d8 da fe 7e 09 00 ff ff e8 2e ff 7e 09 00 ff ff ...~.......~.... b0 6c ff 7e 09 00 ff ff 30 83 00 7f 09 00 ff ff .l.~....0....... backtrace (crc 4116a690): kmemleak_alloc+0xd8/0xf8 __kmalloc_node_track_caller_noprof+0x2c8/0x6f0 krealloc_node_align_noprof+0x13c/0x2c8 coresight_alloc_device_name+0xe4/0x158 [coresight] 0xffffd327ecef8394 0xffffd327ecef85ec amba_probe+0x118/0x1c8 really_probe+0xc8/0x3f0 __driver_probe_device+0x88/0x190 driver_probe_device+0x44/0x120 __driver_attach+0x100/0x238 bus_for_each_dev+0x84/0xf0 driver_attach+0x2c/0x40 bus_add_driver+0x128/0x258 driver_register+0x64/0x138 __amba_driver_register+0x2c/0x48
The memory leak is caused by not freeing the device list that maintains device indices.
This device list preserves stable device indices across unbind and rebind device operations, so it does not share the same lifetime as a device instances and must only be freed when the module is unloaded.
Some modules do not implement a module exit callback because they are registered using module_platform_driver(). As a result, the device list cannot be released during module exit for those modules.
Fix this by moving the device list into the core layer. As a general solution, instead of maintaining a static list in each driver, drivers now allocate device lists via coresight_allocate_device_list() and device indices via coresight_allocate_device_idx().
The list is released only when the core module is unloaded by calling coresight_release_device_list(), avoiding the leak.
Fixes: 0f5f9b6ba9e1 ("coresight: Use platform agnostic names") Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-catu.c | 4 +- drivers/hwtracing/coresight/coresight-core.c | 127 +++++++++++++++------ drivers/hwtracing/coresight/coresight-ctcu-core.c | 4 +- drivers/hwtracing/coresight/coresight-cti-core.c | 19 ++- drivers/hwtracing/coresight/coresight-dummy.c | 7 +- drivers/hwtracing/coresight/coresight-etb10.c | 4 +- drivers/hwtracing/coresight/coresight-funnel.c | 4 +- drivers/hwtracing/coresight/coresight-replicator.c | 4 +- drivers/hwtracing/coresight/coresight-stm.c | 4 +- drivers/hwtracing/coresight/coresight-tmc-core.c | 12 +- drivers/hwtracing/coresight/coresight-tnoc.c | 4 +- drivers/hwtracing/coresight/coresight-tpda.c | 4 +- drivers/hwtracing/coresight/coresight-tpdm.c | 4 +- drivers/hwtracing/coresight/coresight-tpiu.c | 4 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 4 +- include/linux/coresight.h | 14 +-- 16 files changed, 120 insertions(+), 103 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 69b36bae97ab488197f23e6eafe3729ca22445d4..4e7e60ac86e8073db36aeb29e265bcebe72981d3 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -30,8 +30,6 @@ #define catu_dbg(x, ...) do {} while (0) #endif
-DEFINE_CORESIGHT_DEVLIST(catu_devs, "catu"); - struct catu_etr_buf { struct tmc_sg_table *catu_table; dma_addr_t sladdr; @@ -530,7 +528,7 @@ static int __catu_probe(struct device *dev, struct resource *res) if (ret) return ret;
- catu_desc.name = coresight_alloc_device_name(&catu_devs, dev); + catu_desc.name = coresight_alloc_device_name("catu", dev); if (!catu_desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index c660cf8adb1c7cafff8f85e501f056e4e151e372..fe345832c4564bd55b39ca8bcde0a93afdc11a68 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -53,6 +53,9 @@ struct coresight_node { const u32 coresight_barrier_pkt[4] = {0x7fffffff, 0x7fffffff, 0x7fffffff, 0x7fffffff}; EXPORT_SYMBOL_GPL(coresight_barrier_pkt);
+/* List maintains the device index */ +static LIST_HEAD(coresight_dev_idx_list); + static const struct cti_assoc_op *cti_assoc_ops;
void coresight_set_cti_ops(const struct cti_assoc_op *cti_op) @@ -1438,22 +1441,55 @@ void coresight_unregister(struct coresight_device *csdev) } EXPORT_SYMBOL_GPL(coresight_unregister);
+static struct coresight_dev_list * +coresight_allocate_device_list(const char *prefix) +{ + struct coresight_dev_list *list;
-/* - * coresight_search_device_idx - Search the fwnode handle of a device - * in the given dev_idx list. Must be called with the coresight_mutex held. - * - * Returns the index of the entry, when found. Otherwise, -ENOENT. - */ -static int coresight_search_device_idx(struct coresight_dev_list *dict, - struct fwnode_handle *fwnode) + /* Check if have already allocated */ + list_for_each_entry(list, &coresight_dev_idx_list, node) { + if (!strcmp(list->pfx, prefix)) + return list; + } + + list = kzalloc(sizeof(*list), GFP_KERNEL); + if (!list) + return NULL; + + list->pfx = kstrdup(prefix, GFP_KERNEL); + if (!list->pfx) { + kfree(list); + return NULL; + } + + list_add(&list->node, &coresight_dev_idx_list); + return list; +} + +static int coresight_allocate_device_idx(struct coresight_dev_list *list, + struct device *dev) { - int i; + struct fwnode_handle **fwnode_list; + struct fwnode_handle *fwnode = dev_fwnode(dev); + int idx; + + for (idx = 0; idx < list->nr_idx; idx++) + if (list->fwnode_list[idx] == fwnode) + return idx; + + /* Make space for the new entry */ + idx = list->nr_idx; + fwnode_list = krealloc_array(list->fwnode_list, + idx + 1, sizeof(*list->fwnode_list), + GFP_KERNEL); + if (!fwnode_list) + return -ENOMEM;
- for (i = 0; i < dict->nr_idx; i++) - if (dict->fwnode_list[i] == fwnode) - return i; - return -ENOENT; + fwnode_list[idx] = fwnode; + list->fwnode_list = fwnode_list; + list->nr_idx = idx + 1; + + return idx; }
static bool coresight_compare_type(enum coresight_dev_type type_a, @@ -1527,45 +1563,63 @@ bool coresight_loses_context_with_cpu(struct device *dev) EXPORT_SYMBOL_GPL(coresight_loses_context_with_cpu);
/* - * coresight_alloc_device_name - Get an index for a given device in the - * device index list specific to a driver. An index is allocated for a - * device and is tracked with the fwnode_handle to prevent allocating + * coresight_alloc_device_name - Get an index for a given device in the list + * specific to a driver (presented by the prefix string). An index is allocated + * for a device and is tracked with the fwnode_handle to prevent allocating * duplicate indices for the same device (e.g, if we defer probing of * a device due to dependencies), in case the index is requested again. */ -char *coresight_alloc_device_name(struct coresight_dev_list *dict, - struct device *dev) +char *coresight_alloc_device_name(const char *prefix, struct device *dev) { - int idx; + struct coresight_dev_list *list; char *name = NULL; - struct fwnode_handle **list; + int idx;
mutex_lock(&coresight_mutex);
- idx = coresight_search_device_idx(dict, dev_fwnode(dev)); - if (idx < 0) { - /* Make space for the new entry */ - idx = dict->nr_idx; - list = krealloc_array(dict->fwnode_list, - idx + 1, sizeof(*dict->fwnode_list), - GFP_KERNEL); - if (ZERO_OR_NULL_PTR(list)) { - idx = -ENOMEM; - goto done; - } + list = coresight_allocate_device_list(prefix); + if (!list) + goto done;
- list[idx] = dev_fwnode(dev); - dict->fwnode_list = list; - dict->nr_idx = idx + 1; - } + idx = coresight_allocate_device_idx(list, dev);
- name = devm_kasprintf(dev, GFP_KERNEL, "%s%d", dict->pfx, idx); + /* + * If index allocation fails, the device list is not released here; + * it is instead freed later by coresight_release_device_list() when + * the coresight_core module is unloaded. + */ + if (idx < 0) + goto done; + + name = devm_kasprintf(dev, GFP_KERNEL, "%s%d", list->pfx, idx); done: mutex_unlock(&coresight_mutex); return name; } EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
+static void coresight_release_device_list(void) +{ + struct coresight_dev_list *list, *next; + int i; + + /* + * Here is no need to take coresight_mutex; this is during core module + * unloading, no race condition with other modules. + */ + + list_for_each_entry_safe(list, next, &coresight_dev_idx_list, node) { + for (i = 0; i < list->nr_idx; i++) + list->fwnode_list[i] = NULL; + list->nr_idx = 0; + list_del(&list->node); + + kfree(list->pfx); + kfree(list->fwnode_list); + kfree(list); + } +} + const struct bus_type coresight_bustype = { .name = "coresight", }; @@ -1639,6 +1693,7 @@ static void __exit coresight_exit(void) &coresight_notifier); etm_perf_exit(); bus_unregister(&coresight_bustype); + coresight_release_device_list(); }
module_init(coresight_init); diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c index abed15eb72b4acb0d6eb743616afe1b414f1f639..6813ae6e929b05cdc4dcbaab9ff6584653ff4f39 100644 --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c @@ -19,8 +19,6 @@ #include "coresight-ctcu.h" #include "coresight-priv.h"
-DEFINE_CORESIGHT_DEVLIST(ctcu_devs, "ctcu"); - #define ctcu_writel(drvdata, val, offset) __raw_writel((val), drvdata->base + offset) #define ctcu_readl(drvdata, offset) __raw_readl(drvdata->base + offset)
@@ -187,7 +185,7 @@ static int ctcu_probe(struct platform_device *pdev) void __iomem *base; int i, ret;
- desc.name = coresight_alloc_device_name(&ctcu_devs, dev); + desc.name = coresight_alloc_device_name("ctcu", dev); if (!desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index bfbc365bb2ef2744efab11c056b8450472957005..0f1374a79b5765475b7189247049199913f5a99b 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -48,15 +48,6 @@ static int nr_cti_cpu; /* quick lookup list for CPU bound CTIs when power handling */ static struct cti_drvdata *cti_cpu_drvdata[NR_CPUS];
-/* - * CTI naming. CTI bound to cores will have the name cti_cpu<N> where - * N is the CPU ID. System CTIs will have the name cti_sys<I> where I - * is an index allocated by order of discovery. - * - * CTI device name list - for CTI not bound to cores. - */ -DEFINE_CORESIGHT_DEVLIST(cti_sys_devs, "cti_sys"); - /* write set of regs to hardware - call with spinlock claimed */ void cti_write_all_hw_regs(struct cti_drvdata *drvdata) { @@ -903,12 +894,18 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) /* default to powered - could change on PM notifications */ drvdata->config.hw_powered = true;
- /* set up device name - will depend if cpu bound or otherwise */ + /* + * Set up device name - will depend if cpu bound or otherwise. + * + * CTI bound to cores will have the name cti_cpu<N> where N is th + * eCPU ID. System CTIs will have the name cti_sys<I> where I is an + * index allocated by order of discovery. + */ if (drvdata->ctidev.cpu >= 0) cti_desc.name = devm_kasprintf(dev, GFP_KERNEL, "cti_cpu%d", drvdata->ctidev.cpu); else - cti_desc.name = coresight_alloc_device_name(&cti_sys_devs, dev); + cti_desc.name = coresight_alloc_device_name("cti_sys", dev); if (!cti_desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c index 14322c99e29d4696bc27ac2326280026412fe05f..c176a2f57300fd8cd5d153be5ccb6faaef96023d 100644 --- a/drivers/hwtracing/coresight/coresight-dummy.c +++ b/drivers/hwtracing/coresight/coresight-dummy.c @@ -19,9 +19,6 @@ struct dummy_drvdata { u8 traceid; };
-DEFINE_CORESIGHT_DEVLIST(source_devs, "dummy_source"); -DEFINE_CORESIGHT_DEVLIST(sink_devs, "dummy_sink"); - static int dummy_source_enable(struct coresight_device *csdev, struct perf_event *event, enum cs_mode mode, __maybe_unused struct coresight_path *path) @@ -126,7 +123,7 @@ static int dummy_probe(struct platform_device *pdev)
if (of_device_is_compatible(node, "arm,coresight-dummy-source")) {
- desc.name = coresight_alloc_device_name(&source_devs, dev); + desc.name = coresight_alloc_device_name("dummy_source", dev); if (!desc.name) return -ENOMEM;
@@ -155,7 +152,7 @@ static int dummy_probe(struct platform_device *pdev) drvdata->traceid = (u8)trace_id;
} else if (of_device_is_compatible(node, "arm,coresight-dummy-sink")) { - desc.name = coresight_alloc_device_name(&sink_devs, dev); + desc.name = coresight_alloc_device_name("dummy_sink", dev); if (!desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 6657602d8f2e1b75ea6a8440e35c489d21f4245e..b952a1d47f12f44c15cc845914691305713028c4 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -63,8 +63,6 @@ #define ETB_FFSR_BIT 1 #define ETB_FRAME_SIZE_WORDS 4
-DEFINE_CORESIGHT_DEVLIST(etb_devs, "etb"); - /** * struct etb_drvdata - specifics associated to an ETB component * @base: memory mapped base address for this component. @@ -722,7 +720,7 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) struct resource *res = &adev->res; struct coresight_desc desc = { 0 };
- desc.name = coresight_alloc_device_name(&etb_devs, dev); + desc.name = coresight_alloc_device_name("etb", dev); if (!desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 3b248e54471a38f501777fe162fea850d1c851b3..3f56ceccd8c9f6deaaf783a06a0d19fb06eff7c4 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -30,8 +30,6 @@ #define FUNNEL_HOLDTIME (0x7 << FUNNEL_HOLDTIME_SHFT) #define FUNNEL_ENSx_MASK 0xff
-DEFINE_CORESIGHT_DEVLIST(funnel_devs, "funnel"); - /** * struct funnel_drvdata - specifics associated to a funnel component * @base: memory mapped base address for this component. @@ -223,7 +221,7 @@ static int funnel_probe(struct device *dev, struct resource *res) of_device_is_compatible(dev->of_node, "arm,coresight-funnel")) dev_warn_once(dev, "Uses OBSOLETE CoreSight funnel binding\n");
- desc.name = coresight_alloc_device_name(&funnel_devs, dev); + desc.name = coresight_alloc_device_name("funnel", dev); if (!desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index e6472658235dc479cec91ac18f3737f76f8c74f0..07fc04f53b88fdec7ae18d3000046fe74c0cec9d 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -24,8 +24,6 @@ #define REPLICATOR_IDFILTER0 0x000 #define REPLICATOR_IDFILTER1 0x004
-DEFINE_CORESIGHT_DEVLIST(replicator_devs, "replicator"); - /** * struct replicator_drvdata - specifics associated to a replicator component * @base: memory mapped base address for this component. Also indicates @@ -230,7 +228,7 @@ static int replicator_probe(struct device *dev, struct resource *res) dev_warn_once(dev, "Uses OBSOLETE CoreSight replicator binding\n");
- desc.name = coresight_alloc_device_name(&replicator_devs, dev); + desc.name = coresight_alloc_device_name("replicator", dev); if (!desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index e68529bf89c9815a8118955bf3114ad1ed4fb346..aca6cec7885a551ad5960986b1cdf08baa6b5a94 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -110,8 +110,6 @@ struct channel_space { unsigned long *guaranteed; };
-DEFINE_CORESIGHT_DEVLIST(stm_devs, "stm"); - /** * struct stm_drvdata - specifics associated to an STM component * @base: memory mapped base address for this component. @@ -834,7 +832,7 @@ static int __stm_probe(struct device *dev, struct resource *res) struct resource ch_res; struct coresight_desc desc = { 0 };
- desc.name = coresight_alloc_device_name(&stm_devs, dev); + desc.name = coresight_alloc_device_name("stm", dev); if (!desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 36599c431be6203e871fdcb8de569cc6701c52bb..58b469ee73b490932d78f5a6bc704c2abbe2f737 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -32,10 +32,6 @@ #include "coresight-priv.h" #include "coresight-tmc.h"
-DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); -DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); -DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); - int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) { struct coresight_device *csdev = drvdata->csdev; @@ -777,7 +773,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) struct coresight_platform_data *pdata = NULL; struct tmc_drvdata *drvdata; struct coresight_desc desc = { 0 }; - struct coresight_dev_list *dev_list = NULL; + const char *dev_list = NULL;
drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); if (!drvdata) @@ -827,7 +823,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) desc.type = CORESIGHT_DEV_TYPE_SINK; desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; desc.ops = &tmc_etb_cs_ops; - dev_list = &etb_devs; + dev_list = "tmc_etb"; break; case TMC_CONFIG_TYPE_ETR: desc.groups = coresight_etr_groups; @@ -839,7 +835,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) goto out; idr_init(&drvdata->idr); mutex_init(&drvdata->idr_mutex); - dev_list = &etr_devs; + dev_list = "tmc_etr"; break; case TMC_CONFIG_TYPE_ETF: desc.groups = coresight_etf_groups; @@ -847,7 +843,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO; desc.ops = &tmc_etf_cs_ops; - dev_list = &etf_devs; + dev_list = "tmc_etf"; break; default: pr_err("%s: Unsupported TMC config\n", desc.name); diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c index 1128612e70a7312606d79b19ed1f4e91f01ddfc2..96a25877b8240f20921efa979a361eb473c05600 100644 --- a/drivers/hwtracing/coresight/coresight-tnoc.c +++ b/drivers/hwtracing/coresight/coresight-tnoc.c @@ -47,8 +47,6 @@ struct trace_noc_drvdata { int atid; };
-DEFINE_CORESIGHT_DEVLIST(trace_noc_devs, "traceNoc"); - static void trace_noc_enable_hw(struct trace_noc_drvdata *drvdata) { u32 val; @@ -191,7 +189,7 @@ static int _tnoc_probe(struct device *dev, struct resource *res) struct coresight_desc desc = { 0 }; int ret;
- desc.name = coresight_alloc_device_name(&trace_noc_devs, dev); + desc.name = coresight_alloc_device_name("traceNoc", dev); if (!desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c index 7055f8f1342785055ad18d6f7a741a179edc7013..89c8f71f0aff06e87971976efb23feb0057fcce6 100644 --- a/drivers/hwtracing/coresight/coresight-tpda.c +++ b/drivers/hwtracing/coresight/coresight-tpda.c @@ -20,8 +20,6 @@ #include "coresight-trace-id.h" #include "coresight-tpdm.h"
-DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda"); - static void tpda_clear_element_size(struct coresight_device *csdev) { struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); @@ -585,7 +583,7 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id) if (ret) return ret;
- desc.name = coresight_alloc_device_name(&tpda_devs, dev); + desc.name = coresight_alloc_device_name("tpda", dev); if (!desc.name) return -ENOMEM; desc.type = CORESIGHT_DEV_TYPE_LINK; diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c index 06e0a905a67d7aa36625960711c28717b1d27fda..da77bdaad0a4519bd4e0653cc92e2a333e59e60d 100644 --- a/drivers/hwtracing/coresight/coresight-tpdm.c +++ b/drivers/hwtracing/coresight/coresight-tpdm.c @@ -19,8 +19,6 @@ #include "coresight-priv.h" #include "coresight-tpdm.h"
-DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm"); - static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata) { return (drvdata->datasets & TPDM_PIDR0_DS_DSB); @@ -1416,7 +1414,7 @@ static int tpdm_probe(struct device *dev, struct resource *res) }
/* Set up coresight component description */ - desc.name = coresight_alloc_device_name(&tpdm_devs, dev); + desc.name = coresight_alloc_device_name("tpdm", dev); if (!desc.name) return -ENOMEM; desc.type = CORESIGHT_DEV_TYPE_SOURCE; diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index aaa44bc521c330f5b2e385279e0cbda6ce72bccb..b8560b140e0fa4bf8bf1e5c60fe67077b11798fd 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -49,8 +49,6 @@ #define FFCR_FON_MAN BIT(6) #define FFCR_STOP_FI BIT(12)
-DEFINE_CORESIGHT_DEVLIST(tpiu_devs, "tpiu"); - /* * @base: memory mapped base address for this component. * @atclk: optional clock for the core parts of the TPIU. @@ -134,7 +132,7 @@ static int __tpiu_probe(struct device *dev, struct resource *res) struct coresight_desc desc = { 0 }; int ret;
- desc.name = coresight_alloc_device_name(&tpiu_devs, dev); + desc.name = coresight_alloc_device_name("tpiu", dev); if (!desc.name) return -ENOMEM;
diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c index 8f7922a5e534fde57ae4c84f2e72b33749809857..5776f63468fa05df255d3dd716f621c37424c048 100644 --- a/drivers/hwtracing/coresight/ultrasoc-smb.c +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c @@ -17,8 +17,6 @@ #include "coresight-priv.h" #include "ultrasoc-smb.h"
-DEFINE_CORESIGHT_DEVLIST(sink_devs, "ultra_smb"); - #define ULTRASOC_SMB_DSM_UUID "82ae1283-7f6a-4cbe-aa06-53e8fb24db18"
static bool smb_buffer_not_empty(struct smb_drv_data *drvdata) @@ -478,7 +476,7 @@ static int smb_register_sink(struct platform_device *pdev, desc.pdata = pdata; desc.dev = &pdev->dev; desc.groups = smb_sink_groups; - desc.name = coresight_alloc_device_name(&sink_devs, &pdev->dev); + desc.name = coresight_alloc_device_name("ultra_smb", &pdev->dev); if (!desc.name) { dev_err(&pdev->dev, "Failed to alloc coresight device name"); return -ENOMEM; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 2b48be97fcd0d7ea2692206692bd33f35ba4ec79..2131febebee93d609df1aea8534a10898b600be2 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -306,24 +306,19 @@ struct coresight_device { * coresight_dev_list - Mapping for devices to "name" index for device * names. * + * @node: Node on the global device index list. * @nr_idx: Number of entries already allocated. * @pfx: Prefix pattern for device name. * @fwnode_list: Array of fwnode_handles associated with each allocated * index, upto nr_idx entries. */ struct coresight_dev_list { + struct list_head node; int nr_idx; - const char *pfx; + char *pfx; struct fwnode_handle **fwnode_list; };
-#define DEFINE_CORESIGHT_DEVLIST(var, dev_pfx) \ -static struct coresight_dev_list (var) = { \ - .pfx = dev_pfx, \ - .nr_idx = 0, \ - .fwnode_list = NULL, \ -} - #define to_coresight_device(d) container_of(d, struct coresight_device, dev)
/** @@ -663,8 +658,7 @@ void coresight_clear_self_claim_tag(struct csdev_access *csa); void coresight_clear_self_claim_tag_unlocked(struct csdev_access *csa); void coresight_disclaim_device(struct coresight_device *csdev); void coresight_disclaim_device_unlocked(struct coresight_device *csdev); -char *coresight_alloc_device_name(struct coresight_dev_list *devs, - struct device *dev); +char *coresight_alloc_device_name(const char *prefix, struct device *dev);
bool coresight_loses_context_with_cpu(struct device *dev);
The parent device's reference count is incremented before allocating the sink ID map. If the allocation fails, the reference count is not decremented, preventing proper cleanup.
Fix this by incrementing the reference count only after the sink ID map is successfully allocated.
Fixes: 5ad628a76176 ("coresight: Use per-sink trace ID maps for Perf sessions") Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index fe345832c4564bd55b39ca8bcde0a93afdc11a68..26e7ab4005413ab5c59f3ca663b92a681263b488 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1349,12 +1349,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) csdev->dev.parent = desc->dev; csdev->dev.release = coresight_device_release; csdev->dev.bus = &coresight_bustype; - /* - * Hold the reference to our parent device. This will be - * dropped only in coresight_device_release(). - */ - csdev->dev.fwnode = fwnode_handle_get(dev_fwnode(desc->dev)); - dev_set_name(&csdev->dev, "%s", desc->name);
if (csdev->type == CORESIGHT_DEV_TYPE_SINK || csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) { @@ -1366,6 +1360,14 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) goto err_out; } } + + /* + * Hold the reference to our parent device. This will be + * dropped only in coresight_device_release(). + */ + csdev->dev.fwnode = fwnode_handle_get(dev_fwnode(desc->dev)); + dev_set_name(&csdev->dev, "%s", desc->name); + /* * Make sure the device registration and the connection fixup * are synchronised, so that we don't see uninitialised devices
The device registration is protected by CoreSight mutex to ensure the atomic operations when adding a device onto bus. One the other hand, the locking is absent when unregister a device.
Use mutex to ensure atomicity on device unregistration. During unregistration, unbinding the associated CTI device is not included in the locking region, as CTI has its own locking mechanism.
Fixes: 8c1d3f79d9ca ("coresight: core: Fix coresight device probe failure issue") Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 26e7ab4005413ab5c59f3ca663b92a681263b488..5aa178b4def336770ae2b7647372174345564776 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1432,14 +1432,17 @@ EXPORT_SYMBOL_GPL(coresight_register);
void coresight_unregister(struct coresight_device *csdev) { - etm_perf_del_symlink_sink(csdev); /* Remove references of that device in the topology */ if (cti_assoc_ops && cti_assoc_ops->remove) cti_assoc_ops->remove(csdev); + + mutex_lock(&coresight_mutex); + etm_perf_del_symlink_sink(csdev); coresight_remove_conns(csdev); coresight_clear_default_sink(csdev); coresight_release_platform_data(csdev, csdev->dev.parent, csdev->pdata); device_unregister(&csdev->dev); + mutex_unlock(&coresight_mutex); } EXPORT_SYMBOL_GPL(coresight_unregister);
To use a central place for releasing connections, move the output connection sysfs link cleanup into coresight_remove_conns().
Also update the comments accordingly.
Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 5aa178b4def336770ae2b7647372174345564776..4b2cc1806c4f9cdd8a22dbce930973aef41f00c6 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1152,7 +1152,6 @@ static int coresight_clear_filter_source(struct device *dev, void *data) return 0; }
-/* coresight_remove_conns - Remove other device's references to this device */ static void coresight_remove_conns(struct coresight_device *csdev) { int i, j; @@ -1162,10 +1161,6 @@ static void coresight_remove_conns(struct coresight_device *csdev) bus_for_each_dev(&coresight_bustype, NULL, csdev, coresight_clear_filter_source);
- /* - * Remove the input connection references from the destination device - * for each output connection. - */ for (i = 0; i < csdev->pdata->nr_outconns; i++) { conn = csdev->pdata->out_conns[i]; if (conn->filter_src_fwnode) { @@ -1176,6 +1171,13 @@ static void coresight_remove_conns(struct coresight_device *csdev) if (!conn->dest_dev) continue;
+ /* Remove sysfs links for the output connection */ + coresight_remove_links(csdev, conn); + + /* + * Remove the input connection references from the destination + * device for each output connection. + */ for (j = 0; j < conn->dest_dev->pdata->nr_inconns; ++j) if (conn->dest_dev->pdata->in_conns[j] == conn) { conn->dest_dev->pdata->in_conns[j] = NULL; @@ -1306,9 +1308,6 @@ void coresight_release_platform_data(struct coresight_device *csdev, struct coresight_connection **conns = pdata->out_conns;
for (i = 0; i < pdata->nr_outconns; i++) { - /* If we have made the links, remove them now */ - if (csdev && conns[i]->dest_dev) - coresight_remove_links(csdev, conns[i]); /* * Drop the refcount and clear the handle as this device * is going away @@ -1424,7 +1423,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) }
err_out: - /* Cleanup the connection information */ coresight_release_platform_data(NULL, desc->dev, desc->pdata); return ERR_PTR(ret); }
Move the sysfs connection group cleanup into coresight_remove_conns(), so that the driver removes connections and related sysfs resources in one go.
As side effect, the csdev argument to coresight_release_platform_data() is no longer needed; adjust the code for this.
Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 11 +++++------ drivers/hwtracing/coresight/coresight-platform.c | 2 +- drivers/hwtracing/coresight/coresight-priv.h | 3 +-- 3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 4b2cc1806c4f9cdd8a22dbce930973aef41f00c6..b3119832725c08a8375a3a55a11b3bda14b8a3e0 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1200,6 +1200,8 @@ static void coresight_remove_conns(struct coresight_device *csdev) coresight_remove_links(conn->src_dev, conn); conn->dest_dev = NULL; } + + coresight_remove_conns_sysfs_group(csdev); }
/** @@ -1300,8 +1302,7 @@ void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset) * coresight_release_platform_data: Release references to the devices connected * to the output port of this device. */ -void coresight_release_platform_data(struct coresight_device *csdev, - struct device *dev, +void coresight_release_platform_data(struct device *dev, struct coresight_platform_data *pdata) { int i; @@ -1319,8 +1320,6 @@ void coresight_release_platform_data(struct coresight_device *csdev, devm_kfree(dev, pdata->out_conns); devm_kfree(dev, pdata->in_conns); devm_kfree(dev, pdata); - if (csdev) - coresight_remove_conns_sysfs_group(csdev); }
struct coresight_device *coresight_register(struct coresight_desc *desc) @@ -1423,7 +1422,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) }
err_out: - coresight_release_platform_data(NULL, desc->dev, desc->pdata); + coresight_release_platform_data(desc->dev, desc->pdata); return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(coresight_register); @@ -1438,7 +1437,7 @@ void coresight_unregister(struct coresight_device *csdev) etm_perf_del_symlink_sink(csdev); coresight_remove_conns(csdev); coresight_clear_default_sink(csdev); - coresight_release_platform_data(csdev, csdev->dev.parent, csdev->pdata); + coresight_release_platform_data(csdev->dev.parent, csdev->pdata); device_unregister(&csdev->dev); mutex_unlock(&coresight_mutex); } diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c index 0db64c5f499592e1985141161b4f90fa1f0410b4..0ca3bd762454350d33b5630244d0cfe638ee03fb 100644 --- a/drivers/hwtracing/coresight/coresight-platform.c +++ b/drivers/hwtracing/coresight/coresight-platform.c @@ -849,7 +849,7 @@ coresight_get_platform_data(struct device *dev) error: if (!IS_ERR_OR_NULL(pdata)) /* Cleanup the connection information */ - coresight_release_platform_data(NULL, dev, pdata); + coresight_release_platform_data(dev, pdata); return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(coresight_get_platform_data); diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index fd896ac07942ec0a4c7acff9f32421352c1efef2..1ea882dffd703b2873e41b4ce0c2564d2ce9bbad 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -239,8 +239,7 @@ static inline void *coresight_get_uci_data_from_amba(const struct amba_id *table return NULL; }
-void coresight_release_platform_data(struct coresight_device *csdev, - struct device *dev, +void coresight_release_platform_data(struct device *dev, struct coresight_platform_data *pdata); struct coresight_device * coresight_find_csdev_by_fwnode(struct fwnode_handle *r_fwnode);
Move the sink device type checks into etm_perf_add_symlink_sink(), and return -EOPNOTSUPP for unsupported devices. This simplifies the registration flow to invoke etm_perf_add_symlink_sink() unconditionally.
Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 23 +++++++++-------------- drivers/hwtracing/coresight/coresight-etm-perf.c | 5 ++++- 2 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index b3119832725c08a8375a3a55a11b3bda14b8a3e0..c694b4bea3d724464e97a4932ead584d59d8a69a 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1383,21 +1383,16 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) goto out_unlock; }
- if ((csdev->type == CORESIGHT_DEV_TYPE_SINK || - csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) && - sink_ops(csdev)->alloc_buffer) { - ret = etm_perf_add_symlink_sink(csdev); + ret = etm_perf_add_symlink_sink(csdev);
- if (ret) { - device_unregister(&csdev->dev); - /* - * As with the above, all resources are free'd - * explicitly via coresight_device_release() triggered - * from put_device(), which is in turn called from - * function device_unregister(). - */ - goto out_unlock; - } + /* + * As with the above, all resources are free'd explicitly via + * coresight_device_release() triggered from put_device(), which is in + * turn called from function device_unregister(). + */ + if (ret && ret != -EOPNOTSUPP) { + device_unregister(&csdev->dev); + goto out_unlock; } /* Device is now registered */ registered = true; diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 3c8a6f795094bc1da9d8b1d0fda8d58640c87489..3b2a62521396dda819011d336982e960a5bd3d0e 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -902,7 +902,10 @@ int etm_perf_add_symlink_sink(struct coresight_device *csdev)
if (csdev->type != CORESIGHT_DEV_TYPE_SINK && csdev->type != CORESIGHT_DEV_TYPE_LINKSINK) - return -EINVAL; + return -EOPNOTSUPP; + + if (!sink_ops(csdev)->alloc_buffer) + return -EOPNOTSUPP;
if (csdev->ea != NULL) return -EINVAL;
Separate the failure handling path from the successful case. Use the 'out_unlock' label only for failure handling.
Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index c694b4bea3d724464e97a4932ead584d59d8a69a..955af43010446803030973c72f07315492b2fcf3 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1398,17 +1398,22 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) registered = true;
ret = coresight_create_conns_sysfs_group(csdev); - if (!ret) - ret = coresight_fixup_orphan_conns(csdev); + if (ret) + goto out_unlock; + + ret = coresight_fixup_orphan_conns(csdev); + if (ret) + goto out_unlock; + + mutex_unlock(&coresight_mutex); + + if (cti_assoc_ops && cti_assoc_ops->add) + cti_assoc_ops->add(csdev); + + return csdev;
out_unlock: mutex_unlock(&coresight_mutex); - /* Success */ - if (!ret) { - if (cti_assoc_ops && cti_assoc_ops->add) - cti_assoc_ops->add(csdev); - return csdev; - }
/* Unregister the device if needed */ if (registered) {
Unify error handling during registration:
1) Failures before device registration are handled by err_out, which releases platform data.
2) Jump to the out_unlock label on failures after device registration to unwind the flow via coresight_unregister().
The "registered" variable is no longer used, remove it.
Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 955af43010446803030973c72f07315492b2fcf3..65cf975493c86de42515845147d90497aa20c595 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1326,7 +1326,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) { int ret; struct coresight_device *csdev; - bool registered = false;
csdev = kzalloc(sizeof(*csdev), GFP_KERNEL); if (!csdev) { @@ -1380,7 +1379,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) * All resources are free'd explicitly via * coresight_device_release(), triggered from put_device(). */ - goto out_unlock; + mutex_unlock(&coresight_mutex); + goto err_out; }
ret = etm_perf_add_symlink_sink(csdev); @@ -1390,12 +1390,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) * coresight_device_release() triggered from put_device(), which is in * turn called from function device_unregister(). */ - if (ret && ret != -EOPNOTSUPP) { - device_unregister(&csdev->dev); + if (ret && ret != -EOPNOTSUPP) goto out_unlock; - } - /* Device is now registered */ - registered = true;
ret = coresight_create_conns_sysfs_group(csdev); if (ret) @@ -1415,11 +1411,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) out_unlock: mutex_unlock(&coresight_mutex);
- /* Unregister the device if needed */ - if (registered) { - coresight_unregister(csdev); - return ERR_PTR(ret); - } + coresight_unregister(csdev); + return ERR_PTR(ret);
err_out: coresight_release_platform_data(desc->dev, desc->pdata);