Skip to contentSkip to conversation

arm64: kernel: perf: add cpu hotplug notifier

RFC PATCH New Lorenzo Pieralisi 4 participants 10 messages Mar 10, 2015 – Mar 31, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/2 arm64: kernel: perf: add cpu hotplug notifier

When a CPU is taken offline, its PMU registers content is lost and needs to be reset on power up, since for most of the PMU registers content is UNKNOWN upon CPU reset. This patch implements a cpu hotplug notifier and hooks the reset call in the respective notifier callback function.

---
 arch/arm64/kernel/perf_event.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
arch/arm64/kernel/perf_event.c +27 -0
index 25a5308..83f21d8 100644
@@ -21,6 +21,7 @@
 #define pr_fmt(fmt) "hw perfevents: " fmt
 
 #include <linux/bitmap.h>
+#include <linux/cpu.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
@@ -1315,6 +1316,30 @@ static struct pmu_hw_events *armpmu_get_cpu_events(void)
 	return this_cpu_ptr(&cpu_hw_events);
 }
 
+/*
+ * PMU hardware loses all context when a CPU goes offline.
+ * When a CPU is hotplugged back in, since some hardware registers are
+ * UNKNOWN at reset, the PMU must be explicitly reset to avoid reading
+ * junk values out of them.
+ */
+static int cpu_pmu_notifier(struct notifier_block *b, unsigned long action,
+			    void *hcpu)
+{
+	if ((action & ~CPU_TASKS_FROZEN) != CPU_STARTING)
+		return NOTIFY_DONE;
+
+	if (cpu_pmu->reset)
+		cpu_pmu->reset(NULL);
+	else
+		return NOTIFY_DONE;
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block cpu_pmu_notifier_block = {
+	.notifier_call = cpu_pmu_notifier,
+};
+
 static void __init cpu_pmu_init(struct arm_pmu *armpmu)
 {
 	int cpu;
@@ -1325,6 +1350,8 @@ static void __init cpu_pmu_init(struct arm_pmu *armpmu)
 		raw_spin_lock_init(&events->pmu_lock);
 	}
 	armpmu->get_hw_events = armpmu_get_cpu_events;
+
+	register_cpu_notifier(&cpu_pmu_notifier_block);
 }
 
 static int __init init_hw_perf_events(void)
Signature · 1 line
2.2.1
#1 lore raw

RFC PATCH 2/2 arm64: kernel: perf: add pmu CPU PM notifier

When a CPU is being profiled through PMU events and it enters suspend or idle states, the PMU registers content can be lost, which means that counters that were relied upon on power down entry are reset on power up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects on entry if events are being monitored, and if so, it returns failure to the CPU PM notification chain, causing the suspend thread or the idle thread to abort power down, therefore preventing registers content loss.

By triggering CPU PM notification failure this patch prevents suspending a system if the suspend thread is being profiled and it also prevents entering idle deep states on cores that have profile events in use, somehow limiting power management capabilities when there are active perf sessions.

Cc Will Deacon <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Sudeep Holla <[email protected]>
Cc Daniel Lezcano <[email protected]>
Cc Mathieu Poirier <[email protected]>
Cc Mark Rutland <[email protected]>
---
 arch/arm64/kernel/perf_event.c | 50 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)
arch/arm64/kernel/perf_event.c +50 -0
index 83f21d8..4809fca 100644
@@ -22,6 +22,7 @@
 
 #include <linux/bitmap.h>
 #include <linux/cpu.h>
+#include <linux/cpu_pm.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
@@ -1316,6 +1317,53 @@ static struct pmu_hw_events *armpmu_get_cpu_events(void)
 	return this_cpu_ptr(&cpu_hw_events);
 }
 
