Re: [PATCH v3] coresight: Fix scheduling while atomic in coresight_cpu_pm_notify()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling put_device() inside a raw spinlock atomic context in coresight_put_percpu_source_ref() causes a system crash on PREEMPT_RT.
- [High] Double-decrement of hardware refcounts for links/sinks when a device is unregistered while the CPU is idle.
--

--- Patch [1]: [PATCH v3] coresight: Fix scheduling while atomic in coresight_cpu_pm_notify() ---
commit cbdaaaf0779f6cae5c8595e4a2bcad24b819e017
Author: Mohamed Ayman <mohamedaymanworkspace@xxxxxxxxx>

coresight: Fix scheduling while atomic in coresight_cpu_pm_notify()

This commit modifies coresight_cpu_get_active_path() to avoid calling
coresight_get_percpu_source_ref() and coresight_put_percpu_source_ref()
during CPU PM transitions. Instead, it reads the per-cpu source pointer
directly under the coresight_dev_lock raw spinlock to prevent potential
sleeping in atomic context on PREEMPT_RT.

> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 6d65c43d574fd..713bcff53bf4f 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1850,25 +1850,15 @@ static void coresight_release_device_list(void)
>  static struct coresight_path *coresight_cpu_get_active_path(enum cs_mode mode)
>  {
>  	struct coresight_device *source;
> -	bool is_active = false;
> +	struct coresight_path *path = NULL;
>  
> -	source = coresight_get_percpu_source_ref(smp_processor_id());
> -	if (!source)
> -		return NULL;
> -
> -	if (coresight_get_mode(source) & mode)
> -		is_active = true;
> +	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
>  
> -	coresight_put_percpu_source_ref(source);

[Severity: High]
This isn't a bug introduced by this patch, but while this change fixes the
issue for the PM notifier by removing the get/put dance, does the same
"scheduling while atomic" problem still exist in
coresight_put_percpu_source_ref() for other callers?

In coresight_put_percpu_source_ref():

	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
	...
	put_device(&csdev->dev);

This unconditionally acquires coresight_dev_lock as a raw spinlock and calls
put_device(). If concurrent device unregistration occurs, this put_device()
call can drop the final kobject reference, triggering
coresight_device_release().

This calls free_percpu(), which acquires sleepable locks on PREEMPT_RT,
causing a panic. Are there plans to address this remaining put_device()
call under a raw spinlock?

> +	source = per_cpu(csdev_source, smp_processor_id());
> +	if (source && (coresight_get_mode(source) & mode))
> +		path = source->path;
>  
> -	/*
> -	 * It is expected to run in atomic context or with the CPU lock held for
> -	 * sysfs mode, so it cannot be preempted to disable the path. Here
> -	 * returns the active path pointer without concern that its state may
> -	 * change. Since the build path has taken a reference on the component,
> -	 * the path can be safely used by the caller.
> -	 */
> -	return is_active ? source->path : NULL;
> +	return path;
>  }

[Severity: High]
This is a pre-existing issue, but does this PM logic cause a permanent
double-decrement of hardware refcounts if a trace device is unregistered
while the CPU is idle?

When the CPU goes idle, coresight_cpu_pm_notify() runs CPU_PM_ENTER and calls
coresight_pm_save(path). This disables components along the active trace path
and decrements their reference counts.

If the trace driver is concurrently unbound on another CPU,
coresight_unregister() clears the csdev_source per-CPU pointer.

When the idle CPU wakes up, coresight_cpu_pm_notify(CPU_PM_EXIT) calls
coresight_cpu_get_active_path(). Because the pointer was cleared, this
returns NULL.

Consequently, coresight_pm_restore() is skipped, and the hardware refcounts
are never incremented back:

	struct coresight_path *path =
		coresight_cpu_get_active_path(CS_MODE_SYSFS | CS_MODE_PERF);
	int ret;

	ret = coresight_pm_is_needed(path); // returns 0 if path is NULL
	if (ret <= 0)
		return ret ? NOTIFY_BAD : NOTIFY_DONE; // Skips restore

When the trace session is later explicitly closed, coresight_disable_path()
executes and decrements those refcounts again.

Does this cause an underflow that permanently disables shared hardware
components for all cores?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716214155.2049564-1-mohamedaymanworkspace@xxxxxxxxx?part=1