Re: [PATCH v2 5/9] perf/cxl: Keep the overflow interrupt pinned to the managed CPU

Dave Jiang <[email protected]> Thu, 30 Jul 2026 15:19:26 -0700
Newsgroups org.kernel.vger.linux-cxl,org.kernel.vger.linux-perf-users
Message-ID <[email protected]>

On 7/30/26 4:46 AM, Robin Murphy wrote:
> On 29/07/2026 3:55 pm, Dave Jiang wrote:
>> The PMU pins its overflow interrupt to info->on_cpu in the hotplug
>> online/offline callbacks, but requests it with only IRQF_SHARED |
>> IRQF_NO_THREAD. Without IRQF_NOBALANCING, irqbalance or a userspace
>> smp_affinity write can move the interrupt to another CPU. cxl_pmu_irq()
>> then runs cxl_pmu_read() there, doing local64_cmpxchg()/local64_add() on
>> hwc->prev_count and event->count concurrently with the managing CPU;
>> local64_t is only atomic against same-CPU access, so counts get
>> corrupted.
>>
>> Add IRQF_NOBALANCING so the pinning done in the hotplug callbacks holds,
>> matching other uncore-style PMU drivers.
>>
>> Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
>> Reported-by: [email protected]
>> Closes: https://sashiko.dev/#/patchset/[email protected]?part=1
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Dave Jiang <[email protected]>
>> ---
>>   drivers/perf/cxl_pmu.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
>> index d1e810601e36..8b89db8f4d68 100644
>> --- a/drivers/perf/cxl_pmu.c
>> +++ b/drivers/perf/cxl_pmu.c
>> @@ -888,7 +888,8 @@ static int cxl_pmu_probe(struct device *dev)
>>       if (!irq_name)
>>           return -ENOMEM;
>>   -    rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_NO_THREAD,
>> +    rc = devm_request_irq(dev, irq, cxl_pmu_irq,
>> +                  IRQF_SHARED | IRQF_NO_THREAD | IRQF_NOBALANCING,
> 
> Bah, sorry, now I see I misspoke just now on the other patch - PMUs really _shouldn't_ permit shared IRQs, but this one does :(
> 
> Thus it's all well and good to prevent userspace changing affinity, but it doesn't help _all_ that much if other drivers can still legitimately change it behind our backs...

I'm also adding NOBALANCING on the CXL side. The interrupts shouldn't be that frequent and are not used for I/O. So it's ok if we block balancing all around if the PMU needs it. And as Jonathan mentioned, dropping SHARED.
> 
> Thanks,
> Robin.
> 
>>                     irq_name, info);
>>       if (rc)
>>           return rc;
>