mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/8] coresight: Fix device registration and unregistration
@ 2026-01-26 13:52 Leo Yan
  2026-01-26 13:52 ` [PATCH v2 1/8] coresight: Fix memory leak in coresight_alloc_device_name() Leo Yan
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Leo Yan @ 2026-01-26 13:52 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel, Leo Yan

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 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.com/

To: Suzuki K Poulose <suzuki.poulose@arm.com>
To: Mike Leach <mike.leach@linaro.org>
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-core.c     | 105 +++++++++++------------
 drivers/hwtracing/coresight/coresight-etm-perf.c |   5 +-
 drivers/hwtracing/coresight/coresight-platform.c |   2 +-
 drivers/hwtracing/coresight/coresight-priv.h     |   3 +-
 4 files changed, 56 insertions(+), 59 deletions(-)
---
base-commit: eebe8dbd8630f51cf70b1f68a440cd3d7f7a914d
change-id: 20260120-arm_coresight_refactor_dev_register-f16c069db41d

Best regards,
-- 
Leo Yan <leo.yan@arm.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/8] coresight: Fix memory leak in coresight_alloc_device_name()
  2026-01-26 13:52 [PATCH v2 0/8] coresight: Fix device registration and unregistration Leo Yan
@ 2026-01-26 13:52 ` Leo Yan
  2026-01-26 15:48   ` Suzuki K Poulose
  2026-01-26 13:52 ` [PATCH v2 2/8] coresight: Get parent device reference after sink ID map allocation Leo Yan
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Leo Yan @ 2026-01-26 13:52 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel, Leo Yan

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

Change to use devm_krealloc_array() for allocation fwnode_list so that
the device model can automatically release the memory when the module
is unloaded.

