Re: [PATCH v6 02/19] coresight: Set per-CPU source pointer

[Leo Yan <leo.yan@arm.com>]

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