CoreSight September 2025

coresight@lists.linaro.org

8 participants
51 discussions

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

by Leo Yan

On Mon, Sep 15, 2025 at 10:31:24AM -0400, Sean Anderson wrote: > On 9/15/25 05:58, Leo Yan wrote: > > On Fri, Sep 12, 2025 at 11:13:14AM -0400, Sean Anderson wrote: > >> coresight_panic_cb is called with interrupts disabled during panics. > >> However, bus_for_each_dev calls bus_to_subsys which takes > >> bus_kset->list_lock without disabling IRQs. This may cause a deadlock. > > > > I would rephrase it to make it clearer for anyone reading it later: > > > > coresight_panic_cb() is called during panics, which can preempt a flow > > that triggers exceptions (such as data or instruction aborts). > > I don't see what exceptions have to do with it. You can also panic > during a regular interrupt. The commit mentioned "without disabling IRQs" gives the impression that the deadlock is caused by IRQ-unsafe locking, which might mislead into thinking why the issue cannot be fixed with IRQ-safe locking. Regardless of whether IRQs are disabled, and regardless of the context (interrupt, bottom-half, or normal thread), the conditions for the deadlock are only about: (a) The bus lock has been acquired; (b) A panic is triggered to try to acquire the same lock. [...] > > When I review this patch, I recognize we can consolidate panic notifier > > in coresight-tmc-core.c, so we don't need to distribute the changes > > into ETF and ETR drivers (sorry if I misled you in my previous reply). > > And this kind of thing is why I went with the straightforward fix > initially. I do not want to bikeshed the extent that this gets removed. > IMO the whole "panic ops" stuff should be done directly with the panic > notifier, hence this patch. If you do not agree with that, then ack v2 > and send a follow up of your own to fix it how you see fit. I would fix it in one go. I agree with you that "the whole panic ops stuff should be done directly with the panic". The only difference between us is that I would keep the `panic_ops` callback. To me, this encapsulates panic callbacks into different modules, to make the code more general. Could you check if the drafted patch below looks good to you? If so, I will send out a formal patch. ---8<--- From ea78dd22cbdd97f709c5991d5bd3be97be6e137e Mon Sep 17 00:00:00 2001 From: Sean Anderson <sean.anderson(a)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. Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks") Signed-off-by: Sean Anderson <sean.anderson(a)linux.dev> Signed-off-by: Leo Yan <leo.yan(a)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 { -- 2.34.1

2 weeks, 2 days

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

by Leo Yan

On Tue, Sep 16, 2025 at 12:14:40PM -0400, Sean Anderson wrote: [...] > > Could you check if the drafted patch below looks good to you? If so, I > > As stated above I disagree with a half-hearted removal. If you want to do that, > then I will resend v2 done with an rcu list and you can make your own follow-up. It is fine to disagree, but please don't resend v2 :) We have plan to refactor locking in CoreSight driver, I will try my best to avoid adding new lock unless with a strong reason. Thanks, Leo

2 weeks, 2 days

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

by Leo Yan

