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

[Athira Rajeev <atrajeev@linux.ibm.com>]

> On 20 Jul 2026, at 4:25 PM, sashiko-bot@kernel.org wrote:
> 
> 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?

The HTM target event are expected to be opened/pinned on a CPU
The documentation also updates that. 
Returning -EBUSY from htm_event_init() for a duplicate open is
intentional and correct.  A user who mistakenly opens the same HTM
target twice (or runs perf record without cpu=N, causing every online
CPU to attempt an open of the same target) receives a clear "PMU
counters are busy" error from the perf tool, directing them to add the
required cpu=N qualifier.  Opening the same HTM node/chip/core target
from multiple CPUs simultaneously has no meaningful purpose: HTM
hardware tracing operates on the target itself, not on the CPU that
issued the hcall.

This reasoning will be updated in V3

Thanks
Athira
> 

>> + }
>> + }
>> +
>> + 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