On Mon, Mar 16, 2026 at 05:49:47PM +0000, Suzuki K Poulose wrote:
On 16/03/2026 14:38, Leo Yan wrote:
On Fri, Mar 13, 2026 at 11:18:20AM +0000, Suzuki K Poulose wrote:
[...]
static struct coresight_device *coresight_get_source(struct coresight_path *path) { struct coresight_device *csdev; @@ -1401,6 +1452,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) mutex_unlock(&coresight_mutex);
- coresight_set_percpu_source(csdev);
- if (cti_assoc_ops && cti_assoc_ops->add) cti_assoc_ops->add(csdev);
@@ -1427,6 +1480,7 @@ void coresight_unregister(struct coresight_device *csdev) if (cti_assoc_ops && cti_assoc_ops->remove) cti_assoc_ops->remove(csdev);
- coresight_clear_percpu_source(csdev);
Should these be done with the mutex lock held ?
If so, we will create a locking chain:
coresight_mutex -> cpus_read_lock()
Afterwards in patch 18, it uses cpus_read_lock() to protect sysfs knobs, a reversed locking chain will be established:
cpus_read_lock() -> coresight_mutex
LOCKDEP will complain for possible deadlock. This is why this patch avoids to acquire mutex when set / clear per CPU sources.
The question is, what prevents two different CPUs trying to modify the "per_cpu_source" data structure when the CPU is not online.
I am struggling to establish a flow for this scenario.
Each CPU has a unique per-CPU source, and the pointer is only modified during device probe or remove. Both paths are protected by the device lock (I confirmed that __device_attach() and __device_release_driver() are protected by device_lock(dev)). Therefore concurrent updates to the same source pointer should not occur.
One possible corner case is the ETMv4 delayed probe (etm4_probe_cpu()). Since the probe is deferred and coresight_register() is invoked when a CPU is hotplugged, it could race with ETMv4 module unloading, i.e. between etm4_probe_cpu() and etm4_remove_dev().
I discussed with James for another option before: we always access per-cpu source pointer _locally_. For example, we could set the pointer in etm4_init_arch_data() and clear it in clear_etmdrvdata(). These paths are protected by the CPU lock and in SMP call, thus coresight_set_percpu_source() does not need acquire any lock.
The downside is that we set the per-cpu source pointer in ETM driver but cannot use coresight_register/coresight_unregister as common place to manage the pointer.
Thanks, Leo