On Fri, Sep 12, 2025 at 11:13:14AM -0400, Sean Anderson wrote: > coresight_panic_cb is called with interrupts disabled during panics. > However, bus_for_each_dev calls bus_to_subsys which takes > bus_kset->list_lock without disabling IRQs. This may cause a deadlock. I would rephrase it to make it clearer for anyone reading it later: coresight_panic_cb() is called during panics, which can preempt a flow that triggers exceptions (such as data or instruction aborts). However, bus_for_each_dev() calls bus_to_subsys(), which takes 'bus_kset->list_lock'. If a panic occurs after the lock has been acquired, it will cause a deadlock. > Instead of adding a panic API for coresight devices, just have coresight > drivers that need a panic callback register directly with the panic > notifier. > > Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks") > Fixes: 6dbcbcfc4496 ("coresight: tmc: Enable panic sync handling") Let us fix the bug caused by the commit 46006ceb5d02. OTOH, I think we should not touch change introduced in the commit 6dbcbcfc4496. Please see details below. > Signed-off-by: Sean Anderson <sean.anderson(a)linux.dev> > --- > > Changes in v3: > - Rewrite patch to remove the panic sync API entirely > > Changes in v2: > - Add a comment describing csdev_lock/list > - Consolidate list removal in coresight_device_release > > drivers/hwtracing/coresight/coresight-core.c | 42 ------------------- > .../hwtracing/coresight/coresight-tmc-core.c | 20 ++++++++- > .../hwtracing/coresight/coresight-tmc-etf.c | 12 +++--- > .../hwtracing/coresight/coresight-tmc-etr.c | 12 +++--- > drivers/hwtracing/coresight/coresight-tmc.h | 10 ++++- > include/linux/coresight.h | 11 +---- > 6 files changed, 38 insertions(+), 69 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index fa758cc21827..297af270bf3d 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -19,7 +19,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" > @@ -1563,36 +1562,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; > @@ -1605,20 +1574,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); > @@ -1628,8 +1588,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); > } Removing panic notifier in coresight-core.c looks good to me. > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index 88afb16bb6be..012f76dbf7d3 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -16,6 +16,7 @@ > #include <linux/fs.h> > #include <linux/miscdevice.h> > #include <linux/mutex.h> > +#include <linux/panic_notifier.h> > #include <linux/property.h> > #include <linux/uaccess.h> > #include <linux/slab.h> > @@ -834,6 +835,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_SYSMEM; > desc.ops = &tmc_etr_cs_ops; > + drvdata->panic_notifier.notifier_call = tmc_panic_sync_etr; > ret = tmc_etr_setup_caps(dev, devid, &desc.access); > if (ret) > goto out; > @@ -847,6 +849,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; > + drvdata->panic_notifier.notifier_call = tmc_panic_sync_etf; > dev_list = &etf_devs; > break; > default: > @@ -869,11 +872,18 @@ static int __tmc_probe(struct device *dev, struct resource *res) > dev->platform_data = pdata; > desc.pdata = pdata; > > + if (drvdata->panic_notifier.notifier_call) { > + ret = atomic_notifier_chain_register(&panic_notifier_list, > + &drvdata->panic_notifier); > + if (ret) > + goto out; > + } When I review this patch, I recognize we can consolidate panic notifier in coresight-tmc-core.c, so we don't need to distribute the changes into ETF and ETR drivers (sorry if I misled you in my previous reply). How about we change like below? We still can keep the panic_ops callback, and then register a common callback tmc_panic_cb(), which still checks device state and invoke device's sync callback. diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index bd783c9dcb56..f7c175b3ae4c 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,22 @@ 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; + int mode = coresight_get_mode(csdev); + + if (mode == 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; @@ -886,6 +903,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)) @@ -930,6 +953,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 {

2 weeks, 3 days

[PATCH] coresight: tnoc: add new AMBA ID to support Trace Noc V2

by Yuanfang Zhang

Add the new AMBA ID 0x001f0c00 to support Trace Noc V2 instances. Signed-off-by: Yuanfang Zhang <yuanfang.zhang(a)oss.qualcomm.com> --- drivers/hwtracing/coresight/coresight-tnoc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c index d542df46ea39314605290311f683010337bfd4bd..ff9a0a9cfe96e5f5e3077c750ea2f890cdd50d94 100644 --- a/drivers/hwtracing/coresight/coresight-tnoc.c +++ b/drivers/hwtracing/coresight/coresight-tnoc.c @@ -222,6 +222,10 @@ static struct amba_id trace_noc_ids[] = { .id = 0x000f0c00, .mask = 0x00ffff00, }, + { + .id = 0x001f0c00, + .mask = 0x00ffff00, + }, {}, }; MODULE_DEVICE_TABLE(amba, trace_noc_ids); --- base-commit: a80198ba650f50d266d7fc4a6c5262df9970f9f2 change-id: 20250814-tnoc_v2-ec86ebebf62e Best regards, -- Yuanfang Zhang <yuanfang.zhang(a)oss.qualcomm.com>

2 weeks, 3 days

[PATCH v11 0/2] coresight: Add label sysfs node support

by Mao Jinlong