Fixes: 0f5f9b6ba9e1 ("coresight: Use platform agnostic names")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index c660cf8adb1c7cafff8f85e501f056e4e151e372..ed304f292b7b02e89c59053d43f88fbf663cc993 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1546,7 +1546,7 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
 	if (idx < 0) {
 		/* Make space for the new entry */
 		idx = dict->nr_idx;
-		list = krealloc_array(dict->fwnode_list,
+		list = devm_krealloc_array(dev, dict->fwnode_list,
 				      idx + 1, sizeof(*dict->fwnode_list),
 				      GFP_KERNEL);
 		if (ZERO_OR_NULL_PTR(list)) {

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 2/8] coresight: Get parent device reference after sink ID map allocation
  2026-01-26 13:52 [PATCH v2 0/8] coresight: Fix device registration and unregistration Leo Yan
  2026-01-26 13:52 ` [PATCH v2 1/8] coresight: Fix memory leak in coresight_alloc_device_name() Leo Yan
@ 2026-01-26 13:52 ` Leo Yan
  2026-01-26 13:52 ` [PATCH v2 3/8] coresight: Protect unregistration with mutex Leo Yan
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Leo Yan @ 2026-01-26 13:52 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel, Leo Yan

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 ed304f292b7b02e89c59053d43f88fbf663cc993..6f21925a4913330a27db5d5836f34f13382339e7 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1346,12 +1346,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) {
@@ -1363,6 +1357,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

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 3/8] coresight: Protect unregistration with mutex
  2026-01-26 13:52 [PATCH v2 0/8] coresight: Fix device registration and unregistration Leo Yan
  2026-01-26 13:52 ` [PATCH v2 1/8] coresight: Fix memory leak in coresight_alloc_device_name() Leo Yan
  2026-01-26 13:52 ` [PATCH v2 2/8] coresight: Get parent device reference after sink ID map allocation Leo Yan
@ 2026-01-26 13:52 ` Leo Yan
  2026-01-26 13:52 ` [PATCH v2 4/8] coresight: Refactor output connection sysfs link cleanup Leo Yan
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Leo Yan @ 2026-01-26 13:52 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel, Leo Yan

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 6f21925a4913330a27db5d5836f34f13382339e7..b992ef5de8ddbd535e937c616bd7ab663f097f76 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1429,14 +1429,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);
 

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 4/8] coresight: Refactor output connection sysfs link cleanup
  2026-01-26 13:52 [PATCH v2 0/8] coresight: Fix device registration and unregistration Leo Yan
                   ` (2 preceding siblings ...)
  2026-01-26 13:52 ` [PATCH v2 3/8] coresight: Protect unregistration with mutex Leo Yan
@ 2026-01-26 13:52 ` Leo Yan
  2026-01-26 13:52 ` [PATCH v2 5/8] coresight: Refactor sysfs connection group cleanup Leo Yan
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Leo Yan @ 2026-01-26 13:52 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel, Leo Yan

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 b992ef5de8ddbd535e937c616bd7ab663f097f76..b4b87b3e0a608a1df19d81ba796d4db7d01290f5 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1149,7 +1149,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;
@@ -1159,10 +1158,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) {
@@ -1173,6 +1168,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;
@@ -1303,9 +1305,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
@@ -1421,7 +1420,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);
 }

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 5/8] coresight: Refactor sysfs connection group cleanup
  2026-01-26 13:52 [PATCH v2 0/8] coresight: Fix device registration and unregistration Leo Yan
                   ` (3 preceding siblings ...)
  2026-01-26 13:52 ` [PATCH v2 4/8] coresight: Refactor output connection sysfs link cleanup Leo Yan
@ 2026-01-26 13:52 ` Leo Yan
  2026-01-26 13:52 ` [PATCH v2 6/8] coresight: Move sink validation into etm_perf_add_symlink_sink() Leo Yan
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Leo Yan @ 2026-01-26 13:52 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel, Leo Yan

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 b4b87b3e0a608a1df19d81ba796d4db7d01290f5..74fdaf3f445d139b493c2d3041884550931291b3 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1197,6 +1197,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);
 }
 
 /**
@@ -1297,8 +1299,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;
@@ -1316,8 +1317,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)
@@ -1420,7 +1419,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);
@@ -1435,7 +1434,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);

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 6/8] coresight: Move sink validation into etm_perf_add_symlink_sink()
  2026-01-26 13:52 [PATCH v2 0/8] coresight: Fix device registration and unregistration Leo Yan
                   ` (4 preceding siblings ...)
  2026-01-26 13:52 ` [PATCH v2 5/8] coresight: Refactor sysfs connection group cleanup Leo Yan
@ 2026-01-26 13:52 ` Leo Yan
  2026-01-26 13:52 ` [PATCH v2 7/8] coresight: Do not mix success path with failure handling Leo Yan
  2026-01-26 13:52 ` [PATCH v2 8/8] coresight: Unify error handling in coresight_register() Leo Yan
  7 siblings, 0 replies; 11+ messages in thread
From: Leo Yan @ 2026-01-26 13:52 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel, Leo Yan

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 74fdaf3f445d139b493c2d3041884550931291b3..5c29f78de7f115d61f550dea62b6538940ec9182 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1380,21 +1380,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;

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 7/8] coresight: Do not mix success path with failure handling
  2026-01-26 13:52 [PATCH v2 0/8] coresight: Fix device registration and unregistration Leo Yan
                   ` (5 preceding siblings ...)
  2026-01-26 13:52 ` [PATCH v2 6/8] coresight: Move sink validation into etm_perf_add_symlink_sink() Leo Yan
@ 2026-01-26 13:52 ` Leo Yan
  2026-01-26 13:52 ` [PATCH v2 8/8] coresight: Unify error handling in coresight_register() Leo Yan
  7 siblings, 0 replies; 11+ messages in thread
From: Leo Yan @ 2026-01-26 13:52 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel, Leo Yan

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 5c29f78de7f115d61f550dea62b6538940ec9182..c2b5a6efd1c01b0fbfbc289cf27232ccc731ba34 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1395,17 +1395,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) {

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 8/8] coresight: Unify error handling in coresight_register()
  2026-01-26 13:52 [PATCH v2 0/8] coresight: Fix device registration and unregistration Leo Yan
                   ` (6 preceding siblings ...)
  2026-01-26 13:52 ` [PATCH v2 7/8] coresight: Do not mix success path with failure handling Leo Yan
@ 2026-01-26 13:52 ` Leo Yan
  7 siblings, 0 replies; 11+ messages in thread
From: Leo Yan @ 2026-01-26 13:52 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel, Leo Yan

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 c2b5a6efd1c01b0fbfbc289cf27232ccc731ba34..0aa64ee36dd06f86d3f6be1cab3067bf91467686 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1323,7 +1323,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) {
@@ -1377,7 +1376,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);
@@ -1387,12 +1387,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)
@@ -1412,11 +1408,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);

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/8] coresight: Fix memory leak in coresight_alloc_device_name()
  2026-01-26 13:52 ` [PATCH v2 1/8] coresight: Fix memory leak in coresight_alloc_device_name() Leo Yan
