Re: [PATCH v4 15/15] coresight: Move CPU hotplug callbacks to core layer

[James Clark <james.clark@linaro.org>]

On 19/11/2025 2:48 pm, Leo Yan wrote:
> On Mon, Nov 10, 2025 at 12:20:06PM +0000, James Clark wrote:
> 
> [...]
> 
>>> +static int coresight_dying_cpu(unsigned int cpu)
>>> +{
>>> +	struct coresight_device *source = per_cpu(csdev_source, cpu);
>>> +	struct coresight_path *path;
>>> +
>>> +	if (!source || !source->path)
>>> +		return 0;
>>> +
>>> +	/*
>>> +	 * The perf event layer will disable PMU events in the CPU hotplug.
>>> +	 * CoreSight driver should never handle the CS_MODE_PERF case.
>>> +	 */
>>> +	if (coresight_get_mode(source) != CS_MODE_SYSFS)
>>> +		return 0;
>>> +
>>> +	/*
>>> +	 * Save 'source->path' here, as it will be cleared in
>>> +	 * coresight_disable_source().
>>> +	 */
>>> +	path = source->path;
>>> +
>>> +	coresight_disable_source(source, NULL);
>>> +	coresight_disable_path(path);
>>> +	return 0;
>>
>> If the user is expected to re-enable and this new state is visible, don't
>> you need to use the regular coresight_disable_sysfs() function? It calls
>> coresight_disable_source_sysfs() which updates a refcount.
> 
> Good point!  We only need to maintain refcnt for system tracers (e.g.,
> STM).  The per-CPU tracer is only binary states (on or off), we can
> simply use "mode" to track state and no need refcnt.
> 
> I will use a separate patch to refactor refcnt.
> 

If you remove the refcount then wouldn't it break scripts that make 
multiple enable calls? I thought the same logic for the system sources 
applies to CPU sources.

TBH I don't really know what problem the refcount and multiple balanced 
enable/disable calls solves. But even if we decide it's not needed, it's 
too late to change now.

>> Maybe you didn't do it because it has a mutex in it? It would be easier to
>> change that to a spinlock or take the mutex in a wrapper function and share
>> the core disabling code with the hotplug path.
> 
> We have used cpus_read_lock() to avoid race between sysfs knobs and CPU
> hotplug callbacks, we should not acquire mutex or spinlock in this
> case.  For this reason, I would keep to call low level functions.
> 
> Thanks,
> Leo