Change since V10: 1. Update kernel version to 6.18 V10 link: https://lkml.org/lkml/2025/8/6/520 Change since V9: 1. Replace scnprintf with sysfs_emit. 2. Update date in ABI files. V9 link: https://lkml.org/lkml/2025/7/17/832 Change since V8: 1. Add label in all documentations of coresight components. 2. Add control of the visibility of the label sysfs attribute. V8 link: https://lkml.org/lkml/2025/7/3/985 Change since V7: 1. Update the conflict when apply to coresight next. 2. Update the Date and version in ABI file. V7 link: https://patchwork.kernel.org/project/linux-arm-kernel/patch/20250226121926.… Change since V6: 1. Update the date and version in ABI file. Change since V5: 1. Update the kernel version of ABI files. 2. Add link of different patch versions. V5 link: https://patchwork.kernel.org/project/linux-arm-msm/cover/20241210122253.319… Change since V4: 1. Add label in DT and add label sysfs node for each coresight device. V4 link: https://patchwork.kernel.org/project/linux-arm-msm/cover/20240703122340.268… Change since V3: 1. Change device-name to arm,cs-dev-name. 2. Add arm,cs-dev-name to only CTI and sources' dt-binding. V3 link: https://patchwork.kernel.org/project/linux-arm-msm/cover/20240131082628.628… Change since V2: 1. Fix the error in coresight core. drivers/hwtracing/coresight/coresight-core.c:1775:7: error: assigning to 'char *' from 'const char *' discards qualifiers 2. Fix the warning when run dtbinding check. Documentation/devicetree/bindings/arm/arm,coresight-cpu-debug.yaml: device-name: missing type definition V2 link: https://patchwork.kernel.org/project/linux-arm-msm/cover/20240115164252.265… Change since V1: 1. Change coresight-name to device name. 2. Add the device-name in coresight dt bindings. V1 link: https://patchwork.kernel.org/project/linux-arm-kernel/patch/20230208110716.… Mao Jinlong (2): dt-bindings: arm: Add label in the coresight components coresight: Add label sysfs node support .../testing/sysfs-bus-coresight-devices-cti | 6 ++ .../sysfs-bus-coresight-devices-dummy-source | 6 ++ .../testing/sysfs-bus-coresight-devices-etb10 | 6 ++ .../testing/sysfs-bus-coresight-devices-etm3x | 6 ++ .../testing/sysfs-bus-coresight-devices-etm4x | 6 ++ .../sysfs-bus-coresight-devices-funnel | 6 ++ .../testing/sysfs-bus-coresight-devices-stm | 6 ++ .../testing/sysfs-bus-coresight-devices-tmc | 6 ++ .../testing/sysfs-bus-coresight-devices-tpdm | 6 ++ .../testing/sysfs-bus-coresight-devices-trbe | 6 ++ .../bindings/arm/arm,coresight-cti.yaml | 4 ++ .../arm/arm,coresight-dummy-sink.yaml | 4 ++ .../arm/arm,coresight-dummy-source.yaml | 4 ++ .../arm/arm,coresight-dynamic-funnel.yaml | 4 ++ .../arm/arm,coresight-dynamic-replicator.yaml | 4 ++ .../bindings/arm/arm,coresight-etb10.yaml | 4 ++ .../bindings/arm/arm,coresight-etm.yaml | 4 ++ .../arm/arm,coresight-static-funnel.yaml | 4 ++ .../arm/arm,coresight-static-replicator.yaml | 4 ++ .../bindings/arm/arm,coresight-tmc.yaml | 4 ++ .../bindings/arm/arm,coresight-tpiu.yaml | 4 ++ .../bindings/arm/qcom,coresight-ctcu.yaml | 4 ++ .../arm/qcom,coresight-remote-etm.yaml | 4 ++ .../bindings/arm/qcom,coresight-tpda.yaml | 4 ++ .../bindings/arm/qcom,coresight-tpdm.yaml | 4 ++ drivers/hwtracing/coresight/coresight-sysfs.c | 71 ++++++++++++++++++- 26 files changed, 189 insertions(+), 2 deletions(-) -- 2.34.1

2 weeks, 3 days

Re: [PATCH] coresight: trbe: Use scope-based resource management in arm_trbe_alloc_buffer()

by Anshuman Khandual

