Re: [PATCH V2 2/6] powerpc/perf: Reject duplicate HTM target reservations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Rejecting duplicate HTM target reservations with `-EBUSY` breaks `perf record -a`.
--

--- Patch [2]: [PATCH V2 2/6] powerpc/perf: Reject duplicate HTM target reservations ---
commit ab5064c1a585bcfce6e8560bc79aa814badd08ff
Author: Athira Rajeev <atrajeev@linux.ibm.com>

powerpc/perf: Reject duplicate HTM target reservations

This commit tracks reserved HTM targets globally to prevent concurrent
event initialization races. It rejects duplicate reservations for the same
target in htm_event_init() by returning -EBUSY.

> diff --git a/arch/powerpc/perf/htm-perf.c b/arch/powerpc/perf/htm-perf.c
> index 5d3aa9c7fd8d1..a188a40287044 100644
> --- a/arch/powerpc/perf/htm-perf.c
> +++ b/arch/powerpc/perf/htm-perf.c
[ ... ]
> @@ -188,11 +203,30 @@ static int htm_event_init(struct perf_event *event)
>  	}
>  
>  	/* Allocate per-event private state; freed via event->destroy */
> -	event->pmu_private = kzalloc(sizeof(struct htm_target_id), GFP_KERNEL);
> -	if (!event->pmu_private)
> +	target = kzalloc(sizeof(*target), GFP_KERNEL);
> +	if (!target)
>  		return -ENOMEM;
>  
> -	((struct htm_target_id *)event->pmu_private)->cfg = cfg;
> +	target->cfg = cfg;
> +	target->tracing_active = HTM_TRACING_INACTIVE;
> +	INIT_LIST_HEAD(&target->list);
> +
> +	mutex_lock(&htm_targets_lock);
> +	list_for_each_entry(tmp, &htm_active_targets_list, list) {
> +		if (tmp->cfg.htmtype == cfg.htmtype &&
> +			tmp->cfg.nodeindex == cfg.nodeindex &&
> +			tmp->cfg.nodalchipindex == cfg.nodalchipindex &&
> +			tmp->cfg.coreindexonchip == cfg.coreindexonchip) {
> +			mutex_unlock(&htm_targets_lock);
> +			kfree(target);
> +			return -EBUSY;

[Severity: High]
Does returning -EBUSY here break "perf record -a" for this PMU?

Since the commit message notes that a cpumask was deliberately omitted,
the perf tool will attempt to open the event on every online CPU sequentially
when running "perf record -a". The first open (e.g., for CPU 0) will succeed,
but the very next open (for CPU 1) will hit this path and return -EBUSY.

Because the perf tool treats -EBUSY as a fatal error, won't this abort the
entire profiling session instead of just skipping the duplicate target?

> +		}
> +	}
> +
> +	list_add_tail(&target->list, &htm_active_targets_list);
> +	mutex_unlock(&htm_targets_lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720104447.11843-1-atrajeev@linux.ibm.com?part=2