Skip to contentSkip to conversation

Prepatory rework for multi-PMU support.

PATCH New Mark Rutland 2 participants 17 messages Oct 27, 2014 – Oct 28, 2014

Subsystems (3)
lore
Share & tools
lore

Mark Rutland

author maintainer
#0 lore raw

v2 cover Prepatory rework for multi-PMU support.

This series performs inital preparation for big.LITTLE perf support. It depends on the CCI rework posted previously [1], as the removal of get_hw_events and the percpu rework is incompatible with the single global set of events the CCI has.

Since v1 [2]:

* Added cpu_pmu_destroy to clean up dynamically allocated data and cpu notifiers in failure paths. Due to this change I've dropped the existing tags from patches 8 and 9.

* Make pmu_probe_table const.

* Removed the unintentional newline change from patch 3, and added a new patch to make format strings use newlines consistently.

* In patch 4 the armpmu_map_event type variable remains an int, but the (redundant) check against PERF_TYPE_MAX is removed such that it is only used for an equality check, as per [3].

As a result of this series:

* The callchain handling is split from the PMU driver, as with metag, powerpc, sh, and x86. This enables callchain handling for software events when the kernel is built without CONFIG_HW_PERF_EVENTS.

* Legacy MIDR-based PMU probing is factored into a table that a subsequent series will split into separate drivers. On its own this should not result in a behavioural change.

* The ARM PMU accounting structures are reorganised and absorbed by struct arm_pmu, which will make possible the allocation and management of multiple PMUs in a later patch series.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/295509.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/295820.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296088.html

Mark Rutland (9):
arm: perf: factor out callchain code
arm: perf: add missing pr_info newlines
arm: perf: make PMU probing data-driven
arm: perf: use IDR types for CPU PMUs
arm: perf: limit size of accounting data
arm: perf: kill get_hw_events()
arm: perf: fold percpu_pmu into pmu_hw_events
arm: perf: dynamically allocate cpu hardware data
arm: perf: fold hotplug notifier into arm_pmu

 arch/arm/include/asm/perf_event.h   |   2 +-
 arch/arm/include/asm/pmu.h          |  36 +++++++-
 arch/arm/kernel/Makefile            |   2 +-
 arch/arm/kernel/perf_callchain.c    | 137 +++++++++++++++++++++++++++
 arch/arm/kernel/perf_event.c        | 162 ++++----------------------------
 arch/arm/kernel/perf_event_cpu.c    | 179 ++++++++++++++++++------------------
 arch/arm/kernel/perf_event_v6.c     |  12 +--
 arch/arm/kernel/perf_event_v7.c     |  14 +--
 arch/arm/kernel/perf_event_xscale.c |  20 ++--
 9 files changed, 301 insertions(+), 263 deletions(-)
 create mode 100644 arch/arm/kernel/perf_callchain.c
Signature · 1 line
1.9.1
author maintainer
#1 lore raw

PATCH v2 1/9 arm: perf: factor out callchain code

The ARM callchain handling code is currently bundled with the ARM PMU management code, despite the two having no dependency on each other. This bundling has the unfortunate property of making callchain handling depend on CONFIG_HW_PERF_EVENTS, even though the callchain handling could be applied to software events in the absence of PMU hardware support.

This patch separates the two, placing the callchain handling in perf_callchain.c and making it depend on CONFIG_PERF_EVENTS rather than CONFIG_HW_PERF_EVENTS, enabling callchain recording on kernels built without hardware perf event support.

---
 arch/arm/include/asm/perf_event.h |   2 +-
 arch/arm/kernel/Makefile          |   2 +-
 arch/arm/kernel/perf_callchain.c  | 137 ++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/perf_event.c      | 132 +-----------------------------------
 4 files changed, 140 insertions(+), 133 deletions(-)
 create mode 100644 arch/arm/kernel/perf_callchain.c
arch/arm/include/asm/perf_event.h +1 -1
index c3a8369..d9cf138 100644
@@ -12,7 +12,7 @@
 #ifndef __ARM_PERF_EVENT_H__
 #define __ARM_PERF_EVENT_H__
 
-#ifdef CONFIG_HW_PERF_EVENTS
+#ifdef CONFIG_PERF_EVENTS
 struct pt_regs;
 extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
 extern unsigned long perf_misc_flags(struct pt_regs *regs);
arch/arm/kernel/Makefile +1 -1
index 38ddd9f..8dcbed5 100644
@@ -82,7 +82,7 @@ obj-$(CONFIG_CPU_MOHAWK)	+= xscale-cp0.o
 obj-$(CONFIG_CPU_PJ4)		+= pj4-cp0.o
 obj-$(CONFIG_CPU_PJ4B)		+= pj4-cp0.o
 obj-$(CONFIG_IWMMXT)		+= iwmmxt.o
-obj-$(CONFIG_PERF_EVENTS)	+= perf_regs.o
+obj-$(CONFIG_PERF_EVENTS)	+= perf_regs.o perf_callchain.o
 obj-$(CONFIG_HW_PERF_EVENTS)	+= perf_event.o perf_event_cpu.o
 AFLAGS_iwmmxt.o			:= -Wa,-mcpu=iwmmxt
 obj-$(CONFIG_ARM_CPU_TOPOLOGY)  += topology.o
arch/arm/kernel/perf_callchain.c new file +137 -0

137 lines not on this page · raw message

arch/arm/kernel/perf_event.c +1 -131

152 lines not on this page · raw message

Signature · 1 line
1.9.1
author maintainer
#2 lore raw

PATCH v2 2/9 arm: perf: add missing pr_info newlines