On 09/09/25 5:58 PM, Markus Elfring wrote: > From: Markus Elfring <elfring(a)users.sourceforge.net> > Date: Tue, 9 Sep 2025 14:20:06 +0200 > > Scope-based resource management became supported for some > programming interfaces by contributions of Peter Zijlstra on 2023-05-26. > See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SHA ID here just needs first 12 digits not the entire string. > Introduce __cleanup() based infrastructure"). > > * Thus use the attribute “__free(kfree)”. > > * Reduce the scopes for the local variables “buf” and “pglist”. What is that required ? > > * Omit four kfree() calls accordingly. Right. The commit message should be re-written with little more context from arm_trbe_alloc_buffer(), after describing the new scope-based resource management. > > Signed-off-by: Markus Elfring <elfring(a)users.sourceforge.net> > --- > drivers/hwtracing/coresight/coresight-trbe.c | 21 ++++++++------------ > 1 file changed, 8 insertions(+), 13 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c > index 8f9bbef71f23..1b0d58bf8613 100644 > --- a/drivers/hwtracing/coresight/coresight-trbe.c > +++ b/drivers/hwtracing/coresight/coresight-trbe.c > @@ -733,8 +733,6 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, > struct perf_event *event, void **pages, > int nr_pages, bool snapshot) > { > - struct trbe_buf *buf; > - struct page **pglist; > int i; > > /* > @@ -746,32 +744,29 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, > if (nr_pages < 2) > return NULL; > > - buf = kzalloc_node(sizeof(*buf), GFP_KERNEL, trbe_alloc_node(event)); > + struct trbe_buf *buf __free(kfree) = kzalloc_node(sizeof(*buf), > + GFP_KERNEL, > + trbe_alloc_node(event)); > if (!buf) > return NULL; > > - pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); > - if (!pglist) { > - kfree(buf); > + struct page **pglist __free(kfree) = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); > + if (!pglist) > return NULL; > - } > > for (i = 0; i < nr_pages; i++) > pglist[i] = virt_to_page(pages[i]); > > buf->trbe_base = (unsigned long)vmap(pglist, nr_pages, VM_MAP, PAGE_KERNEL); > - if (!buf->trbe_base) { > - kfree(pglist); > - kfree(buf); > + if (!buf->trbe_base) > return NULL; > - } > + > buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE; > buf->trbe_write = buf->trbe_base; > buf->snapshot = snapshot; > buf->nr_pages = nr_pages; > buf->pages = pages; > - kfree(pglist); > - return buf; > + return_ptr(buf); > } > > static void arm_trbe_free_buffer(void *config) Seems like a good idea though. But please keep the local variable declaration in the current place which is bit cleaner and reduces code churn as well. diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 7a226316c48f..7babba1a9afb 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -733,8 +733,8 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, struct perf_event *event, void **pages, int nr_pages, bool snapshot) { - struct trbe_buf *buf; - struct page **pglist; + struct trbe_buf *buf __free(kfree); + struct page **pglist __free(kfree); int i; /* @@ -751,27 +751,22 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, return NULL; pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); - if (!pglist) { - kfree(buf); + if (!pglist) return NULL; - } for (i = 0; i < nr_pages; i++) pglist[i] = virt_to_page(pages[i]); buf->trbe_base = (unsigned long)vmap(pglist, nr_pages, VM_MAP, PAGE_KERNEL); - if (!buf->trbe_base) { - kfree(pglist); - kfree(buf); + if (!buf->trbe_base) return NULL; - } + buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE; buf->trbe_write = buf->trbe_base; buf->snapshot = snapshot; buf->nr_pages = nr_pages; buf->pages = pages; - kfree(pglist); - return buf; + return_ptr(buf); } static void arm_trbe_free_buffer(void *config)

2 weeks, 6 days

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

by Leo Yan

Hi Sean, On Thu, Sep 11, 2025 at 11:33:15AM -0400, Sean Anderson wrote: > coresight_panic_cb is called with interrupts disabled during panics. > However, bus_for_each_dev calls bus_to_subsys which takes > bus_kset->list_lock without disabling IRQs. This will cause a deadlock > if a panic occurs while one of the other coresight functions that uses > bus_for_each_dev is running. The decription is a bit misleading. Even when IRQ is disabled, if an exception happens, a CPU still can be trapped for handling kernel panic. > Maintain a separate list of coresight devices to access during a panic. Rather than maintaining a separate list and introducing a new spinlock, I would argue if we can simply register panic notifier in TMC ETR and ETF drviers (see tmc_panic_sync_etr() and tmc_panic_sync_etf()). If there is no dependency between CoreSight modules in panic sync flow, it is not necessary to maintain list (and lock) for these modules. I have not involved in panic patches before, so I would like to know the maintainers' opinion. Thanks, Leo

2 weeks, 6 days

[PATCH v1 0/5] coresight: Fix device registration and unregistration

by Leo Yan

This series is to fix device registration and unregistration. The first patch addresses the resource is not released properly for a failure case during a device registration. The second patch is to use mutex to protect unregistration flow. The last three patches are for refactoring. Patch 03 explicitly uses the parent device handler. Patch 04 separates the success and failure flows for code readable and easier maintenance. Patch 05 improves the error handling by invoking specific functions for resource cleanup. Leo Yan (5): coresight: Correct sink ID map allocation failure handling coresight: Protect unregistration with mutex coresight: Explicitly use the parent device handler coresight: Separate failure and success flows coresight: Refine error handling for device registration drivers/hwtracing/coresight/coresight-core.c | 67 +++++++++++--------- 1 file changed, 37 insertions(+), 30 deletions(-) -- 2.34.1

3 weeks

[PATCH v3 0/3] Coresight TMC-ETR some bugfixes and cleanups

by Junhao He

