[PATCH] perf/arm-cmn: Allow userspace to select the PMU's CPU

[PATCH] perf/arm-cmn: Allow userspace to select the PMU's CPU

[Haris Okanovic]

arm_cmn_probe() picks the CPU that owns the PMU with

	cmn->cpu = cpumask_local_spread(0, dev_to_node(cmn->dev));

and arm_cmn_event_init() then binds every event to it unconditionally.
That choice is only revisited by the CPU hotplug callbacks, so in
practice the PMU stays on the first CPU local to the interconnect's NUMA
node, which is usually CPU 0.

All of the PMU's recurring work therefore lands on one CPU. This is
problematic on systems which reserve particular CPUs for latency
sensitive work or confine background activity to a chosen set of
housekeeping CPUs.

There is no way to set it explicitly. perf_event_open() and 'perf stat
-C' have no effect because arm_cmn_event_init() overwrites event->cpu;
/proc/irq/*/smp_affinity is refused for the DTC interrupts, which are
requested with IRQF_NOBALANCING because their affinity has to follow the
owning CPU.

Make the existing 'cpumask' attribute writable. It already reports the
CPU which owns the PMU; writing a CPU number now migrates the PMU there
via the existing arm_cmn_migrate(), which moves the perf contexts and
the DTC interrupt affinity together. Since the PMU has a single owning
CPU, anything other than one CPU number is rejected.

The attribute expresses a preference rather than a guarantee. The
hotplug callbacks may still move the PMU, for example when the chosen
CPU is offlined.

Signed-off-by: Haris Okanovic <harisokn@amazon.com>
---
I made the existing 'cpumask' attribute writable rather than
adding a new one. Every other implementation of that file is read-only,
so I am happy to switch to a separate attribute if you would prefer to
keep 'cpumask' uniformly read-only across PMUs.

Tested on two platforms with CONFIG_PROVE_LOCKING=y:

  AWS m9g.metal-48xl CMN S3,  one mesh,   192 CPUs
  AWS m8g.metal-48xl CMN-700, two meshes, 192 CPUs

  - Multiplexing occurs on the configured CPU.
  - Offlining the owning CPU migrates the PMU and updates the attribute.
  - Writes racing CPU offline/online produced no lockdep reports.
---
 Documentation/admin-guide/perf/arm-cmn.rst | 18 ++++++++++
 drivers/perf/arm-cmn.c                     | 40 +++++++++++++++++++++-
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/perf/arm-cmn.rst b/Documentation/admin-guide/perf/arm-cmn.rst
index 796e25b7027b2..bf75b686cef76 100644
--- a/Documentation/admin-guide/perf/arm-cmn.rst
+++ b/Documentation/admin-guide/perf/arm-cmn.rst
@@ -44,6 +44,24 @@ given type. To target a specific node, "bynodeid" must be set to 1 and
 "nodeid" to the appropriate value derived from the CMN configuration
 (as defined in the "Node ID Mapping" section of the TRM).
 
+CPU affinity
+------------
+
+The driver also provides a "cpumask" sysfs attribute, which contains a
+single CPU ID, of the processor which will be used to handle all the CMN
+PMU events.
+
+The attribute is writable, and accepts a single CPU ID to move the PMU
+to that processor, for instance to keep counter reads, interrupt handling
+and event rotation away from CPUs reserved for latency sensitive work::
+
+  $# echo 5 > /sys/bus/event_source/devices/arm_cmn_0/cpumask
+
+This expresses a preference rather than a guarantee. In case of the
+chosen processor being offlined, or a processor local to the
+interconnect's NUMA node coming online while the chosen one is not, the
+events and interrupts are migrated and the attribute is updated.
+
 Watchpoints
 -----------
 
diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
index 5378fba916cf5..4f82d667d8828 100644
--- a/drivers/perf/arm-cmn.c
+++ b/drivers/perf/arm-cmn.c
@@ -5,6 +5,7 @@
 #include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
+#include <linux/cpu.h>
 #include <linux/debugfs.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -12,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
@@ -404,6 +406,8 @@ struct arm_cmn_nodeid {
 	u8 dev;
 };
 
+static void arm_cmn_migrate(struct arm_cmn *cmn, unsigned int cpu);
+
 static int arm_cmn_xyidbits(const struct arm_cmn *cmn)
 {
 	return fls((cmn->mesh_x - 1) | (cmn->mesh_y - 1));
@@ -1519,8 +1523,42 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev,
 	return sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(cpumask_of(cmn->cpu)));
 }
 
+static ssize_t arm_cmn_cpumask_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	static DEFINE_MUTEX(cpumask_mutex);
+
+	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
+	unsigned int cpu;
+	int err;
+
+	err = kstrtouint(buf, 0, &cpu);
+	if (err)
+		return err;
+
+	if (cpu >= nr_cpu_ids)
+		return -EINVAL;
+
+	/* Serialises multiple writers against each other */
+	mutex_lock(&cpumask_mutex);
+	/* Blocks hotplug during write */
+	cpus_read_lock();
+
+	if (!cpu_online(cpu))
+		err = -EINVAL;
+	else if (cpu != cmn->cpu)
+		arm_cmn_migrate(cmn, cpu);
+
+	cpus_read_unlock();
+	mutex_unlock(&cpumask_mutex);
+
+	return err ?: count;
+}
+
 static struct device_attribute arm_cmn_cpumask_attr =