In reply to Mark Rutland

Most of the pr_info format strings in perf_event_cpu.c are missing newlines. Currently we get away with this as the format strings for subsequent calls to printk (including all pr_* calls) begin with a log prefix, and the printk core adds the omitted newline for this case. While generates the output we expect, we probably should not rely on the format of successive printk calls in order to get legible output.

This patch adds the missing newlines to pr_info format strings in perf_event_cpu.c, making them consistent with the format strings for other pr_info, warn, and pr_err calls, and preventing potentially illegible output if the next printk/pr_* format string doesn't begin with a log prefix.

---
 arch/arm/kernel/perf_event_cpu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/kernel/perf_event_cpu.c +4 -4
index eb2c4d5..8e901f7 100644
@@ -299,13 +299,13 @@ static int cpu_pmu_device_probe(struct platform_device *pdev)
 	int ret = -ENODEV;
 
 	if (cpu_pmu) {
-		pr_info("attempt to register multiple PMU devices!");
+		pr_info("attempt to register multiple PMU devices!\n");
 		return -ENOSPC;
 	}
 
 	pmu = kzalloc(sizeof(struct arm_pmu), GFP_KERNEL);
 	if (!pmu) {
-		pr_info("failed to allocate PMU device!");
+		pr_info("failed to allocate PMU device!\n");
 		return -ENOMEM;
 	}
 
@@ -320,7 +320,7 @@ static int cpu_pmu_device_probe(struct platform_device *pdev)
 	}
 
 	if (ret) {
-		pr_info("failed to probe PMU!");
+		pr_info("failed to probe PMU!\n");
 		goto out_free;
 	}
 
@@ -331,7 +331,7 @@ static int cpu_pmu_device_probe(struct platform_device *pdev)
 		return 0;
 
 out_free:
-	pr_info("failed to register PMU devices!");
+	pr_info("failed to register PMU devices!\n");
 	kfree(pmu);
 	return ret;
 }
Signature · 1 line
1.9.1
#3 lore raw

PATCH v2 2/9 arm: perf: add missing pr_info newlines

Quoted 7 lines · Mark Rutland

Most of the pr_info format strings in perf_event_cpu.c are missing newlines. Currently we get away with this as the format strings for subsequent calls to printk (including all pr_* calls) begin with a log prefix, and the printk core adds the omitted newline for this case. While generates the output we expect, we probably should not rely on the format of successive printk calls in order to get legible output.

Quoted 7 lines

This patch adds the missing newlines to pr_info format strings in perf_event_cpu.c, making them consistent with the format strings for other pr_info, warn, and pr_err calls, and preventing potentially illegible output if the next printk/pr_* format string doesn't begin with a log prefix.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author maintainer
#4 lore raw

PATCH v2 3/9 arm: perf: make PMU probing data-driven

In reply to Mark Rutland

The current PMU probing logic consists of a single switch statement, which means that the core arm_pmu core in perf_event_cpu.c needs to know about every CPU PMU variant supported by a driver using the arm_pmu framework. This makes it rather difficult to decouple the drivers from the (otherwise generic) probing code.

The patch refactors that switch statement to a table-driven lookup, separating the logic and knowledge (in the form of the table). Later patches will split the table across the relevant PMU drivers, which can pass their tables to the generic probing function.

---
 arch/arm/include/asm/pmu.h       | 23 ++++++++++++++++++
 arch/arm/kernel/perf_event_cpu.c | 50 +++++++++++++++-------------------------
 2 files changed, 41 insertions(+), 32 deletions(-)
arch/arm/include/asm/pmu.h +23 -0
index 0b648c5..ff39290 100644
@@ -15,6 +15,8 @@
 #include <linux/interrupt.h>
 #include <linux/perf_event.h>
 
+#include <asm/cputype.h>
+
 /*
  * struct arm_pmu_platdata - ARM PMU platform data
  *
@@ -127,6 +129,27 @@ int armpmu_map_event(struct perf_event *event,
 						[PERF_COUNT_HW_CACHE_RESULT_MAX],
 		     u32 raw_event_mask);
 
+struct pmu_probe_info {
+	unsigned int cpuid;
+	unsigned int mask;
+	int (*init)(struct arm_pmu *);
+};
+
+#define PMU_PROBE(_cpuid, _mask, _fn)	\
+{					\
+	.cpuid = (_cpuid),		\
+	.mask = (_mask),		\
+	.init = (_fn),			\
+}
+
+#define ARM_PMU_PROBE(_cpuid, _fn) \
+	PMU_PROBE(_cpuid, ARM_CPU_PART_MASK, _fn)
+
+#define ARM_PMU_XSCALE_MASK	((0xff << 24) | ARM_CPU_XSCALE_ARCH_MASK)
+
+#define XSCALE_PMU_PROBE(_version, _fn) \
+	PMU_PROBE(ARM_CPU_IMP_INTEL << 24 | _version, ARM_PMU_XSCALE_MASK, _fn)
+
 #endif /* CONFIG_HW_PERF_EVENTS */
 
 #endif /* __ARM_PMU_H__ */
arch/arm/kernel/perf_event_cpu.c +18 -32

66 lines not on this page · raw message

Signature · 1 line
1.9.1
#5 lore raw

PATCH v2 3/9 arm: perf: make PMU probing data-driven

Quoted 6 lines · Mark Rutland

The current PMU probing logic consists of a single switch statement, which means that the core arm_pmu core in perf_event_cpu.c needs to know about every CPU PMU variant supported by a driver using the arm_pmu framework. This makes it rather difficult to decouple the drivers from the (otherwise generic) probing code.

