Re: [PATCH] s390/cpum_cf: Handle CPU hotplug add and delete

[Heiko Carstens <hca@linux.ibm.com>]

On Thu, Aug 06, 2026 at 03:11:31PM +0200, Thomas Richter wrote:
> The command 'perf stat -e cycles -- <command>' crashes the kernel
> when CPUs are hotplug added during that run.
> 
> Root cause is the allocation of struct cpu_cf_events at first
> event initialization. The allocation is dynamic and the first
> event that has task context creates such a structure for
> each online CPU. This is not sufficient. CPUs may be offline
> during event creation and can be set online during the
> perf run time. For example commands
...
> The issue arises only in per-task context when the CPUMF facility is
> used and the scheduler picks a random CPU for such a process to run on.
> The scheduler enables the CPUMF infrastructure via PMU callback
> functions pmu::add() and pmu::del().
> Now count all per-task processes currently running and active.
> When a CPU is hotplug added, check for running per-task context
> processes. If one or more are active, install the CPUMF infrastructure
> on that new CPU.  This ensures the infrastructure is available when
> new CPU is selected to run the per-task context process.

Wouldn't it be easier to add another cpu hotplug handler which is called while
the new / going cpu is offline? That is allocating and freeing of per-cpu data
structures is done on a different CPU. It looks like this would solve also a
couple of the other Sashiko reports about life time.

_Something_ like the below. Completely untested and might be completely
broken, however the implementation looks much simpler, and you don't need to
sprinkle NULL checks everywhere.

---
 arch/s390/kernel/perf_cpum_cf.c | 40 +++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
index 2076ac22e2c4..c88ca94b9ec4 100644
--- a/arch/s390/kernel/perf_cpum_cf.c
+++ b/arch/s390/kernel/perf_cpum_cf.c
@@ -110,6 +110,7 @@ struct cpu_cf_ptr {
 
 static struct cpu_cf_root {		/* Anchor to per CPU data */
 	refcount_t refcnt;		/* Overall active events */
+	refcount_t syswide_refcnt;	/* System wide active events */
 	struct cpu_cf_ptr __percpu *cfptr;
 } cpu_cf_root;
 
@@ -293,6 +294,7 @@ static int cpum_cf_alloc(int cpu)
 	if (cpu == -1) {
 		if (!zalloc_cpumask_var(&mask, GFP_KERNEL))
 			return -ENOMEM;
+		cpus_read_lock();
 		for_each_online_cpu(cpu) {
 			rc = cpum_cf_alloc_cpu(cpu);
 			if (rc) {
@@ -302,6 +304,9 @@ static int cpum_cf_alloc(int cpu)
 			}
 			cpumask_set_cpu(cpu, mask);
 		}
+		if (!rc)
+			refcount_inc(&cpu_cf_root.syswide_refcnt);
+		cpus_read_unlock();
 		free_cpumask_var(mask);
 	} else {
 		rc = cpum_cf_alloc_cpu(cpu);
@@ -312,8 +317,11 @@ static int cpum_cf_alloc(int cpu)
 static void cpum_cf_free(int cpu)
 {
 	if (cpu == -1) {
+		cpus_read_lock();
+		refcount_dec(&cpu_cf_root.syswide_refcnt);
 		for_each_online_cpu(cpu)
 			cpum_cf_free_cpu(cpu);
+		cpus_read_unlock();
 	} else {
 		cpum_cf_free_cpu(cpu);
 	}
@@ -1089,6 +1097,22 @@ static refcount_t cfset_opencnt = REFCOUNT_INIT(0);	/* Access count */
  */
 static DEFINE_MUTEX(cfset_ctrset_mutex);
 
+static int cpum_cf_prepare_cpu(unsigned int cpu)
+{
+	int rc = 0;
+
+	if (refcount_read(&cpu_cf_root.syswide_refcnt))
+		rc = cpum_cf_alloc_cpu(cpu);
+	return rc;
+}
+
+static int cpum_cf_dead_cpu(unsigned int cpu)
+{
+	if (refcount_read(&cpu_cf_root.syswide_refcnt))
+		cpum_cf_free_cpu(cpu);
+	return 0;
+}
+
 /*
  * CPU hotplug handles only /dev/hwctr device.
  * For perf_event_open() the CPU hotplug handling is done on kernel common
@@ -1183,7 +1207,7 @@ static void cpumf_measurement_alert(struct ext_code ext_code,
 static int cfset_init(void);
 static int __init cpumf_pmu_init(void)
 {
-	int rc;
+	int state, rc;
 
 	/* Extract counter measurement facility information */
 	if (!cpum_cf_avail() || qctri(&cpumf_ctr_info))
@@ -1225,11 +1249,23 @@ static int __init cpumf_pmu_init(void)
 		cfset_init();
 	}
 
+	rc = cpuhp_setup_state(CPUHP_BP_PREPARE_DYN,
+			       "perf/s390/cf:prepare",
+			       cpum_cf_prepare_cpu, cpum_cf_dead_cpu);
+	if (rc < 0)
+		goto out3;
+	state = rc;
 	rc = cpuhp_setup_state(CPUHP_AP_PERF_S390_CF_ONLINE,
 			       "perf/s390/cf:online",
 			       cpum_cf_online_cpu, cpum_cf_offline_cpu);
-	return rc;
+	if (rc < 0)
+		goto out4;
+	return 0;
 
+out4:
+	cpuhp_remove_state(state);
+out3:
+	perf_pmu_unregister(&cpumf_pmu);
 out2:
 	debug_unregister_view(cf_dbg, &debug_sprintf_view);
 	debug_unregister(cf_dbg);
-- 
2.53.0