-		__ATTR(cpumask, 0444, arm_cmn_cpumask_show, NULL);
+		__ATTR(cpumask, 0644, arm_cmn_cpumask_show,
+		       arm_cmn_cpumask_store);
 
 static ssize_t arm_cmn_identifier_show(struct device *dev,
 				       struct device_attribute *attr, char *buf)
-- 
2.34.1

Re: [PATCH] perf/arm-cmn: Allow userspace to select the PMU's CPU

[Robin Murphy]

On 2026-09-29 11:32 pm, Haris Okanovic wrote:
> arm_cmn_probe() picks the CPU that owns the PMU with
> 
> 	cmn->cpu = cpumask_local_spread(0, dev_to_node(cmn->dev));
> 
> and arm_cmn_event_init() then binds every event to it unconditionally.
> That choice is only revisited by the CPU hotplug callbacks, so in
> practice the PMU stays on the first CPU local to the interconnect's NUMA
> node, which is usually CPU 0.
> 
> All of the PMU's recurring work therefore lands on one CPU. This is
> problematic on systems which reserve particular CPUs for latency
> sensitive work or confine background activity to a chosen set of
> housekeeping CPUs.
> 
> There is no way to set it explicitly. perf_event_open() and 'perf stat
> -C' have no effect because arm_cmn_event_init() overwrites event->cpu;
> /proc/irq/*/smp_affinity is refused for the DTC interrupts, which are
> requested with IRQF_NOBALANCING because their affinity has to follow the
> owning CPU.

All system PMU drivers have the same concern in this regard - why should 
arm-cmn be special?

> Make the existing 'cpumask' attribute writable. It already reports the
> CPU which owns the PMU; writing a CPU number now migrates the PMU there
> via the existing arm_cmn_migrate(), which moves the perf contexts and
> the DTC interrupt affinity together. Since the PMU has a single owning
> CPU, anything other than one CPU number is rejected.
> 
> The attribute expresses a preference rather than a guarantee. The
> hotplug callbacks may still move the PMU, for example when the chosen
> CPU is offlined.
> 
> Signed-off-by: Haris Okanovic <harisokn@amazon.com>
> ---
> I made the existing 'cpumask' attribute writable rather than
> adding a new one. Every other implementation of that file is read-only,
> so I am happy to switch to a separate attribute if you would prefer to
> keep 'cpumask' uniformly read-only across PMUs.
> 
> Tested on two platforms with CONFIG_PROVE_LOCKING=y:
> 
>    AWS m9g.metal-48xl CMN S3,  one mesh,   192 CPUs
>    AWS m8g.metal-48xl CMN-700, two meshes, 192 CPUs
> 
>    - Multiplexing occurs on the configured CPU.
>    - Offlining the owning CPU migrates the PMU and updates the attribute.
>    - Writes racing CPU offline/online produced no lockdep reports.

Lockdep isn't going to do much anyway, since the whole point is that 
perf mostly depends on CPU affinity and IRQ masking for mutual exclusion 
and serialisation, rather than explicit locking. What prevents 
perf_event_open racing against this update such that the new event->cpu 
still ends up with the old value, and thus will no longer be correctly 
synchronised against other scheduling calls/overflow interrupts/etc.?

Now yes, I think technically that race might already exist in a tiny 
window due to the order of hotplug callbacks (yet another reason why I'd 
like to clean up hotplug handling...), but at worst it's still 
relatively benign if the old CPU is actually going offline, since it 
will very soon reach a state where that misconfigured new event just 
won't work - since trying to schedule or read it depends on 
cross-calling a CPU that's now gone - but at least it's then not capable 
of corrupting _other_ events.

Thanks,
Robin.

> ---
>   Documentation/admin-guide/perf/arm-cmn.rst | 18 ++++++++++
>   drivers/perf/arm-cmn.c                     | 40 +++++++++++++++++++++-
>   2 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/perf/arm-cmn.rst b/Documentation/admin-guide/perf/arm-cmn.rst
> index 796e25b7027b2..bf75b686cef76 100644
> --- a/Documentation/admin-guide/perf/arm-cmn.rst
> +++ b/Documentation/admin-guide/perf/arm-cmn.rst
> @@ -44,6 +44,24 @@ given type. To target a specific node, "bynodeid" must be set to 1 and
>   "nodeid" to the appropriate value derived from the CMN configuration
>   (as defined in the "Node ID Mapping" section of the TRM).
>   
> +CPU affinity
> +------------
> +
> +The driver also provides a "cpumask" sysfs attribute, which contains a
> +single CPU ID, of the processor which will be used to handle all the CMN
> +PMU events.
> +
> +The attribute is writable, and accepts a single CPU ID to move the PMU
> +to that processor, for instance to keep counter reads, interrupt handling
> +and event rotation away from CPUs reserved for latency sensitive work::
> +
> +  $# echo 5 > /sys/bus/event_source/devices/arm_cmn_0/cpumask
> +
> +This expresses a preference rather than a guarantee. In case of the
> +chosen processor being offlined, or a processor local to the
> +interconnect's NUMA node coming online while the chosen one is not, the
> +events and interrupts are migrated and the attribute is updated.
> +
>   Watchpoints
>   -----------
>   
> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index 5378fba916cf5..4f82d667d8828 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
> @@ -5,6 +5,7 @@
>   #include <linux/acpi.h>
>   #include <linux/bitfield.h>
>   #include <linux/bitops.h>
> +#include <linux/cpu.h>
>   #include <linux/debugfs.h>
>   #include <linux/interrupt.h>
>   #include <linux/io.h>
> @@ -12,6 +13,7 @@
>   #include <linux/kernel.h>
>   #include <linux/list.h>
>   #include <linux/module.h>
> +#include <linux/mutex.h>
>   #include <linux/of.h>
>   #include <linux/perf_event.h>
>   #include <linux/platform_device.h>
> @@ -404,6 +406,8 @@ struct arm_cmn_nodeid {
>   	u8 dev;
>   };
>   
> +static void arm_cmn_migrate(struct arm_cmn *cmn, unsigned int cpu);
> +
>   static int arm_cmn_xyidbits(const struct arm_cmn *cmn)
>   {
>   	return fls((cmn->mesh_x - 1) | (cmn->mesh_y - 1));
> @@ -1519,8 +1523,42 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev,
>   	return sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(cpumask_of(cmn->cpu)));
>   }
>   
> +static ssize_t arm_cmn_cpumask_store(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t count)
> +{
> +	static DEFINE_MUTEX(cpumask_mutex);
> +
> +	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
> +	unsigned int cpu;
> +	int err;
> +
> +	err = kstrtouint(buf, 0, &cpu);
> +	if (err)
> +		return err;
> +
> +	if (cpu >= nr_cpu_ids)
> +		return -EINVAL;
> +
> +	/* Serialises multiple writers against each other */
> +	mutex_lock(&cpumask_mutex);
> +	/* Blocks hotplug during write */
> +	cpus_read_lock();
> +
> +	if (!cpu_online(cpu))
> +		err = -EINVAL;
> +	else if (cpu != cmn->cpu)
> +		arm_cmn_migrate(cmn, cpu);
> +
> +	cpus_read_unlock();
> +	mutex_unlock(&cpumask_mutex);
> +
> +	return err ?: count;
> +}
> +
>   static struct device_attribute arm_cmn_cpumask_attr =
> -		__ATTR(cpumask, 0444, arm_cmn_cpumask_show, NULL);
> +		__ATTR(cpumask, 0644, arm_cmn_cpumask_show,
> +		       arm_cmn_cpumask_store);
>   
>   static ssize_t arm_cmn_identifier_show(struct device *dev,
>   				       struct device_attribute *attr, char *buf)