Quoted 8 lines

The patch refactors that switch statement to a table-driven lookup, separating the logic and knowledge (in the form of the table). Later patches will split the table across the relevant PMU drivers, which can pass their tables to the generic probing function.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author maintainer
#6 lore raw

PATCH v2 4/9 arm: perf: use IDR types for CPU PMUs

In reply to Mark Rutland

For systems with heterogeneous CPUs (e.g. big.LITTLE systems) the PMUs can be different in each cluster, and not all events can be migrated between clusters. To allow userspace to deal with this, it must be possible to address each PMU independently.

This patch changes PMUs to be registered with dynamic (IDR) types, allowing them to be targeted individually. Each PMU's type can be found in ${SYSFS_ROOT}/bus/event_source/devices/${PMU_NAME}/type.

Quoted 1 line

From userspace, raw events can be targeted at a specific PMU:

$ perf stat -e ${PMU_NAME}/config=V,config1=V1,.../

Doing this does not break existing tools which use existing perf types:
when perf core can't find a PMU of matching type (in perf_init_event) it'll iterate over the set of all PMUs. If a compatible PMU exists, it'll be found eventually. If more than one compatible PMU exists, the event will be handled by whichever PMU happens to be earlier in the pmus list (which currently will be the last compatible PMU registered).

---
 arch/arm/kernel/perf_event.c     | 6 +++++-
 arch/arm/kernel/perf_event_cpu.c | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)
