Re: [PATCH v3] coresight: Fix possible deadlock in coresight_panic_cb

On Tue, Sep 16, 2025 at 05:09:01PM +0100, Suzuki Kuruppassery Poulose wrote:

[...]

...

From ea78dd22cbdd97f709c5991d5bd3be97be6e137e Mon Sep 17 00:00:00 2001 From: Sean Anderson sean.anderson@linux.dev Date: Tue, 16 Sep 2025 16:03:58 +0100 Subject: [PATCH] coresight: Fix possible deadlock in coresight_panic_cb()

coresight_panic_cb() is called during a panic. It invokes bus_for_each_dev(), which then calls bus_to_subsys() and takes the 'bus_kset->list_lock'. If a panic occurs after the lock has been acquired, it can lead to a deadlock.

Instead of using a common panic notifier to iterate the bus, this commit directly registers the TMC device's panic notifier. This avoids bus iteration and effectively eliminates the race condition that could cause the deadlock.

Well, if you are going that far, why not register the notifier from coresight-core ?

I have thought this but gave up.

When register a panic's notifier, it does not provide an argument for passing a private data. So the code below uses container_of() to convert notifier block pointer to the TMC driver data, as a result, the code is specific to TMC driver.

I searched the kernel but did not find a case for passing private data for panic notifier (note, this should be only an issue for panic notifier but not for common notifiers).

We could always check the panic_ops->sync, and then register/unregister the callback. Add the notifer to the csdev. Otherwise, there is no point in keeping the panic_ops in coresight_device, it is just for the TMC device.

ETF and ETR devices both support panic, this is why I think keeping `panic_ops` callbacks can benefit code consolidation in coresight-tmc-core.c (you could see the difference between my proposed change and Sean's v3 patch).

But it is fine for me to drop `panic_ops` in coresight_device. At least, this is much better than adding lock and list in patch v2.

Thanks, Leo

...

Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks") Signed-off-by: Sean Anderson sean.anderson@linux.dev Signed-off-by: Leo Yan leo.yan@arm.com

---

drivers/hwtracing/coresight/coresight-core.c | 42 ------------------- .../hwtracing/coresight/coresight-tmc-core.c | 26 ++++++++++++ drivers/hwtracing/coresight/coresight-tmc.h | 2 + 3 files changed, 28 insertions(+), 42 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 3267192f0c1c..cb0cc8d77056 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -21,7 +21,6 @@ #include <linux/property.h> #include <linux/delay.h> #include <linux/pm_runtime.h> -#include <linux/panic_notifier.h> #include "coresight-etm-perf.h" #include "coresight-priv.h" @@ -1566,36 +1565,6 @@ const struct bus_type coresight_bustype = { .name = "coresight", }; -static int coresight_panic_sync(struct device *dev, void *data) -{

• int mode;
• struct coresight_device *csdev;
• /* Run through panic sync handlers for all enabled devices */
• csdev = container_of(dev, struct coresight_device, dev);
• mode = coresight_get_mode(csdev);
• if ((mode == CS_MODE_SYSFS) || (mode == CS_MODE_PERF)) {

• if (panic_ops(csdev))
  

• 	panic_ops(csdev)->sync(csdev);
  

• }
• return 0;

-}

-static int coresight_panic_cb(struct notifier_block *self,

• 	       unsigned long v, void *p)
  

-{

• bus_for_each_dev(&coresight_bustype, NULL, NULL,

• 		 coresight_panic_sync);
  

• return 0;

-}

-static struct notifier_block coresight_notifier = {

• .notifier_call = coresight_panic_cb,

-};

• static int __init coresight_init(void) { int ret;

@@ -1608,20 +1577,11 @@ static int __init coresight_init(void) if (ret) goto exit_bus_unregister;

• /* Register function to be called for panic */
• ret = atomic_notifier_chain_register(&panic_notifier_list,

• 			     &coresight_notifier);
  

• if (ret)

• goto exit_perf;
  

• /* initialise the coresight syscfg API */ ret = cscfg_init(); if (!ret) return 0;
• atomic_notifier_chain_unregister(&panic_notifier_list,

• 			     &coresight_notifier);
  

-exit_perf: etm_perf_exit(); exit_bus_unregister: bus_unregister(&coresight_bustype); @@ -1631,8 +1591,6 @@ static int __init coresight_init(void) static void __exit coresight_exit(void) { cscfg_exit();

• atomic_notifier_chain_unregister(&panic_notifier_list,

• 			     &coresight_notifier);
  

  etm_perf_exit(); bus_unregister(&coresight_bustype); }

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 36599c431be6..108ed9daf56d 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -21,6 +21,7 @@ #include <linux/slab.h> #include <linux/dma-mapping.h> #include <linux/spinlock.h> +#include <linux/panic_notifier.h> #include <linux/pm_runtime.h> #include <linux/of.h> #include <linux/of_address.h> @@ -769,6 +770,21 @@ static void register_crash_dev_interface(struct tmc_drvdata *drvdata, "Valid crash tracedata found\n"); } +static int tmc_panic_cb(struct notifier_block *nb, unsigned long v, void *p) +{

• struct tmc_drvdata *drvdata = container_of(nb, struct tmc_drvdata,

• 				   panic_notifier);
  

• struct coresight_device *csdev = drvdata->csdev;
• if (coresight_get_mode(csdev) == CS_MODE_DISABLED)

• return 0;
  

• if (panic_ops(csdev))

• panic_ops(csdev)->sync(csdev);
  

• return 0;

+}

• static int __tmc_probe(struct device *dev, struct resource *res) { int ret = 0;

@@ -885,6 +901,12 @@ static int __tmc_probe(struct device *dev, struct resource *res) goto out; }

• if (panic_ops(drvdata->csdev)) {

• drvdata->panic_notifier.notifier_call = tmc_panic_cb;
  

• atomic_notifier_chain_register(&panic_notifier_list,
  

• 			       &drvdata->panic_notifier);
  

• }
• out: if (is_tmc_crashdata_valid(drvdata) && !tmc_prepare_crashdata(drvdata))

@@ -929,6 +951,10 @@ static void __tmc_remove(struct device *dev) { struct tmc_drvdata *drvdata = dev_get_drvdata(dev);

• if (panic_ops(drvdata->csdev))

• atomic_notifier_chain_unregister(&panic_notifier_list,
  

• 				 &drvdata->panic_notifier);
  

• /*
  • Since misc_open() holds a refcount on the f_ops, which is
  • etb fops in this case, device is there until last file

diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index cbb4ba439158..873c5427673c 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -243,6 +243,7 @@ struct tmc_resrv_buf {

• (after crash) by default.
  

• @crash_mdata: Reserved memory for storing tmc crash metadata.

•  Used by ETR/ETF.
  

• • @panic_notifier: Notifier used to clean up during a panic
  */ struct tmc_drvdata { struct clk *atclk;

@@ -273,6 +274,7 @@ struct tmc_drvdata { struct etr_buf *perf_buf; struct tmc_resrv_buf resrv_buf; struct tmc_resrv_buf crash_mdata;

• struct notifier_block panic_notifier; }; struct etr_buf_operations {