Re: [PATCH] perf/arm-cmn: Allow userspace to select the PMU's CPU

[Okanovic, Haris]

Hi Robin,

> > All of the PMU's recurring work therefore lands on one CPU. This is
> > problematic on systems which reserve particular CPUs for latency
> > sensitive work or confine background activity to a chosen set of
> > housekeeping CPUs.

> > There is no way to set it explicitly. perf_event_open() and 'perf stat
> > -C' have no effect because arm_cmn_event_init() overwrites event->cpu;
> > /proc/irq/*/smp_affinity is refused for the DTC interrupts, which are
> > requested with IRQF_NOBALANCING because their affinity has to follow the
> > owning CPU.

> All system PMU drivers have the same concern in this regard - why
> should arm-cmn be special?

As I mentioned earlier, we can improve performance on certain system
topologies by assigning PMU work to housekeeping CPUs.

arm-cmn has no constraints around CPU assignment, so this is possible.
Every register access is MMIO and the DTC interrupts can be affinitised
anywhere, so any online CPU can own it. That's what makes "write any
online CPU" a sound interface here. I've moved it to CPUs in both NUMA
nodes on the two platforms I tested.

You're right that the concern is general, but a single interface isn't
easily shared, because the set of CPUs that a PMU may be driven from
is platform/device-specific:

arm_dsu_pmu, for instance, can only be driven from the CPUs attached to
the DSU, which it already exposes as "associated_cpus" and enforces in
event_init(); hisi_uncore_pmu publishes the same attribute.

Intel uncore is per-die: MSR-accessed boxes must be read from a CPU on
the target die, since rdmsr reads the executing CPU. Accepting an
arbitrary CPU there would silently read a different die's counters.

Do you have an alternate API in mind?

> What prevents 
> perf_event_open racing against this update such that the new event->cpu 
> still ends up with the old value, and thus will no longer be correctly 
> synchronised against other scheduling calls/overflow interrupts/etc.?

> Now yes, I think technically that race might already exist in a tiny 
> window due to the order of hotplug callbacks (yet another reason why I'd 
> like to clean up hotplug handling...), but at worst it's still 
> relatively benign if the old CPU is actually going offline, since it 
> will very soon reach a state where that misconfigured new event just 
> won't work - since trying to schedule or read it depends on 
> cross-calling a CPU that's now gone - but at least it's then not capable 
> of corrupting _other_ events.

You're right, nothing prevents it today. I assumed the cpus_read_lock() in
the store covered this, but perf_event_open() doesn't take it. I'll look into
closing that window in a v2.

--
Haris Okanovic
AWS Graviton