Re: [PATCH v2 8/9] perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after unregister

On Thu, 30 Jul 2026 12:38:28 +0100
Robin Murphy <[email protected]> wrote:

> On 29/07/2026 3:55 pm, Dave Jiang wrote:
> > On device removal the devm actions unwind LIFO, so cxl_pmu_perf_unregister()
> > runs first and perf_pmu_unregister() frees info->pmu.dev (device_del() +
> > put_device() -> kfree()). The overflow IRQ (freed last) and the CPU-hotplug
> > instance (removed next) are still live at that point, and both
> > cxl_pmu_irq() and cxl_pmu_offline_cpu() log via dev_dbg()/dev_err() on
> > info->pmu.dev, dereferencing freed memory. The shared IRQ can be entered
> > for a co-function on the same MSI vector, and a CPU can go offline in the
> > window before the hotplug instance is removed.
> > 
> > Log through info->pmu.parent instead, the cxl_pmu device passed to probe,
> > which is devm-managed and outlives every teardown action.
> > 
> > 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 | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> > index 2e817a52ff1e..f42238b2b6b0 100644
> > --- a/drivers/perf/cxl_pmu.c
> > +++ b/drivers/perf/cxl_pmu.c
> > @@ -803,7 +803,7 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
> >   		struct perf_event *event = info->hw_events[i];
> >   
> >   		if (!event) {
> > -			dev_dbg(info->pmu.dev,
> > +			dev_dbg(info->pmu.parent,
> >   				"overflow but on non enabled counter %d\n", i);
> >   			continue;  
> 
> This seems dubious - we don't permit sharing the IRQ, and all events 
> must have been stopped and descheduled to allow the PMU to be removed in 
> the first place, so how would an overflow interrupt happen?

Hmm. Today the driver itself does permit sharing.  Which is awkward given
need for the interrupts not to get migrated to different CPUs which
I guess might happen if we get a race with driver bind and hotplug events.
I may well be missing other reasons sharing is bad, but that one seems
like enough to rule it out.

Perhaps the fix for now is remove the IRQF_SHARED flag.   Clear no one
was using the driver yet with real hardware given some of the fixes
in this series so we aren't going to regress anyone.

Hopefully no one actually thinks a device that puts PMUs on shared interrupts
is a good idea and no host running CXL runs out and has to force the PCIe
stuff to collapse them to a smaller set of vectors.

> 
> >   		}
> > @@ -966,7 +966,7 @@ static int cxl_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> >   	info->on_cpu = -1;
> >   	
target = cpumask_any_but(cpu_online_mask, cpu);
> >   	if (target >= nr_cpu_ids) {  
> 
> And this again is nonsense anyway - if the pointless dead code is 
> bothering people, just delete the whole check.
True enough.  I fear I gut and paste that from somewhere so might
be worth a more general scrub for other instances :(  

J
> 
> Thanks,
> Robin.
> 
> > -		dev_err(info->pmu.dev, "Unable to find a suitable CPU\n");
> > +		dev_err(info->pmu.parent, "Unable to find a suitable CPU\n");
> >   		return 0;
> >   	}
> >     
> 
>