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@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)
device_unregister(&csdev->dev); } EXPORT_SYMBOL_GPL(coresight_unregister);list_del(&csdev->csdev_list);
@@ -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; };