+#ifdef CONFIG_CPU_PM
+static int cpu_pm_pmu_notifier(struct notifier_block *self,
+			       unsigned long cmd, void *v)
+{
+	switch (cmd) {
+	case CPU_PM_ENTER: {
+		/*
+		 * Check events in use and fail if events are being monitored.
+		 * On failure, system suspend and idle entry will consequently
+		 * abort power down operations, preventing CPU reset on power
+		 * down, that would cause PMU registers reset in turn.
+		 * The obvious drawback is that we can't enter idle
+		 * states if a cpu is being profiled, and we can't
+		 * profile the suspend thread (ie profiling it has the side
+		 * effect of disabling system suspend), but that's the price
+		 * to pay to keep the PMU registers powered up.
+		 */
+		struct pmu_hw_events *hw_events = cpu_pmu->get_hw_events();
+		int enabled = bitmap_weight(hw_events->used_mask,
+					    cpu_pmu->num_events);
+		return enabled ? NOTIFY_BAD : NOTIFY_OK;
+	}
+	case CPU_PM_EXIT:
+		if (cpu_pmu->reset)
+			cpu_pmu->reset(NULL);
+		return NOTIFY_OK;
+	case CPU_PM_ENTER_FAILED:
+	default:
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block cpu_pm_pmu_notifier_block = {
+	.notifier_call = cpu_pm_pmu_notifier,
+};
+
+static void cpu_pm_pmu_init(void)
+{
+	cpu_pm_register_notifier(&cpu_pm_pmu_notifier_block);
+}
+
+#else
+static inline void cpu_pm_pmu_init(void) { }
+#endif /* CONFIG_CPU_PM */
+
 /*
  * PMU hardware loses all context when a CPU goes offline.
  * When a CPU is hotplugged back in, since some hardware registers are
@@ -1352,6 +1400,8 @@ static void __init cpu_pmu_init(struct arm_pmu *armpmu)
 	armpmu->get_hw_events = armpmu_get_cpu_events;
 
 	register_cpu_notifier(&cpu_pmu_notifier_block);
+
+	cpu_pm_pmu_init();
 }
 
 static int __init init_hw_perf_events(void)
Signature · 1 line
2.2.1
#2 lore raw

RFC PATCH 2/2 arm64: kernel: perf: add pmu CPU PM notifier

Quoted 11 lines · Lorenzo Pieralisi

When a CPU is being profiled through PMU events and it enters suspend or idle states, the PMU registers content can be lost, which means that counters that were relied upon on power down entry are reset on power up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects on entry if events are being monitored, and if so, it returns failure to the CPU PM notification chain, causing the suspend thread or the idle thread to abort power down, therefore preventing registers content loss.

Quoted 5 lines

By triggering CPU PM notification failure this patch prevents suspending a system if the suspend thread is being profiled and it also prevents entering idle deep states on cores that have profile events in use, somehow limiting power management capabilities when there are active perf sessions.

I guess that's one choice. Couldn't you also stop the PMU and save/restore it's context in the notifiers? so that you wouldn't affect PM capabilities?

That would imply that you lose the ability to profile after a certain point in suspend/idle, but maybe that's a better trade off than having profiling disable certain PM features?

Kevin

#3 lore raw

RFC PATCH 2/2 arm64: kernel: perf: add pmu CPU PM notifier

[Cc'ing Dave]

Quoted 19 lines · Kevin Hilman
Quoted 11 lines · Lorenzo Pieralisi

When a CPU is being profiled through PMU events and it enters suspend or idle states, the PMU registers content can be lost, which means that counters that were relied upon on power down entry are reset on power up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects on entry if events are being monitored, and if so, it returns failure to the CPU PM notification chain, causing the suspend thread or the idle thread to abort power down, therefore preventing registers content loss.

Quoted 5 lines

By triggering CPU PM notification failure this patch prevents suspending a system if the suspend thread is being profiled and it also prevents entering idle deep states on cores that have profile events in use, somehow limiting power management capabilities when there are active perf sessions.

Quoted 3 lines

I guess that's one choice. Couldn't you also stop the PMU and save/restore it's context in the notifiers? so that you wouldn't affect PM capabilities?

Yes, that's why I sent this an RFC. This solution can also be easily ported to power domains, when we put them in place.

To save/restore PMU counters we can either reuse perf core (IIRC Dave had a stab at this, it is not a trivial patch) or do arch specific save/restore (with related buffers for registers context) but I think Will does not like the idea at all (and he has a point since the context memory is already there in perf core).

Quoted 3 lines

That would imply that you lose the ability to profile after a certain point in suspend/idle, but maybe that's a better trade off than having profiling disable certain PM features?

I am ok either way, as long as we make a decision, it has been hanging in the balance for aeons.

Thanks,
Lorenzo

#4 lore raw

RFC PATCH 2/2 arm64: kernel: perf: add pmu CPU PM notifier

In reply to Kevin Hilman
Quoted 19 lines · Kevin Hilman
Quoted 11 lines · Lorenzo Pieralisi

When a CPU is being profiled through PMU events and it enters suspend or idle states, the PMU registers content can be lost, which means that counters that were relied upon on power down entry are reset on power up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects on entry if events are being monitored, and if so, it returns failure to the CPU PM notification chain, causing the suspend thread or the idle thread to abort power down, therefore preventing registers content loss.

Quoted 5 lines

By triggering CPU PM notification failure this patch prevents suspending a system if the suspend thread is being profiled and it also prevents entering idle deep states on cores that have profile events in use, somehow limiting power management capabilities when there are active perf sessions.

Quoted 7 lines

I guess that's one choice. Couldn't you also stop the PMU and save/restore it's context in the notifiers? so that you wouldn't affect PM capabilities?

That would imply that you lose the ability to profile after a certain point in suspend/idle, but maybe that's a better trade off than having profiling disable certain PM features?

I had something like that a few years ago on the Kraits and Scorpions [1].

[1] https://www.codeaurora.org/cgit/quic/la/kernel/msm/commit/arch/arm/kernel?h=msm-3.4&id=b5ca687960f0fea2f4735e83ca5c9543474c19de

Cheers,
Ashwin.

#5 lore raw

RFC PATCH 2/2 arm64: kernel: perf: add pmu CPU PM notifier

Quoted 24 lines · Ashwin Chaugule
Quoted 23 lines · Kevin Hilman
Quoted 11 lines · Lorenzo Pieralisi

When a CPU is being profiled through PMU events and it enters suspend or idle states, the PMU registers content can be lost, which means that counters that were relied upon on power down entry are reset on power up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects on entry if events are being monitored, and if so, it returns failure to the CPU PM notification chain, causing the suspend thread or the idle thread to abort power down, therefore preventing registers content loss.

Quoted 5 lines

By triggering CPU PM notification failure this patch prevents suspending a system if the suspend thread is being profiled and it also prevents entering idle deep states on cores that have profile events in use, somehow limiting power management capabilities when there are active perf sessions.

I guess that's one choice. Couldn't you also stop the PMU and save/restore it's context in the notifiers? so that you wouldn't affect PM capabilities?

Quoted 5 lines
Quoted 3 lines

That would imply that you lose the ability to profile after a certain point in suspend/idle, but maybe that's a better trade off than having profiling disable certain PM features?

I had something like that a few years ago on the Kraits and Scorpions [1].

That's another option, but the point is understanding how we want to tackle the issue, by preventing power down or by restoring the PMU registers.

BTW, does your patch below need optimizing ? IIUC it "restores" the PMU counters even when that's not needed, I spotted some code in your tree that adds additional checks (ie check if returning from idle but I am not sure that's viable).

All in all going to deep idle must emulate a perf context switch through power down, I am pretty sured Dave implemented something like that, I will chase the code and then we will see what's the best option.

Lorenzo

Quoted 5 lines

[1] https://www.codeaurora.org/cgit/quic/la/kernel/msm/commit/arch/arm/kernel?h=msm-3.4&id=b5ca687960f0fea2f4735e83ca5c9543474c19de

Cheers,
Ashwin.

#6 lore raw

RFC PATCH 2/2 arm64: kernel: perf: add pmu CPU PM notifier

Quoted 29 lines · Lorenzo Pieralisi
Quoted 28 lines · Ashwin Chaugule
Quoted 26 lines · Kevin Hilman
Quoted 11 lines · Lorenzo Pieralisi

When a CPU is being profiled through PMU events and it enters suspend or idle states, the PMU registers content can be lost, which means that counters that were relied upon on power down entry are reset on power up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects on entry if events are being monitored, and if so, it returns failure to the CPU PM notification chain, causing the suspend thread or the idle thread to abort power down, therefore preventing registers content loss.

Quoted 5 lines

By triggering CPU PM notification failure this patch prevents suspending a system if the suspend thread is being profiled and it also prevents entering idle deep states on cores that have profile events in use, somehow limiting power management capabilities when there are active perf sessions.

I guess that's one choice. Couldn't you also stop the PMU and save/restore it's context in the notifiers? so that you wouldn't affect PM capabilities?

That would imply that you lose the ability to profile after a certain point in suspend/idle, but maybe that's a better trade off than having profiling disable certain PM features?

Quoted 5 lines
Quoted 1 line

I had something like that a few years ago on the Kraits and Scorpions [1].

That's another option, but the point is understanding how we want to tackle the issue, by preventing power down or by restoring the PMU registers.

Personally, I think the save/restore approach is preferred. IMO, it's more intuitive from the perspective of a user who doesn't understand all the mechanics and also actually allows you to profile most of the low-power paths and still actually hit the low power states.

Quoted 4 lines

BTW, does your patch below need optimizing ? IIUC it "restores" the PMU counters even when that's not needed, I spotted some code in your tree that adds additional checks (ie check if returning from idle but I am not sure that's viable).

Such an optimization could also be done when we (someday) move this cpu_pm notifier stuff to a proper pm_domain associated with the CPU/cluster.

Kevin

#7 lore raw

RFC PATCH 2/2 arm64: kernel: perf: add pmu CPU PM notifier

Quoted 33 lines · Kevin Hilman
Quoted 31 lines · Lorenzo Pieralisi
Quoted 29 lines · Ashwin Chaugule
Quoted 19 lines · Kevin Hilman
Quoted 11 lines · Lorenzo Pieralisi

When a CPU is being profiled through PMU events and it enters suspend or idle states, the PMU registers content can be lost, which means that counters that were relied upon on power down entry are reset on power up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects on entry if events are being monitored, and if so, it returns failure to the CPU PM notification chain, causing the suspend thread or the idle thread to abort power down, therefore preventing registers content loss.

Quoted 5 lines

By triggering CPU PM notification failure this patch prevents suspending a system if the suspend thread is being profiled and it also prevents entering idle deep states on cores that have profile events in use, somehow limiting power management capabilities when there are active perf sessions.

Quoted 7 lines

I guess that's one choice. Couldn't you also stop the PMU and save/restore it's context in the notifiers? so that you wouldn't affect PM capabilities?

That would imply that you lose the ability to profile after a certain point in suspend/idle, but maybe that's a better trade off than having profiling disable certain PM features?

I had something like that a few years ago on the Kraits and Scorpions [1].

Quoted 8 lines
Quoted 3 lines

That's another option, but the point is understanding how we want to tackle the issue, by preventing power down or by restoring the PMU registers.

Personally, I think the save/restore approach is preferred. IMO, it's more intuitive from the perspective of a user who doesn't understand all the mechanics and also actually allows you to profile most of the low-power paths and still actually hit the low power states.

I agree that save/restore is the nicest thing to do, but until we have a generic description of the power-domains in device-tree I really don't want PMU-specific hacks in the arch code to deal with this (since many platforms do not lose PMU state over suspend).

What Lorenzo is proposing is a stop-gap to prevent perf silently losing its state on platforms where the register contents is lost.

Will

#8 lore raw

RFC PATCH 2/2 arm64: kernel: perf: add pmu CPU PM notifier

Quoted 43 lines · Will Deacon
Quoted 33 lines · Kevin Hilman
Quoted 31 lines · Lorenzo Pieralisi
Quoted 29 lines · Ashwin Chaugule
Quoted 19 lines · Kevin Hilman
Quoted 11 lines · Lorenzo Pieralisi

When a CPU is being profiled through PMU events and it enters suspend or idle states, the PMU registers content can be lost, which means that counters that were relied upon on power down entry are reset on power up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects on entry if events are being monitored, and if so, it returns failure to the CPU PM notification chain, causing the suspend thread or the idle thread to abort power down, therefore preventing registers content loss.

Quoted 5 lines

By triggering CPU PM notification failure this patch prevents suspending a system if the suspend thread is being profiled and it also prevents entering idle deep states on cores that have profile events in use, somehow limiting power management capabilities when there are active perf sessions.

Quoted 7 lines

I guess that's one choice. Couldn't you also stop the PMU and save/restore it's context in the notifiers? so that you wouldn't affect PM capabilities?

That would imply that you lose the ability to profile after a certain point in suspend/idle, but maybe that's a better trade off than having profiling disable certain PM features?

I had something like that a few years ago on the Kraits and Scorpions [1].

Quoted 8 lines
Quoted 3 lines

That's another option, but the point is understanding how we want to tackle the issue, by preventing power down or by restoring the PMU registers.

Personally, I think the save/restore approach is preferred. IMO, it's more intuitive from the perspective of a user who doesn't understand all the mechanics and also actually allows you to profile most of the low-power paths and still actually hit the low power states.

Quoted 7 lines

I agree that save/restore is the nicest thing to do, but until we have a generic description of the power-domains in device-tree I really don't want PMU-specific hacks in the arch code to deal with this (since many platforms do not lose PMU state over suspend).

What Lorenzo is proposing is a stop-gap to prevent perf silently losing its state on platforms where the register contents is lost.

Yes, that's exactly the goal. I am fine either way, at the moment we are not in a position to define whether a core loses the PMU context, that's certain.

I am not sure to understand why people want the core to really go to shutdown and save/restore perf context if perf is on (eg profiling a cpu for cache stats, clearly powering down a core biases the results and might even obfuscate them).

It is not an easy call. If we go for this patch approach I am happy to rebase and retest the series again, for power domains I fear we have to wait given the sheer amount of changes in ARM CPUidle world at the moment, too much stuff going on there.

Lorenzo

#9 lore raw

RFC PATCH 2/2 arm64: kernel: perf: add pmu CPU PM notifier

Quoted 52 lines · Lorenzo Pieralisi
Quoted 43 lines · Will Deacon
Quoted 33 lines · Kevin Hilman
Quoted 31 lines · Lorenzo Pieralisi
Quoted 29 lines · Ashwin Chaugule
Quoted 19 lines · Kevin Hilman
Quoted 11 lines · Lorenzo Pieralisi

When a CPU is being profiled through PMU events and it enters suspend or idle states, the PMU registers content can be lost, which means that counters that were relied upon on power down entry are reset on power up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects on entry if events are being monitored, and if so, it returns failure to the CPU PM notification chain, causing the suspend thread or the idle thread to abort power down, therefore preventing registers content loss.

Quoted 5 lines

By triggering CPU PM notification failure this patch prevents suspending a system if the suspend thread is being profiled and it also prevents entering idle deep states on cores that have profile events in use, somehow limiting power management capabilities when there are active perf sessions.

Quoted 7 lines

I guess that's one choice. Couldn't you also stop the PMU and save/restore it's context in the notifiers? so that you wouldn't affect PM capabilities?

That would imply that you lose the ability to profile after a certain point in suspend/idle, but maybe that's a better trade off than having profiling disable certain PM features?

I had something like that a few years ago on the Kraits and Scorpions [1].

Quoted 8 lines
Quoted 3 lines

That's another option, but the point is understanding how we want to tackle the issue, by preventing power down or by restoring the PMU registers.

Personally, I think the save/restore approach is preferred. IMO, it's more intuitive from the perspective of a user who doesn't understand all the mechanics and also actually allows you to profile most of the low-power paths and still actually hit the low power states.

Quoted 7 lines

I agree that save/restore is the nicest thing to do, but until we have a generic description of the power-domains in device-tree I really don't want PMU-specific hacks in the arch code to deal with this (since many platforms do not lose PMU state over suspend).

What Lorenzo is proposing is a stop-gap to prevent perf silently losing its state on platforms where the register contents is lost.

Quoted 8 lines

Yes, that's exactly the goal. I am fine either way, at the moment we are not in a position to define whether a core loses the PMU context, that's certain.

I am not sure to understand why people want the core to really go to shutdown and save/restore perf context if perf is on (eg profiling a cpu for cache stats, clearly powering down a core biases the results and might even obfuscate them).

The main thing I was considering is when using perf to profile the low-power paths (suspend or idle.)

Quoted 4 lines

It is not an easy call. If we go for this patch approach I am happy to rebase and retest the series again, for power domains I fear we have to wait given the sheer amount of changes in ARM CPUidle world at the moment, too much stuff going on there.

Ultimately, I'm OK with the proposed solution as a short/medium term solution. When our PM domain and CPUidle integration story gets a little better, we can revisit this.

Kevin