Re: [PATCH v3 7/9] perf/cxl: Unfreeze counters after handling an overflow interrupt

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Cross-CPU data race on local64_t variables and spurious counter increments during CPU hotplug migration.
- [High] Unregistering the PMU before removing the CPU hotplug instance creates a Use-After-Free window during driver removal.
--

--- Patch [7]: [PATCH v3 7/9] perf/cxl: Unfreeze counters after handling an overflow interrupt ---
commit d6decd45c5a5019eddbbf261ba3f712c1ad75535
Author: Dave Jiang <dave.jiang@intel.com>

perf/cxl: Unfreeze counters after handling an overflow interrupt

This patch ensures CXL PMU counters are unfrozen after clearing an overflow
interrupt. Previously, counters remained frozen until the next pmu_enable(),
causing events to be lost during that window. It tracks the PMU enabled
state to avoid unfreezing if the PMU was disabled concurrently.

Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 6fdc66a01fb67..580a75bc9210a 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c

[ ... ]

> @@ -803,6 +807,9 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
>  
>  	writeq(overflowed, base + CXL_PMU_OVERFLOW_REG);
>  
> +	if (READ_ONCE(info->enabled))
> +		writeq(0, base + CXL_PMU_FREEZE_REG);
> +
>  	return IRQ_HANDLED;
>  }

[Severity: High]
This is a pre-existing issue, but can this sequence corrupt performance
counters during CPU hotplug?

Looking at cxl_pmu_offline_cpu(), perf_pmu_migrate_context() is called
before the IRQ affinity is updated:

drivers/perf/cxl_pmu.c:cxl_pmu_offline_cpu() {
	...
	perf_pmu_migrate_context(&info->pmu, cpu, target);
	info->on_cpu = target;
	/*
	 * CPU HP lock is held so we should be guaranteed that this CPU hasn't
	 * yet gone away.
	 */
	WARN_ON(irq_set_affinity(info->irq, cpumask_of(target)));
	...
}

If an overflow MSI is delivered to the old CPU in this narrow window,
cxl_pmu_irq() runs on the old CPU. If the new CPU has already re-added and
started the event, it has cleared hwc->prev_count to 0.

Because the hardware overflow register is only cleared by the IRQ handler,
the old CPU sees a stale overflow flag and invokes __cxl_pmu_read(). The
condition new_cnt >= prev_cnt passes (since prev_cnt is 0), causing it to
erroneously add a full period (mask + 1) to the counter:

drivers/perf/cxl_pmu.c:__cxl_pmu_read() {
	...
	do {
		prev_cnt = local64_read(&hwc->prev_count);
		new_cnt = cxl_pmu_read_counter(event);
	} while (local64_cmpxchg(&hwc->prev_count, prev_cnt, new_cnt) != prev_cnt);

	/* ... */
	mask = GENMASK_ULL(info->counter_width - 1, 0);
	delta = (new_cnt - prev_cnt) & mask;
	if (overflow && new_cnt >= prev_cnt)
		delta += mask + 1;

	local64_add(delta, &event->count);
}

Does this also cause a data race since local64_t operations lack cross-CPU
atomic guarantees?

[Severity: High]
This isn't a bug introduced by this patch, but does the devm teardown order
create a use-after-free window during driver removal?

In cxl_pmu_probe(), the CPU hotplug instance is registered before the PMU:

drivers/perf/cxl_pmu.c:cxl_pmu_probe() {
	...
	rc = cpuhp_state_add_instance(cxl_pmu_cpuhp_state_num, &info->node);
	if (rc)
		return rc;

	rc = devm_add_action_or_reset(dev, cxl_pmu_cpuhp_remove, info);
	if (rc)
		return rc;

	rc = perf_pmu_register(&info->pmu, info->pmu.name, -1);
	if (rc)
		return rc;

	rc = devm_add_action_or_reset(dev, cxl_pmu_perf_unregister, info);
	...
}

During driver removal, devm executes actions in LIFO order.
cxl_pmu_perf_unregister() runs first, delegating to perf_pmu_unregister(),
which frees pmu->cpu_pmu_context.

If a CPU is offlined concurrently before cxl_pmu_cpuhp_remove() runs,
cxl_pmu_offline_cpu() is invoked:

drivers/perf/cxl_pmu.c:cxl_pmu_offline_cpu() {
	...
	perf_pmu_migrate_context(&info->pmu, cpu, target);
	...
}

Will perf_pmu_migrate_context() access the freed cpu_pmu_context when it
calls find_get_pmu_context()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731232827.401447-1-dave.jiang@intel.com?part=7