arch/arm/kernel/perf_event.c +5 -1
index ae96b98..7ffb267 100644
@@ -77,8 +77,12 @@ armpmu_map_event(struct perf_event *event,
 		 u32 raw_event_mask)
 {
 	u64 config = event->attr.config;
+	int type = event->attr.type;
 
-	switch (event->attr.type) {
+	if (type == event->pmu->type)
+		return armpmu_map_raw_event(raw_event_mask, config);
+
+	switch (type) {
 	case PERF_TYPE_HARDWARE:
 		return armpmu_map_hw_event(event_map, config);
 	case PERF_TYPE_HW_CACHE:
arch/arm/kernel/perf_event_cpu.c +1 -1
index 64127c6..64adf397 100644
@@ -311,7 +311,7 @@ static int cpu_pmu_device_probe(struct platform_device *pdev)
 	}
 
 	cpu_pmu_init(cpu_pmu);
-	ret = armpmu_register(cpu_pmu, PERF_TYPE_RAW);
+	ret = armpmu_register(cpu_pmu, -1);
 
 	if (!ret)
 		return 0;
Signature · 1 line
1.9.1
author maintainer
#7 lore raw

PATCH v2 5/9 arm: perf: limit size of accounting data

In reply to Mark Rutland

Commit 3fc2c83087 (ARM: perf: remove event limit from pmu_hw_events) got rid of the upper limit on the number of events an arm_pmu could handle, but introduced additional complexity and places a burden on each PMU driver to allocate accounting data somehow. So far this has not generally been useful as the only users of arm_pmu are the CPU backend and the CCI driver.

Now that the CCI driver plugs into the perf subsystem directly, we can remove some of the complexities that get in the way of supporting heterogeneous CPU PMUs.

This patch restores the original limits on pmu_hw_events fields such that the pmu_hw_events data can be allocated as a contiguous block. This will simplify dynamic pmu_hw_events allocation in later patches.

---
 arch/arm/include/asm/pmu.h       | 4 ++--
 arch/arm/kernel/perf_event.c     | 4 +---
 arch/arm/kernel/perf_event_cpu.c | 4 ----
 3 files changed, 3 insertions(+), 9 deletions(-)
arch/arm/include/asm/pmu.h +2 -2
index ff39290..3d7e30b 100644
@@ -68,13 +68,13 @@ struct pmu_hw_events {
 	/*
 	 * The events that are active on the PMU for the given index.
 	 */
-	struct perf_event	**events;
+	struct perf_event	*events[ARMPMU_MAX_HWEVENTS];
 
 	/*
 	 * A 1 bit for an index indicates that the counter is being used for
 	 * an event. A 0 means that the counter can be used.
 	 */
-	unsigned long           *used_mask;
+	DECLARE_BITMAP(used_mask, ARMPMU_MAX_HWEVENTS);
 
 	/*
 	 * Hardware lock to serialize accesses to PMU registers. Needed for the
arch/arm/kernel/perf_event.c +1 -3
index 7ffb267..8648107 100644
@@ -275,14 +275,12 @@ validate_group(struct perf_event *event)
 {
 	struct perf_event *sibling, *leader = event->group_leader;
 	struct pmu_hw_events fake_pmu;
-	DECLARE_BITMAP(fake_used_mask, ARMPMU_MAX_HWEVENTS);
 
 	/*
 	 * Initialise the fake PMU. We only need to populate the
 	 * used_mask for the purposes of validation.
 	 */
-	memset(fake_used_mask, 0, sizeof(fake_used_mask));
-	fake_pmu.used_mask = fake_used_mask;
+	memset(&fake_pmu.used_mask, 0, sizeof(fake_pmu.used_mask));
 
 	if (!validate_event(&fake_pmu, leader))
 		return -EINVAL;
arch/arm/kernel/perf_event_cpu.c +0 -4
index 64adf397..1528d3c 100644
@@ -36,8 +36,6 @@
 static struct arm_pmu *cpu_pmu;
 
 static DEFINE_PER_CPU(struct arm_pmu *, percpu_pmu);
-static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
-static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
 static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
 
 /*
@@ -172,8 +170,6 @@ static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	int cpu;
 	for_each_possible_cpu(cpu) {
 		struct pmu_hw_events *events = &per_cpu(cpu_hw_events, cpu);
-		events->events = per_cpu(hw_events, cpu);
-		events->used_mask = per_cpu(used_mask, cpu);
 		raw_spin_lock_init(&events->pmu_lock);
 		per_cpu(percpu_pmu, cpu) = cpu_pmu;
 	}
Signature · 1 line
1.9.1
#8 lore raw

PATCH v2 5/9 arm: perf: limit size of accounting data

Quoted 11 lines · Mark Rutland

Commit 3fc2c83087 (ARM: perf: remove event limit from pmu_hw_events) got rid of the upper limit on the number of events an arm_pmu could handle, but introduced additional complexity and places a burden on each PMU driver to allocate accounting data somehow. So far this has not generally been useful as the only users of arm_pmu are the CPU backend and the CCI driver.

Now that the CCI driver plugs into the perf subsystem directly, we can remove some of the complexities that get in the way of supporting heterogeneous CPU PMUs.

Quoted 7 lines

This patch restores the original limits on pmu_hw_events fields such that the pmu_hw_events data can be allocated as a contiguous block. This will simplify dynamic pmu_hw_events allocation in later patches.

I ran it through some Krait specific events and it looks ok.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author maintainer
#9 lore raw

PATCH v2 6/9 arm: perf: kill get_hw_events()

In reply to Mark Rutland

Now that the arm pmu code is limited to CPU PMUs the get_hw_events() function is superfluous, as we'll always have a set of per-cpu pmu_hw_events structures.

This patch removes the get_hw_events() function, replacing it with a percpu hw_events pointer. Uses of get_hw_events are updated to use this_cpu_ptr.

---
 arch/arm/include/asm/pmu.h          |  2 +-
 arch/arm/kernel/perf_event.c        |  6 +++---
 arch/arm/kernel/perf_event_cpu.c    |  7 +------
 arch/arm/kernel/perf_event_v6.c     | 12 ++++++------
 arch/arm/kernel/perf_event_v7.c     | 14 +++++++-------
 arch/arm/kernel/perf_event_xscale.c | 20 ++++++++++----------
 6 files changed, 28 insertions(+), 33 deletions(-)
arch/arm/include/asm/pmu.h +1 -1
index 3d7e30b..f273dd2 100644
@@ -109,7 +109,7 @@ struct arm_pmu {
 	struct mutex	reserve_mutex;
 	u64		max_period;
 	struct platform_device	*plat_device;
-	struct pmu_hw_events	*(*get_hw_events)(void);
+	struct pmu_hw_events	__percpu *hw_events;
 };
 
 #define to_arm_pmu(p) (container_of(p, struct arm_pmu, pmu))
arch/arm/kernel/perf_event.c +3 -3
index 8648107..05ac5ee 100644
@@ -201,7 +201,7 @@ static void
 armpmu_del(struct perf_event *event, int flags)
 {
 	struct arm_pmu *armpmu = to_arm_pmu(event->pmu);
-	struct pmu_hw_events *hw_events = armpmu->get_hw_events();
+	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
 	struct hw_perf_event *hwc = &event->hw;
 	int idx = hwc->idx;
 
@@ -218,7 +218,7 @@ static int
 armpmu_add(struct perf_event *event, int flags)
 {
 	struct arm_pmu *armpmu = to_arm_pmu(event->pmu);
-	struct pmu_hw_events *hw_events = armpmu->get_hw_events();
+	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
 	struct hw_perf_event *hwc = &event->hw;
 	int idx;
 	int err = 0;
@@ -467,7 +467,7 @@ static int armpmu_event_init(struct perf_event *event)
 static void armpmu_enable(struct pmu *pmu)
 {
 	struct arm_pmu *armpmu = to_arm_pmu(pmu);
-	struct pmu_hw_events *hw_events = armpmu->get_hw_events();
+	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
 	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
 
 	if (enabled)
arch/arm/kernel/perf_event_cpu.c +1 -6
index 1528d3c..d3fa1cc 100644
@@ -67,11 +67,6 @@ EXPORT_SYMBOL_GPL(perf_num_counters);
 #include "perf_event_v6.c"
 #include "perf_event_v7.c"
 
-static struct pmu_hw_events *cpu_pmu_get_cpu_events(void)
-{
-	return this_cpu_ptr(&cpu_hw_events);
-}
-
 static void cpu_pmu_enable_percpu_irq(void *data)
 {
 	int irq = *(int *)data;
@@ -174,7 +169,7 @@ static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
 		per_cpu(percpu_pmu, cpu) = cpu_pmu;
 	}
 
-	cpu_pmu->get_hw_events	= cpu_pmu_get_cpu_events;
+	cpu_pmu->hw_events	= &cpu_hw_events;
 	cpu_pmu->request_irq	= cpu_pmu_request_irq;
 	cpu_pmu->free_irq	= cpu_pmu_free_irq;
 
arch/arm/kernel/perf_event_v6.c +6 -6
index abfeb04..f2ffd5c 100644
@@ -262,7 +262,7 @@ static void armv6pmu_enable_event(struct perf_event *event)
 	unsigned long val, mask, evt, flags;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 	int idx = hwc->idx;
 
 	if (ARMV6_CYCLE_COUNTER == idx) {
@@ -300,7 +300,7 @@ armv6pmu_handle_irq(int irq_num,
 	unsigned long pmcr = armv6_pmcr_read();
 	struct perf_sample_data data;
 	struct arm_pmu *cpu_pmu = (struct arm_pmu *)dev;
-	struct pmu_hw_events *cpuc = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *cpuc = this_cpu_ptr(cpu_pmu->hw_events);
 	struct pt_regs *regs;
 	int idx;
 
@@ -356,7 +356,7 @@ armv6pmu_handle_irq(int irq_num,
 static void armv6pmu_start(struct arm_pmu *cpu_pmu)
 {
 	unsigned long flags, val;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	raw_spin_lock_irqsave(&events->pmu_lock, flags);
 	val = armv6_pmcr_read();
@@ -368,7 +368,7 @@ static void armv6pmu_start(struct arm_pmu *cpu_pmu)
 static void armv6pmu_stop(struct arm_pmu *cpu_pmu)
 {
 	unsigned long flags, val;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	raw_spin_lock_irqsave(&events->pmu_lock, flags);
 	val = armv6_pmcr_read();
@@ -409,7 +409,7 @@ static void armv6pmu_disable_event(struct perf_event *event)
 	unsigned long val, mask, evt, flags;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 	int idx = hwc->idx;
 
 	if (ARMV6_CYCLE_COUNTER == idx) {
@@ -444,7 +444,7 @@ static void armv6mpcore_pmu_disable_event(struct perf_event *event)
 	unsigned long val, mask, flags, evt = 0;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 	int idx = hwc->idx;
 
 	if (ARMV6_CYCLE_COUNTER == idx) {
arch/arm/kernel/perf_event_v7.c +7 -7
index 116758b..fa76b25 100644
@@ -705,7 +705,7 @@ static void armv7pmu_enable_event(struct perf_event *event)
 	unsigned long flags;
 	struct hw_perf_event *hwc = &event->hw;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 	int idx = hwc->idx;
 
 	if (!armv7_pmnc_counter_valid(cpu_pmu, idx)) {
@@ -751,7 +751,7 @@ static void armv7pmu_disable_event(struct perf_event *event)
 	unsigned long flags;
 	struct hw_perf_event *hwc = &event->hw;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 	int idx = hwc->idx;
 
 	if (!armv7_pmnc_counter_valid(cpu_pmu, idx)) {
@@ -783,7 +783,7 @@ static irqreturn_t armv7pmu_handle_irq(int irq_num, void *dev)
 	u32 pmnc;
 	struct perf_sample_data data;
 	struct arm_pmu *cpu_pmu = (struct arm_pmu *)dev;
-	struct pmu_hw_events *cpuc = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *cpuc = this_cpu_ptr(cpu_pmu->hw_events);
 	struct pt_regs *regs;
 	int idx;
 
@@ -843,7 +843,7 @@ static irqreturn_t armv7pmu_handle_irq(int irq_num, void *dev)
 static void armv7pmu_start(struct arm_pmu *cpu_pmu)
 {
 	unsigned long flags;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	raw_spin_lock_irqsave(&events->pmu_lock, flags);
 	/* Enable all counters */
@@ -854,7 +854,7 @@ static void armv7pmu_start(struct arm_pmu *cpu_pmu)
 static void armv7pmu_stop(struct arm_pmu *cpu_pmu)
 {
 	unsigned long flags;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	raw_spin_lock_irqsave(&events->pmu_lock, flags);
 	/* Disable all counters */
@@ -1287,7 +1287,7 @@ static void krait_pmu_disable_event(struct perf_event *event)
 	struct hw_perf_event *hwc = &event->hw;
 	int idx = hwc->idx;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	/* Disable counter and interrupt */
 	raw_spin_lock_irqsave(&events->pmu_lock, flags);
@@ -1313,7 +1313,7 @@ static void krait_pmu_enable_event(struct perf_event *event)
 	struct hw_perf_event *hwc = &event->hw;
 	int idx = hwc->idx;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	/*
 	 * Enable counter and interrupt, and set the counter to count
arch/arm/kernel/perf_event_xscale.c +10 -10
index 08da0af..8af9f1f 100644
@@ -138,7 +138,7 @@ xscale1pmu_handle_irq(int irq_num, void *dev)
 	unsigned long pmnc;
 	struct perf_sample_data data;
 	struct arm_pmu *cpu_pmu = (struct arm_pmu *)dev;
-	struct pmu_hw_events *cpuc = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *cpuc = this_cpu_ptr(cpu_pmu->hw_events);
 	struct pt_regs *regs;
 	int idx;
 
@@ -198,7 +198,7 @@ static void xscale1pmu_enable_event(struct perf_event *event)
 	unsigned long val, mask, evt, flags;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 	int idx = hwc->idx;
 
 	switch (idx) {
@@ -234,7 +234,7 @@ static void xscale1pmu_disable_event(struct perf_event *event)
 	unsigned long val, mask, evt, flags;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 	int idx = hwc->idx;
 
 	switch (idx) {
@@ -287,7 +287,7 @@ xscale1pmu_get_event_idx(struct pmu_hw_events *cpuc,
 static void xscale1pmu_start(struct arm_pmu *cpu_pmu)
 {
 	unsigned long flags, val;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	raw_spin_lock_irqsave(&events->pmu_lock, flags);
 	val = xscale1pmu_read_pmnc();
@@ -299,7 +299,7 @@ static void xscale1pmu_start(struct arm_pmu *cpu_pmu)
 static void xscale1pmu_stop(struct arm_pmu *cpu_pmu)
 {
 	unsigned long flags, val;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	raw_spin_lock_irqsave(&events->pmu_lock, flags);
 	val = xscale1pmu_read_pmnc();
@@ -485,7 +485,7 @@ xscale2pmu_handle_irq(int irq_num, void *dev)
 	unsigned long pmnc, of_flags;
 	struct perf_sample_data data;
 	struct arm_pmu *cpu_pmu = (struct arm_pmu *)dev;
-	struct pmu_hw_events *cpuc = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *cpuc = this_cpu_ptr(cpu_pmu->hw_events);
 	struct pt_regs *regs;
 	int idx;
 
@@ -539,7 +539,7 @@ static void xscale2pmu_enable_event(struct perf_event *event)
 	unsigned long flags, ien, evtsel;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 	int idx = hwc->idx;
 
 	ien = xscale2pmu_read_int_enable();
@@ -585,7 +585,7 @@ static void xscale2pmu_disable_event(struct perf_event *event)
 	unsigned long flags, ien, evtsel, of_flags;
 	struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
 	struct hw_perf_event *hwc = &event->hw;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 	int idx = hwc->idx;
 
 	ien = xscale2pmu_read_int_enable();
@@ -651,7 +651,7 @@ out:
 static void xscale2pmu_start(struct arm_pmu *cpu_pmu)
 {
 	unsigned long flags, val;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	raw_spin_lock_irqsave(&events->pmu_lock, flags);
 	val = xscale2pmu_read_pmnc() & ~XSCALE_PMU_CNT64;
@@ -663,7 +663,7 @@ static void xscale2pmu_start(struct arm_pmu *cpu_pmu)
 static void xscale2pmu_stop(struct arm_pmu *cpu_pmu)
 {
 	unsigned long flags, val;
-	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
 
 	raw_spin_lock_irqsave(&events->pmu_lock, flags);
 	val = xscale2pmu_read_pmnc();
Signature · 1 line
1.9.1
#10 lore raw

PATCH v2 6/9 arm: perf: kill get_hw_events()

Quoted 11 lines · Mark Rutland

Now that the arm pmu code is limited to CPU PMUs the get_hw_events() function is superfluous, as we'll always have a set of per-cpu pmu_hw_events structures.

This patch removes the get_hw_events() function, replacing it with a percpu hw_events pointer. Uses of get_hw_events are updated to use this_cpu_ptr.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author maintainer
#11 lore raw

PATCH v2 7/9 arm: perf: fold percpu_pmu into pmu_hw_events

In reply to Mark Rutland

Currently the percpu_pmu pointers used as percpu_irq dev_id values are defined separately from the other per-cpu accounting data, which make dynamically allocating the data (as will be required for systems with heterogeneous CPUs) difficult.

This patch moves the percpu_pmu pointers into pmu_hw_events (which is itself allocated per cpu), which will allow for easier dynamic allocation. Both percpu and regular irqs are requested using percpu_pmu pointers as tokens, freeing us from having to know whether an irq is percpu within the handler, and thus avoiding a radix tree lookup on the handler path.

---
 arch/arm/include/asm/pmu.h       |  6 ++++++
 arch/arm/kernel/perf_event.c     | 14 +++++++++-----
 arch/arm/kernel/perf_event_cpu.c | 14 ++++++++------
 3 files changed, 23 insertions(+), 11 deletions(-)
arch/arm/include/asm/pmu.h +6 -0
index f273dd2..cc01498 100644
@@ -81,6 +81,12 @@ struct pmu_hw_events {
 	 * read/modify/write sequences.
 	 */
 	raw_spinlock_t		pmu_lock;
+
+	/*
+	 * When using percpu IRQs, we need a percpu dev_id. Place it here as we
+	 * already have to allocate this struct per cpu.
+	 */
+	struct arm_pmu		*percpu_pmu;
 };
 
 struct arm_pmu {
arch/arm/kernel/perf_event.c +9 -5
index 05ac5ee..e34934f 100644
@@ -304,17 +304,21 @@ static irqreturn_t armpmu_dispatch_irq(int irq, void *dev)
 	int ret;
 	u64 start_clock, finish_clock;
 
-	if (irq_is_percpu(irq))
-		dev = *(void **)dev;
-	armpmu = dev;
+	/*
+	 * we request the IRQ with a (possibly percpu) struct arm_pmu**, but
+	 * the handlers expect a struct arm_pmu*. The percpu_irq framework will
+	 * do any necessary shifting, we just need to perform the first
+	 * dereference.
+	 */
+	armpmu = *(void **)dev;
 	plat_device = armpmu->plat_device;
 	plat = dev_get_platdata(&plat_device->dev);
 
 	start_clock = sched_clock();
 	if (plat && plat->handle_irq)
-		ret = plat->handle_irq(irq, dev, armpmu->handle_irq);
+		ret = plat->handle_irq(irq, armpmu, armpmu->handle_irq);
 	else
-		ret = armpmu->handle_irq(irq, dev);
+		ret = armpmu->handle_irq(irq, armpmu);
 	finish_clock = sched_clock();
 
 	perf_sample_event_took(finish_clock - start_clock);
arch/arm/kernel/perf_event_cpu.c +8 -6
index d3fa1cc..5eecfe9 100644
@@ -35,7 +35,6 @@
 /* Set at runtime when we know what CPU type we are. */
 static struct arm_pmu *cpu_pmu;
 
-static DEFINE_PER_CPU(struct arm_pmu *, percpu_pmu);
 static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
 
 /*
@@ -85,20 +84,21 @@ static void cpu_pmu_free_irq(struct arm_pmu *cpu_pmu)
 {
 	int i, irq, irqs;
 	struct platform_device *pmu_device = cpu_pmu->plat_device;
+	struct pmu_hw_events __percpu *hw_events = cpu_pmu->hw_events;
 
 	irqs = min(pmu_device->num_resources, num_possible_cpus());
 
 	irq = platform_get_irq(pmu_device, 0);
 	if (irq >= 0 && irq_is_percpu(irq)) {
 		on_each_cpu(cpu_pmu_disable_percpu_irq, &irq, 1);
-		free_percpu_irq(irq, &percpu_pmu);
+		free_percpu_irq(irq, &hw_events->percpu_pmu);
 	} else {
 		for (i = 0; i < irqs; ++i) {
 			if (!cpumask_test_and_clear_cpu(i, &cpu_pmu->active_irqs))
 				continue;
 			irq = platform_get_irq(pmu_device, i);
 			if (irq >= 0)
-				free_irq(irq, cpu_pmu);
+				free_irq(irq, per_cpu_ptr(&hw_events->percpu_pmu, i));
 		}
 	}
 }
@@ -107,6 +107,7 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 {
 	int i, err, irq, irqs;
 	struct platform_device *pmu_device = cpu_pmu->plat_device;
+	struct pmu_hw_events __percpu *hw_events = cpu_pmu->hw_events;
 
 	if (!pmu_device)
 		return -ENODEV;
@@ -119,7 +120,8 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 
 	irq = platform_get_irq(pmu_device, 0);
 	if (irq >= 0 && irq_is_percpu(irq)) {
-		err = request_percpu_irq(irq, handler, "arm-pmu", &percpu_pmu);
+		err = request_percpu_irq(irq, handler, "arm-pmu",
+					 &hw_events->percpu_pmu);
 		if (err) {
 			pr_err("unable to request IRQ%d for ARM PMU counters\n",
 				irq);
@@ -146,7 +148,7 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 
 			err = request_irq(irq, handler,
 					  IRQF_NOBALANCING | IRQF_NO_THREAD, "arm-pmu",
-					  cpu_pmu);
+					  per_cpu_ptr(&hw_events->percpu_pmu, i));
 			if (err) {
 				pr_err("unable to request IRQ%d for ARM PMU counters\n",
 					irq);
@@ -166,7 +168,7 @@ static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	for_each_possible_cpu(cpu) {
 		struct pmu_hw_events *events = &per_cpu(cpu_hw_events, cpu);
 		raw_spin_lock_init(&events->pmu_lock);
-		per_cpu(percpu_pmu, cpu) = cpu_pmu;
+		events->percpu_pmu = cpu_pmu;
 	}
 
 	cpu_pmu->hw_events	= &cpu_hw_events;
Signature · 1 line
1.9.1
author maintainer
#12 lore raw

PATCH v2 8/9 arm: perf: dynamically allocate cpu hardware data

In reply to Mark Rutland

To support multiple PMUs, each PMU will need its own accounting data. As we don't know how (in general) many PMUs we'll have to support at compile-time, we must allocate the data at runtime dynamically

---
 arch/arm/kernel/perf_event_cpu.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)
arch/arm/kernel/perf_event_cpu.c +25 -8
index 5eecfe9..6e550cf 100644
@@ -35,8 +35,6 @@
 /* Set at runtime when we know what CPU type we are. */
 static struct arm_pmu *cpu_pmu;
 
-static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
-
 /*
  * Despite the names, these two functions are CPU-specific and are used
  * by the OProfile/perf code.
@@ -162,16 +160,22 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 	return 0;
 }
 
-static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
+static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	int cpu;
+	struct pmu_hw_events __percpu *cpu_hw_events;
+
+	cpu_hw_events = alloc_percpu(struct pmu_hw_events);
+	if (!cpu_hw_events)
+		return -ENOMEM;
+
 	for_each_possible_cpu(cpu) {
-		struct pmu_hw_events *events = &per_cpu(cpu_hw_events, cpu);
+		struct pmu_hw_events *events = per_cpu_ptr(cpu_hw_events, cpu);
 		raw_spin_lock_init(&events->pmu_lock);
 		events->percpu_pmu = cpu_pmu;
 	}
 
-	cpu_pmu->hw_events	= &cpu_hw_events;
+	cpu_pmu->hw_events	= cpu_hw_events;
 	cpu_pmu->request_irq	= cpu_pmu_request_irq;
 	cpu_pmu->free_irq	= cpu_pmu_free_irq;
 
@@ -182,6 +186,13 @@ static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	/* If no interrupts available, set the corresponding capability flag */
 	if (!platform_get_irq(cpu_pmu->plat_device, 0))
 		cpu_pmu->pmu.capabilities |= PERF_PMU_CAP_NO_INTERRUPT;
+
+	return 0;
+}
+
+static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
+{
+	free_percpu(cpu_pmu->hw_events);
 }
 
 /*
@@ -303,12 +314,18 @@ static int cpu_pmu_device_probe(struct platform_device *pdev)
 		goto out_free;
 	}
 
-	cpu_pmu_init(cpu_pmu);
+	ret = cpu_pmu_init(cpu_pmu);
+	if (ret)
+		goto out_free;
+
 	ret = armpmu_register(cpu_pmu, -1);
+	if (ret)
+		goto out_destroy;
 
-	if (!ret)
-		return 0;
+	return 0;
 
+out_destroy:
+	cpu_pmu_destroy(cpu_pmu);
 out_free:
 	pr_info("failed to register PMU devices!\n");
 	kfree(pmu);
Signature · 1 line
1.9.1
#13 lore raw

PATCH v2 8/9 arm: perf: dynamically allocate cpu hardware data

Quoted 5 lines · Mark Rutland

To support multiple PMUs, each PMU will need its own accounting data. As we don't know how (in general) many PMUs we'll have to support at compile-time, we must allocate the data at runtime dynamically

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author maintainer
#14 lore raw

PATCH v2 9/9 arm: perf: fold hotplug notifier into arm_pmu

In reply to Mark Rutland

Handling multiple PMUs using a single hotplug notifier requires a list of PMUs to be maintained, with synchronisation in the probe, remove, and notify paths. This is error-prone and makes the code much harder to maintain.

Instead of using a single notifier, we can dynamically allocate a notifier block per-PMU. The end result is the same, but the list of PMUs is implicit in the hotplug notifier list rather than within a perf-local data structure, which makes the code far easier to handle.

---
 arch/arm/include/asm/pmu.h       |  1 +
 arch/arm/kernel/perf_event_cpu.c | 69 ++++++++++++++++++++--------------------
 2 files changed, 35 insertions(+), 35 deletions(-)
arch/arm/include/asm/pmu.h +1 -0
index cc01498..b1596bd 100644
@@ -116,6 +116,7 @@ struct arm_pmu {
 	u64		max_period;
 	struct platform_device	*plat_device;
 	struct pmu_hw_events	__percpu *hw_events;
+	struct notifier_block	hotplug_nb;
 };
 
 #define to_arm_pmu(p) (container_of(p, struct arm_pmu, pmu))
arch/arm/kernel/perf_event_cpu.c +34 -35

102 lines not on this page · raw message

Signature · 1 line
1.9.1
#15 lore raw

PATCH v2 9/9 arm: perf: fold hotplug notifier into arm_pmu

Quoted 11 lines · Mark Rutland

Handling multiple PMUs using a single hotplug notifier requires a list of PMUs to be maintained, with synchronisation in the probe, remove, and notify paths. This is error-prone and makes the code much harder to maintain.

Instead of using a single notifier, we can dynamically allocate a notifier block per-PMU. The end result is the same, but the list of PMUs is implicit in the hotplug notifier list rather than within a perf-local data structure, which makes the code far easier to handle.

One nit below.

Quoted 18 lines
arch/arm/kernel/perf_event_cpu.c +7 -0
@@ -169,6 +192,11 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	if (!cpu_hw_events)
 		return -ENOMEM;
 
+	cpu_pmu->hotplug_nb.notifier_call = cpu_pmu_notify;
+	err = register_cpu_notifier(&cpu_pmu->hotplug_nb);
+	if (err)
+		goto out_hw_events;
+
 	for_each_possible_cpu(cpu) {
 		struct pmu_hw_events *events = per_cpu_ptr(cpu_hw_events, cpu);
 		raw_spin_lock_init(&events->pmu_lock);
@@ -188,38 +216,19 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 		cpu_pmu->pmu.capabilities |= PERF_PMU_CAP_NO_INTERRUPT;
 
 	return 0;
+
+out_hw_events:

arch/arm/kernel/perf_event_cpu.c static int cpu_pmu_init(struct arm_pmu *cpu_pmu) · line 230

Quoted 10 lines
arch/arm/kernel/perf_event_cpu.c +3 -0
+	free_percpu(cpu_hw_events);
+	return err;
 }
 
 static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
 {
 	free_percpu(cpu_pmu->hw_events);
+	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
 }
 

I would expect the order to be the other way, but it probably doesn't matter all that much on the registration error path.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author maintainer
#16 lore raw

PATCH v2 9/9 arm: perf: fold hotplug notifier into arm_pmu

Quoted 39 lines · Stephen Boyd
Quoted 11 lines · Mark Rutland

Handling multiple PMUs using a single hotplug notifier requires a list of PMUs to be maintained, with synchronisation in the probe, remove, and notify paths. This is error-prone and makes the code much harder to maintain.

Instead of using a single notifier, we can dynamically allocate a notifier block per-PMU. The end result is the same, but the list of PMUs is implicit in the hotplug notifier list rather than within a perf-local data structure, which makes the code far easier to handle.

One nit below.

Quoted 22 lines
arch/arm/kernel/perf_event_cpu.c +9 -0
@@ -169,6 +192,11 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	if (!cpu_hw_events)
 		return -ENOMEM;
 
+	cpu_pmu->hotplug_nb.notifier_call = cpu_pmu_notify;
+	err = register_cpu_notifier(&cpu_pmu->hotplug_nb);
+	if (err)
+		goto out_hw_events;
+
 	for_each_possible_cpu(cpu) {
 		struct pmu_hw_events *events = per_cpu_ptr(cpu_hw_events, cpu);
 		raw_spin_lock_init(&events->pmu_lock);
@@ -188,38 +216,19 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 		cpu_pmu->pmu.capabilities |= PERF_PMU_CAP_NO_INTERRUPT;
 
 	return 0;
+
+out_hw_events:
+	free_percpu(cpu_hw_events);
+	return err;
 }
 
Quoted 9 lines

arch/arm/kernel/perf_event_cpu.c static int cpu_pmu_init(struct arm_pmu *cpu_pmu) · line 230

Quoted 6 lines
arch/arm/kernel/perf_event_cpu.c +1 -0
 static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
 {
 	free_percpu(cpu_pmu->hw_events);
+	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
 }
 

I would expect the order to be the other way, but it probably doesn't matter all that much on the registration error path.

I've flipped this around locally, I agree it's nicer the other way around.

Thanks for the review, it's much appreciated.

Thanks,
Mark.