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

On 7/30/26 11:55 AM, Jonathan Cameron wrote:
> On Thu, 30 Jul 2026 12:46:24 +0100
> Robin Murphy <[email protected]> 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...
> 
> We do in theory know what drivers might do that (and today I don't think
> any but this one do).  In practice though I think two instances of this
> on same device could foul things up.
> 
> As per other suggestion, Dave would you mind just dropping shared support
> for now?  Can revisit if anyone built that insane hardware.

Yup I'll do that.