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

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 30 Jul 2026 12:46:24 +0100
Robin Murphy <robin.murphy@arm.com> 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: sashiko-bot@kernel.org
> > Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > ---
> >   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.

J
> 
> Thanks,
> Robin.
> 
> >   			      irq_name, info);
> >   	if (rc)
> >   		return rc;  
>