This patchset builds upon Yicong's previous patches [1]. Introducing fix two race issues found by using TMC-ETR and CATU, Two cleanups found when debugging the issues. [1] https://lore.kernel.org/linux-arm-kernel/20241202092419.11777-1-yangyicong@… --- Changes in v3: - Patches 1: Additional comment for tmc_drvdata::etr_mode. Update comment for tmc_drvdata::reading with Jonathan's Tag. - Patches 2: Replace scoped_guard with guard with Jonathan's Tag. - Patches 2: Fix spinlock to raw_spinlock, and refactor this code based on Leo's suggested solution. - Patches 3: change the size's type to ssize_t and use max_t to simplify the code with Leo's Tag. Link: https://lore.kernel.org/linux-arm-kernel/20250620075412.952934-1-hejunhao3@… Changes in v2: - Updated the commit of patch2. - Rebase to v6.16-rc1 Junhao He (1): coresight: tmc: refactor the tmc-etr mode setting to avoid race conditions Yicong Yang (2): coresight: tmc: Add missing doc including reading and etr_mode of struct tmc_drvdata coresight: tmc: Decouple the perf buffer allocation from sysfs mode .../hwtracing/coresight/coresight-tmc-etr.c | 110 ++++++++---------- drivers/hwtracing/coresight/coresight-tmc.h | 2 + 2 files changed, 53 insertions(+), 59 deletions(-) -- 2.33.0

3 weeks

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

by Suzuki K Poulose

Hi Sean On 28/08/2025 19:17, Sean Anderson wrote: > coresight_panic_cb is called with interrupts disabled during panics. > However, bus_for_each_dev calls bus_to_subsys which takes > bus_kset->list_lock without disabling IRQs. This will cause a deadlock > if a panic occurs while one of the other coresight functions that uses > bus_for_each_dev is running. > > Maintain a separate list of coresight devices to access during a panic. Thanks for the patch. I have a minor comment. > > Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks") > Signed-off-by: Sean Anderson <sean.anderson(a)linux.dev> > --- > > drivers/hwtracing/coresight/coresight-core.c | 40 ++++++++++---------- > include/linux/coresight.h | 1 + > 2 files changed, 22 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index fa758cc21827..1f1bf0e2bf92 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -1315,6 +1315,9 @@ void coresight_release_platform_data(struct coresight_device *csdev, > coresight_remove_conns_sysfs_group(csdev); > } > > +static DEFINE_SPINLOCK(csdev_lock); > +static LIST_HEAD(csdev_list); May be add a comment here to mention why we maintain this list ? > + > struct coresight_device *coresight_register(struct coresight_desc *desc) > { > int ret; > @@ -1374,11 +1377,16 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > goto out_unlock; > } > > + scoped_guard(spinlock_irq, &csdev_lock) > + list_add(&csdev->csdev_list, &csdev_list); > + > if (csdev->type == CORESIGHT_DEV_TYPE_SINK || > csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) { > ret = etm_perf_add_symlink_sink(csdev); > > if (ret) { > + scoped_guard(spinlock_irq, &csdev_lock) > + list_del(&csdev->csdev_list); Could this be moved to coresight_device_release(), which will be called when the device gets unregistered ? That way, you don't need it here and in coresight_unregister() too. Rest looks good to me Suzuki > device_unregister(&csdev->dev); > /* > * As with the above, all resources are free'd > @@ -1427,6 +1435,8 @@ void coresight_unregister(struct coresight_device *csdev) > coresight_remove_conns(csdev); > coresight_clear_default_sink(csdev); > coresight_release_platform_data(csdev, csdev->dev.parent, csdev->pdata); > + scoped_guard(spinlock_irq, &csdev_lock) > + list_del(&csdev->csdev_list); > device_unregister(&csdev->dev); > } > EXPORT_SYMBOL_GPL(coresight_unregister); > @@ -1563,28 +1573,20 @@ 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); > + struct coresight_device *csdev; > + > + guard(spinlock)(&csdev_lock); > + list_for_each_entry(csdev, &csdev_list, csdev_list) { > + /* Run through panic sync handlers for all enabled devices */ > + int mode = coresight_get_mode(csdev); > + > + if ((mode == CS_MODE_SYSFS || mode == CS_MODE_PERF) && > + panic_ops(csdev)) > + panic_ops(csdev)->sync(csdev); > + } > > return 0; > } > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 4ac65c68bbf4..a5e62ebd03b5 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -302,6 +302,7 @@ struct coresight_device { > /* system configuration and feature lists */ > struct list_head feature_csdev_list; > struct list_head config_csdev_list; > + struct list_head csdev_list; > raw_spinlock_t cscfg_csdev_lock; > void *active_cscfg_ctxt; > };

3 weeks, 1 day

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight September 2025