@ 2026-01-26 15:48   ` Suzuki K Poulose
  2026-01-26 16:18     ` Leo Yan
  0 siblings, 1 reply; 11+ messages in thread
From: Suzuki K Poulose @ 2026-01-26 15:48 UTC (permalink / raw)
  To: Leo Yan, Mike Leach, James Clark, Alexander Shishkin,
	Greg Kroah-Hartman, Mathieu Poirier, Mao Jinlong
  Cc: coresight, linux-arm-kernel, linux-kernel

Hi Leo

On 26/01/2026 13:52, Leo Yan wrote:
> 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
> 
> Change to use devm_krealloc_array() for allocation fwnode_list so that
> the device model can automatically release the memory when the module
> is unloaded.
> 
> Fixes: 0f5f9b6ba9e1 ("coresight: Use platform agnostic names")
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index c660cf8adb1c7cafff8f85e501f056e4e151e372..ed304f292b7b02e89c59053d43f88fbf663cc993 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1546,7 +1546,7 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
>   	if (idx < 0) {
>   		/* Make space for the new entry */
>   		idx = dict->nr_idx;
> -		list = krealloc_array(dict->fwnode_list,
> +		list = devm_krealloc_array(dev, dict->fwnode_list,
>   				      idx + 1, sizeof(*dict->fwnode_list),

This is wrong. This would mean, the array gets released when the "dev" 
goes away and thats not good. This is a per-driver list, not a 
per-device list. Also, please remember that, we need to keep that node alive
when the device goes away, to make sure to allocate the same device name
when if/it comes back.  May be, you could release the  list and array, 
when the "driver" goes away.


Kind regards
Suzuki

>   				      GFP_KERNEL);
>   		if (ZERO_OR_NULL_PTR(list)) {
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/8] coresight: Fix memory leak in coresight_alloc_device_name()
  2026-01-26 15:48   ` Suzuki K Poulose
@ 2026-01-26 16:18     ` Leo Yan
  0 siblings, 0 replies; 11+ messages in thread
From: Leo Yan @ 2026-01-26 16:18 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Mike Leach, James Clark, Alexander Shishkin, Greg Kroah-Hartman,
	Mathieu Poirier, Mao Jinlong, coresight, linux-arm-kernel,
	linux-kernel

Hi Suzuki,

On Mon, Jan 26, 2026 at 03:48:28PM +0000, Suzuki K Poulose wrote:

[...]

> > @@ -1546,7 +1546,7 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
> >   	if (idx < 0) {
> >   		/* Make space for the new entry */
> >   		idx = dict->nr_idx;
> > -		list = krealloc_array(dict->fwnode_list,
> > +		list = devm_krealloc_array(dev, dict->fwnode_list,
> >   				      idx + 1, sizeof(*dict->fwnode_list),
> 
> This is wrong. This would mean, the array gets released when the "dev" goes
> away and thats not good. This is a per-driver list, not a per-device list.
> Also, please remember that, we need to keep that node alive
> when the device goes away, to make sure to allocate the same device name
> when if/it comes back.  May be, you could release the  list and array, when
> the "driver" goes away.

Indeed.  In next spin, I will to fix the leak in module exit.

Thanks for pointing this out.

Leo

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-01-26 16:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-01-26 13:52 [PATCH v2 0/8] coresight: Fix device registration and unregistration Leo Yan
2026-01-26 13:52 ` [PATCH v2 1/8] coresight: Fix memory leak in coresight_alloc_device_name() Leo Yan
2026-01-26 15:48   ` Suzuki K Poulose
2026-01-26 16:18     ` Leo Yan
2026-01-26 13:52 ` [PATCH v2 2/8] coresight: Get parent device reference after sink ID map allocation Leo Yan
2026-01-26 13:52 ` [PATCH v2 3/8] coresight: Protect unregistration with mutex Leo Yan
2026-01-26 13:52 ` [PATCH v2 4/8] coresight: Refactor output connection sysfs link cleanup Leo Yan
2026-01-26 13:52 ` [PATCH v2 5/8] coresight: Refactor sysfs connection group cleanup Leo Yan
2026-01-26 13:52 ` [PATCH v2 6/8] coresight: Move sink validation into etm_perf_add_symlink_sink() Leo Yan
2026-01-26 13:52 ` [PATCH v2 7/8] coresight: Do not mix success path with failure handling Leo Yan
2026-01-26 13:52 ` [PATCH v2 8/8] coresight: Unify error handling in coresight_register() Leo Yan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®