[SRU][R][PATCH 1/1] s390/cpum_cf: Handle CPU hotplug via prepare/dead callbacks
Massimiliano Pellizzer
massimiliano.pellizzer at canonical.com
Thu Sep 3 16:01:15 UTC 2026
From: Thomas Richter <tmricht at linux.ibm.com>
BugLink: https://bugs.launchpad.net/bugs/2165732
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
# echo 0 > /sys/devices/system/cpu/cpu1/online
# perf stat -e cycles -i -- stress-ng -t10s --matrix X
# sleep 1
# echo 1 > /sys/devices/system/cpu/cpu1/online
create an event for CPUs 0,2-X. Since the events are created with
task-context, the scheduler will eventually schedule the program
on CPU1. This CPU has not created and initialized any per
CPU event infrastructure as that CPU was not online at the time
of the perf invocation. Thus when the scheduler runs stress-ng
on CPU1, the function cpumf_pmu_add() refers to a NULL pointer:
struct cpu_cf_events *cpuhw = this_cpu_cfhw();
This function call is invoked after the task stress-ng has been
made runnable on CPU1. And this_cpu_cfhw() returns NULL.
The result is a panic:
Unable to handle kernel pointer dereference in virtual kernel address space
Failing address: 0000000000000000 TEID: 0000000000000483
....
Krnl PSW : 0404d00180000000 000003ef8291fd0c (cpumf_pmu_add+0x3c/0x80)
....
Call Trace:
[<000003ef8291fd0c>] cpumf_pmu_add+0x3c/0x80
[<000003ef82bb5e3e>] event_sched_in+0xae/0x190
[<000003ef82bb60d6>] merge_sched_in+0x1b6/0x390
[<000003ef82bb65b8>] visit_groups_merge.constprop.0.isra.0+0x308/0x5b0
[<000003ef82bb689a>] pmu_groups_sched_in+0x3a/0x50
[<000003ef82bb6a30>] ctx_sched_in+0x180/0x260
[<000003ef82bb780c>] perf_event_context_sched_in+0x11c/0x2d0
[<000003ef82bb79ee>] __perf_event_task_sched_in+0x2e/0xc0
[<000003ef82994834>] finish_task_switch.isra.0+0x1a4/0x250
....
Last Breaking-Event-Address:
[<000003ef8291f1d8>] this_cpu_cfhw+0x38/0x40
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().
Introduce a CPU hotplug prepare/dead callback pair which creates and
removes the per CPU counter data while the CPU is offline. Count the
users which track every CPU (cpu == -1), that is perf_event_open()
events with task context and /dev/hwctr device sessions, in the new
counter cpu_cf_root::tskcnt, protected by pmc_reserve_mutex.
This ensures the infrastructure is available when
new CPU is selected to run the per-task context process.
In cpum_cf_free_root() and cpum_cf_free_cpu() ensure the reference
pointer to data structures is set to NULL before the data is freed
to prevent interrupt handlers to access stale data.
[gor at linux.ibm.com: change commit message]
Fixes: 9b9cf3c77e7e ("s390/cpum_cf: rework PER_CPU_DEFINE of struct cpu_cf_events")
Cc: stable at vger.kernel.org # v6.5+
Suggested-by: Heiko Carstens <hca at linux.ibm.com>
Suggested-by: Christian Borntraeger <borntraeger at linux.ibm.com>
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Thomas Richter <tmricht at linux.ibm.com>
Acked-by: Heiko Carstens <hca at linux.ibm.com>
Signed-off-by: Vasily Gorbik <gor at linux.ibm.com>
(cherry picked from commit 337bd95507a16063687cfc286ea90de5cca48c37)
Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer at canonical.com>
---
arch/s390/kernel/perf_cpum_cf.c | 215 +++++++++++++++++++++-----------
1 file changed, 142 insertions(+), 73 deletions(-)
diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
index 2076ac22e2c49..2a8d4cc1d6a2c 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 */
+ unsigned int tskctx; /* Users tracking all CPUs (cpu == -1) */
struct cpu_cf_ptr __percpu *cfptr;
} cpu_cf_root;
@@ -118,13 +119,15 @@ static struct cpu_cf_root { /* Anchor to per CPU data */
* user space in task context with perf_event_open() and close()
* system calls.
*
- * This mutex serializes functions cpum_cf_alloc_cpu() called at event
- * initialization via cpumf_pmu_event_init() and function cpum_cf_free_cpu()
- * called at event removal via call back function hw_perf_event_destroy()
- * when the event is deleted. They are serialized to enforce correct
- * bookkeeping of pointer and reference counts anchored by
- * struct cpu_cf_root and the access to cpu_cf_root::refcnt and the
- * per CPU pointers stored in cpu_cf_root::cfptr.
+ * This mutex serializes the allocation and removal of the per CPU counter
+ * data via cpum_cf_alloc_cpu() and cpum_cf_free_cpu(). They are called with
+ * this mutex held at event initialization via cpumf_pmu_event_init(), at
+ * event removal via call back function hw_perf_event_destroy() when the
+ * event is deleted, and from the CPU hotplug prepare/dead callbacks. The
+ * mutex enforces correct bookkeeping of pointer and reference counts
+ * anchored by struct cpu_cf_root and protects the access to
+ * cpu_cf_root::refcnt, cpu_cf_root::tskctx and the per CPU pointers
+ * stored in cpu_cf_root::cfptr.
*/
static DEFINE_MUTEX(pmc_reserve_mutex);
@@ -167,12 +170,14 @@ static void cpum_cf_reset_cpu(void *flags)
}
/* Free per CPU data when the last event is removed. */
-static void cpum_cf_free_root(void)
+static void cpum_cf_free_root(unsigned int num)
{
- if (!refcount_dec_and_test(&cpu_cf_root.refcnt))
+ struct cpu_cf_ptr __percpu *p = cpu_cf_root.cfptr;
+
+ if (!refcount_sub_and_test(num, &cpu_cf_root.refcnt))
return;
- free_percpu(cpu_cf_root.cfptr);
cpu_cf_root.cfptr = NULL;
+ free_percpu(p);
irq_subclass_unregister(IRQ_SUBCLASS_MEASUREMENT_ALERT);
on_each_cpu(cpum_cf_reset_cpu, NULL, 1);
debug_sprintf_event(cf_dbg, 4, "%s root.refcnt %u cfptr %d\n",
@@ -186,17 +191,17 @@ static void cpum_cf_free_root(void)
* CPUs possible, which might be larger than the number of CPUs currently
* online.
*/
-static int cpum_cf_alloc_root(void)
+static int cpum_cf_alloc_root(unsigned int num)
{
int rc = 0;
- if (refcount_inc_not_zero(&cpu_cf_root.refcnt))
+ if (refcount_add_not_zero(num, &cpu_cf_root.refcnt))
return rc;
/* The memory is already zeroed. */
cpu_cf_root.cfptr = alloc_percpu(struct cpu_cf_ptr);
if (cpu_cf_root.cfptr) {
- refcount_set(&cpu_cf_root.refcnt, 1);
+ refcount_set(&cpu_cf_root.refcnt, num);
on_each_cpu(cpum_cf_reset_cpu, NULL, 1);
irq_subclass_register(IRQ_SUBCLASS_MEASUREMENT_ALERT);
} else {
@@ -206,20 +211,23 @@ static int cpum_cf_alloc_root(void)
return rc;
}
-/* Free CPU counter data structure for a PMU */
-static void cpum_cf_free_cpu(int cpu)
+/*
+ * Remove num references to the CPU counter data structure of a PMU.
+ * Called with pmc_reserve_mutex held.
+ */
+static void cpum_cf_free_cpu(int cpu, unsigned int num)
{
struct cpu_cf_events *cpuhw;
struct cpu_cf_ptr *p;
- mutex_lock(&pmc_reserve_mutex);
+ lockdep_assert_held(&pmc_reserve_mutex);
/*
* When invoked via CPU hotplug handler, there might be no events
* installed or that particular CPU might not have an
* event installed. This anchor pointer can be NULL!
*/
if (!cpu_cf_root.cfptr)
- goto out;
+ return;
p = per_cpu_ptr(cpu_cf_root.cfptr, cpu);
cpuhw = p->cpucf;
/*
@@ -227,28 +235,29 @@ static void cpum_cf_free_cpu(int cpu)
* installed on that CPU, but on different CPUs.
*/
if (!cpuhw)
- goto out;
+ return;
- if (refcount_dec_and_test(&cpuhw->refcnt)) {
- kfree(cpuhw);
+ if (refcount_sub_and_test(num, &cpuhw->refcnt)) {
p->cpucf = NULL;
+ kfree(cpuhw);
}
- cpum_cf_free_root();
-out:
- mutex_unlock(&pmc_reserve_mutex);
+ cpum_cf_free_root(num);
}
-/* Allocate CPU counter data structure for a PMU. Called under mutex lock. */
-static int cpum_cf_alloc_cpu(int cpu)
+/*
+ * Add num references to the CPU counter data structure of a PMU and
+ * allocate it when necessary. Called with pmc_reserve_mutex held.
+ */
+static int cpum_cf_alloc_cpu(int cpu, unsigned int num)
{
struct cpu_cf_events *cpuhw;
struct cpu_cf_ptr *p;
int rc;
- mutex_lock(&pmc_reserve_mutex);
- rc = cpum_cf_alloc_root();
+ lockdep_assert_held(&pmc_reserve_mutex);
+ rc = cpum_cf_alloc_root(num);
if (rc)
- goto unlock;
+ return rc;
p = per_cpu_ptr(cpu_cf_root.cfptr, cpu);
cpuhw = p->cpucf;
@@ -256,12 +265,12 @@ static int cpum_cf_alloc_cpu(int cpu)
cpuhw = kzalloc_obj(*cpuhw);
if (cpuhw) {
p->cpucf = cpuhw;
- refcount_set(&cpuhw->refcnt, 1);
+ refcount_set(&cpuhw->refcnt, num);
} else {
rc = -ENOMEM;
}
} else {
- refcount_inc(&cpuhw->refcnt);
+ refcount_add(num, &cpuhw->refcnt);
}
if (rc) {
/*
@@ -269,10 +278,8 @@ static int cpum_cf_alloc_cpu(int cpu)
* cpu_cf_event in not created, its destroy() function is not
* invoked. Adjust the reference counter for the anchor.
*/
- cpum_cf_free_root();
+ cpum_cf_free_root(num);
}
-unlock:
- mutex_unlock(&pmc_reserve_mutex);
return rc;
}
@@ -284,39 +291,70 @@ static int cpum_cf_alloc_cpu(int cpu)
* perf_event_open() with task context and /dev/hwctr interface.
* If cpu is non-zero install event on this CPU only. This setup handles
* perf_event_open() with CPU context.
+ * Users with cpu == -1 are counted in cpu_cf_root::tskctx. The CPU hotplug
+ * prepare and dead callbacks use this count to install and remove the per
+ * CPU counter data on a new or dying CPU.
*/
-static int cpum_cf_alloc(int cpu)
+static int cpum_cf_alloc_cpuslocked(int cpu)
{
cpumask_var_t mask;
int rc;
+ lockdep_assert_cpus_held();
if (cpu == -1) {
if (!zalloc_cpumask_var(&mask, GFP_KERNEL))
return -ENOMEM;
+ mutex_lock(&pmc_reserve_mutex);
for_each_online_cpu(cpu) {
- rc = cpum_cf_alloc_cpu(cpu);
+ rc = cpum_cf_alloc_cpu(cpu, 1);
if (rc) {
for_each_cpu(cpu, mask)
- cpum_cf_free_cpu(cpu);
+ cpum_cf_free_cpu(cpu, 1);
break;
}
cpumask_set_cpu(cpu, mask);
}
+ if (!rc)
+ cpu_cf_root.tskctx++;
+ mutex_unlock(&pmc_reserve_mutex);
free_cpumask_var(mask);
} else {
- rc = cpum_cf_alloc_cpu(cpu);
+ mutex_lock(&pmc_reserve_mutex);
+ rc = cpum_cf_alloc_cpu(cpu, 1);
+ mutex_unlock(&pmc_reserve_mutex);
}
return rc;
}
-static void cpum_cf_free(int cpu)
+static int cpum_cf_alloc(int cpu)
+{
+ int rc;
+
+ cpus_read_lock();
+ rc = cpum_cf_alloc_cpuslocked(cpu);
+ cpus_read_unlock();
+ return rc;
+}
+
+static void cpum_cf_free_cpuslocked(int cpu)
{
+ lockdep_assert_cpus_held();
+ mutex_lock(&pmc_reserve_mutex);
if (cpu == -1) {
+ cpu_cf_root.tskctx--;
for_each_online_cpu(cpu)
- cpum_cf_free_cpu(cpu);
+ cpum_cf_free_cpu(cpu, 1);
} else {
- cpum_cf_free_cpu(cpu);
+ cpum_cf_free_cpu(cpu, 1);
}
+ mutex_unlock(&pmc_reserve_mutex);
+}
+
+static void cpum_cf_free(int cpu)
+{
+ cpus_read_lock();
+ cpum_cf_free_cpuslocked(cpu);
+ cpus_read_unlock();
}
#define CF_DIAG_CTRSET_DEF 0xfeef /* Counter set header mark */
@@ -1090,53 +1128,67 @@ static refcount_t cfset_opencnt = REFCOUNT_INIT(0); /* Access count */
static DEFINE_MUTEX(cfset_ctrset_mutex);
/*
- * CPU hotplug handles only /dev/hwctr device.
- * For perf_event_open() the CPU hotplug handling is done on kernel common
- * code:
+ * CPU hotplug handling:
+ *
+ * cpum_cf_prepare_cpu() and cpum_cf_dead_cpu() run while the new or dying
+ * CPU is offline. They create and remove the per CPU counter data for all
+ * users tracking every CPU (cpu == -1), that is perf_event_open() events
+ * with task context and /dev/hwctr device sessions. Each such user holds
+ * one reference to the per CPU counter data of each CPU. Therefore install
+ * and remove one reference per user, tracked in cpu_cf_root::tskctx. This
+ * guarantees the per CPU counter data exists before the new CPU executes
+ * its first task and is removed only after the dying CPU is gone.
+ *
+ * cpum_cf_online_cpu() and cpum_cf_offline_cpu() run while the new or
+ * dying CPU is online. They handle only the counter set state of open
+ * /dev/hwctr device sessions on that CPU. For perf_event_open() events
+ * nothing is done:
* - CPU add: Nothing is done since a file descriptor can not be created
* and returned to the user.
* - CPU delete: Handled by common code via pmu_disable(), pmu_stop() and
- * pmu_delete(). The event itself is removed when the file descriptor is
- * closed.
+ * pmu_delete(). During task exit processing of grouped perf events
+ * triggered by CPU hotplug processing, pmu_disable() is called as part
+ * of perf context removal process. The event itself is removed when the
+ * event file descriptor is closed.
*/
+static int cpum_cf_prepare_cpu(unsigned int cpu)
+{
+ int rc = 0;
+
+ mutex_lock(&pmc_reserve_mutex);
+ if (cpu_cf_root.tskctx)
+ rc = cpum_cf_alloc_cpu(cpu, cpu_cf_root.tskctx);
+ mutex_unlock(&pmc_reserve_mutex);
+ return rc;
+}
+
+static int cpum_cf_dead_cpu(unsigned int cpu)
+{
+ mutex_lock(&pmc_reserve_mutex);
+ if (cpu_cf_root.tskctx)
+ cpum_cf_free_cpu(cpu, cpu_cf_root.tskctx);
+ mutex_unlock(&pmc_reserve_mutex);
+ return 0;
+}
+
static int cfset_online_cpu(unsigned int cpu);
static int cpum_cf_online_cpu(unsigned int cpu)
{
- int rc = 0;
-
- /*
- * Ignore notification for perf_event_open().
- * Handle only /dev/hwctr device sessions.
- */
mutex_lock(&cfset_ctrset_mutex);
- if (refcount_read(&cfset_opencnt)) {
- rc = cpum_cf_alloc_cpu(cpu);
- if (!rc)
- cfset_online_cpu(cpu);
- }
+ if (refcount_read(&cfset_opencnt))
+ cfset_online_cpu(cpu);
mutex_unlock(&cfset_ctrset_mutex);
- return rc;
+ return 0;
}
static int cfset_offline_cpu(unsigned int cpu);
static int cpum_cf_offline_cpu(unsigned int cpu)
{
- /*
- * During task exit processing of grouped perf events triggered by CPU
- * hotplug processing, pmu_disable() is called as part of perf context
- * removal process. Therefore do not trigger event removal now for
- * perf_event_open() created events. Perf common code triggers event
- * destruction when the event file descriptor is closed.
- *
- * Handle only /dev/hwctr device sessions.
- */
mutex_lock(&cfset_ctrset_mutex);
- if (refcount_read(&cfset_opencnt)) {
+ if (refcount_read(&cfset_opencnt))
cfset_offline_cpu(cpu);
- cpum_cf_free_cpu(cpu);
- }
mutex_unlock(&cfset_ctrset_mutex);
return 0;
}
@@ -1183,7 +1235,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 +1277,24 @@ 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);
@@ -1385,6 +1450,7 @@ static void cfset_all_stop(struct cfset_request *req)
*/
static int cfset_release(struct inode *inode, struct file *file)
{
+ cpus_read_lock();
mutex_lock(&cfset_ctrset_mutex);
/* Open followed by close/exit has no private_data */
if (file->private_data) {
@@ -1395,9 +1461,10 @@ static int cfset_release(struct inode *inode, struct file *file)
}
if (refcount_dec_and_test(&cfset_opencnt)) { /* Last close */
on_each_cpu(cfset_release_cpu, NULL, 1);
- cpum_cf_free(-1);
+ cpum_cf_free_cpuslocked(-1);
}
mutex_unlock(&cfset_ctrset_mutex);
+ cpus_read_unlock();
return 0;
}
@@ -1416,15 +1483,17 @@ static int cfset_open(struct inode *inode, struct file *file)
return -EPERM;
file->private_data = NULL;
+ cpus_read_lock();
mutex_lock(&cfset_ctrset_mutex);
if (!refcount_inc_not_zero(&cfset_opencnt)) { /* First open */
- rc = cpum_cf_alloc(-1);
+ rc = cpum_cf_alloc_cpuslocked(-1);
if (!rc) {
cfset_session_init();
refcount_set(&cfset_opencnt, 1);
}
}
mutex_unlock(&cfset_ctrset_mutex);
+ cpus_read_unlock();
/* nonseekable_open() never fails */
return rc ?: nonseekable_open(inode, file);
--
2.53.0
More information about the kernel-team
mailing list