Skip to contentSkip to conversation

x86: PM: Register syscore_ops for scale invariance

PATCH Applied Rafael J. Wysocki 5 participants 7 messages Jan 8, 2021 – Jan 19, 2021 x86

lore
Share & tools
lore

Rafael J. Wysocki

author
#0 lore raw

PATCH 1/1 x86: PM: Register syscore_ops for scale invariance

From: Rafael J. Wysocki <[email protected]>

On x86 scale invariace tends to be disabled during resume from suspend-to-RAM, because the MPERF or APERF MSR values are not as expected then due to updates taking place after the platform firmware has been invoked to complete the suspend transition.

That, of course, is not desirable, especially if the schedutil scaling governor is in use, because the lack of scale invariance causes it to be less reliable.

To counter that effect, modify init_freq_invariance() to register a syscore_ops object for scale invariance with the ->resume callback pointing to init_counter_refs() which will run on the CPU starting the resume transition (the other CPUs will be taken care of the "online" operations taking place later).

Fixes e2b0d619b400 ("x86, sched: check for counters overflow in frequency invariant accounting")
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 arch/x86/kernel/smpboot.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
arch/x86/kernel/smpboot.c +19 -0
Index: linux-pm/arch/x86/kernel/smpboot.c
===================================================================
@@ -56,6 +56,7 @@
 #include <linux/numa.h>
 #include <linux/pgtable.h>
 #include <linux/overflow.h>
+#include <linux/syscore_ops.h>
 
 #include <asm/acpi.h>
 #include <asm/desc.h>

@@ -2083,6 +2084,23 @@ static void init_counter_refs(void)
 	this_cpu_write(arch_prev_mperf, mperf);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static struct syscore_ops freq_invariance_syscore_ops = {
+	.resume = init_counter_refs,
+};
+
+static void register_freq_invariance_syscore_ops(void)
+{
+	/* Bail out if registered already. */
+	if (freq_invariance_syscore_ops.node.prev)
+		return;
+
+	register_syscore_ops(&freq_invariance_syscore_ops);
+}
+#else
+static inline void register_freq_invariance_syscore_ops(void) {}
+#endif
+
 static void init_freq_invariance(bool secondary, bool cppc_ready)
 {
 	bool ret = false;

@@ -2109,6 +2127,7 @@ static void init_freq_invariance(bool se
 	if (ret) {
 		init_counter_refs();
 		static_branch_enable(&arch_scale_freq_key);
+		register_freq_invariance_syscore_ops();
 		pr_info("Estimated ratio of average max frequency by base frequency (times 1024): %llu\n", arch_max_freq_ratio);
 	} else {
 		pr_debug("Couldn't determine max cpu frequency, necessary for scale-invariant accounting.\n");
#1 lore raw
Quoted 17 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

On x86 scale invariace tends to be disabled during resume from suspend-to-RAM, because the MPERF or APERF MSR values are not as expected then due to updates taking place after the platform firmware has been invoked to complete the suspend transition.

That, of course, is not desirable, especially if the schedutil scaling governor is in use, because the lack of scale invariance causes it to be less reliable.

To counter that effect, modify init_freq_invariance() to register a syscore_ops object for scale invariance with the ->resume callback pointing to init_counter_refs() which will run on the CPU starting the resume transition (the other CPUs will be taken care of the "online" operations taking place later).

Quoted 3 lines
Fixes e2b0d619b400 ("x86, sched: check for counters overflow in frequency invariant accounting")
Signed-off-by Rafael J. Wysocki <[email protected]>

<snip>

Thanks for writing this, Rafael.

Peter Zijlstra asked to fix this problem months ago; I started but got stucked and never finished.

Giovanni Gherdovich

#2 lore raw
In reply to Rafael J. Wysocki
Quoted 11 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

On x86 scale invariace tends to be disabled during resume from suspend-to-RAM, because the MPERF or APERF MSR values are not as expected then due to updates taking place after the platform firmware has been invoked to complete the suspend transition.

That, of course, is not desirable, especially if the schedutil scaling governor is in use, because the lack of scale invariance causes it to be less reliable.

Quoted 8 lines

To counter that effect, modify init_freq_invariance() to register a syscore_ops object for scale invariance with the ->resume callback pointing to init_counter_refs() which will run on the CPU starting the resume transition (the other CPUs will be taken care of the "online" operations taking place later).

Fixes e2b0d619b400 ("x86, sched: check for counters overflow in frequency invariant accounting")
Signed-off-by Rafael J. Wysocki <[email protected]>

Thanks!, I'll take it through the sched/urgent tree?

#3 lore raw
Quoted 19 lines · Peter Zijlstra
Quoted 17 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

On x86 scale invariace tends to be disabled during resume from suspend-to-RAM, because the MPERF or APERF MSR values are not as expected then due to updates taking place after the platform firmware has been invoked to complete the suspend transition.

That, of course, is not desirable, especially if the schedutil scaling governor is in use, because the lack of scale invariance causes it to be less reliable.

To counter that effect, modify init_freq_invariance() to register a syscore_ops object for scale invariance with the ->resume callback pointing to init_counter_refs() which will run on the CPU starting the resume transition (the other CPUs will be taken care of the "online" operations taking place later).

Quoted 4 lines
Quoted 2 lines
Fixes e2b0d619b400 ("x86, sched: check for counters overflow in frequency invariant accounting")
Signed-off-by Rafael J. Wysocki <[email protected]>

Thanks!, I'll take it through the sched/urgent tree?

That works, thanks!

#4 lore raw
Quoted 21 lines · Rafael J. Wysocki
Quoted 19 lines · Peter Zijlstra
Quoted 17 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

On x86 scale invariace tends to be disabled during resume from suspend-to-RAM, because the MPERF or APERF MSR values are not as expected then due to updates taking place after the platform firmware has been invoked to complete the suspend transition.

That, of course, is not desirable, especially if the schedutil scaling governor is in use, because the lack of scale invariance causes it to be less reliable.

To counter that effect, modify init_freq_invariance() to register a syscore_ops object for scale invariance with the ->resume callback pointing to init_counter_refs() which will run on the CPU starting the resume transition (the other CPUs will be taken care of the "online" operations taking place later).

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Fixes e2b0d619b400 ("x86, sched: check for counters overflow in frequency invariant accounting")
Signed-off-by Rafael J. Wysocki <[email protected]>

Thanks!, I'll take it through the sched/urgent tree?

That works, thanks!

Any news on this front? It's been a few days ...

#5 lore raw
Quoted 22 lines · Rafael J. Wysocki
Quoted 21 lines · Rafael J. Wysocki
Quoted 19 lines · Peter Zijlstra
Quoted 17 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

On x86 scale invariace tends to be disabled during resume from suspend-to-RAM, because the MPERF or APERF MSR values are not as expected then due to updates taking place after the platform firmware has been invoked to complete the suspend transition.

That, of course, is not desirable, especially if the schedutil scaling governor is in use, because the lack of scale invariance causes it to be less reliable.

To counter that effect, modify init_freq_invariance() to register a syscore_ops object for scale invariance with the ->resume callback pointing to init_counter_refs() which will run on the CPU starting the resume transition (the other CPUs will be taken care of the "online" operations taking place later).

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Fixes e2b0d619b400 ("x86, sched: check for counters overflow in frequency invariant accounting")
Signed-off-by Rafael J. Wysocki <[email protected]>

Thanks!, I'll take it through the sched/urgent tree?

That works, thanks!

Any news on this front? It's been a few days ...

My bad, it's been held up behind me trying to fix another sched regression. Lemme push out just this one so it doesn't go walk-about.

merge bot Applied
#6 lore raw
In reply to Rafael J. Wysocki
The following commit has been merged into the sched/urgent branch of tip:

The following commit has been merged into the sched/urgent branch of tip:

Commit-ID:     9c7d9017a49fb8516c13b7bff59b7da2abed23e1
Gitweb:        https://git.kernel.org/tip/9c7d9017a49fb8516c13b7bff59b7da2abed23e1
Author:        Rafael J. Wysocki <[email protected]>
AuthorDate:    Fri, 08 Jan 2021 19:05:59 +01:00
Committer:     Peter Zijlstra <[email protected]>
CommitterDate: Tue, 19 Jan 2021 17:04:03 +01:00

x86: PM: Register syscore_ops for scale invariance

On x86 scale invariace tends to be disabled during resume from suspend-to-RAM, because the MPERF or APERF MSR values are not as expected then due to updates taking place after the platform firmware has been invoked to complete the suspend transition.

That, of course, is not desirable, especially if the schedutil scaling governor is in use, because the lack of scale invariance causes it to be less reliable.

To counter that effect, modify init_freq_invariance() to register a syscore_ops object for scale invariance with the ->resume callback pointing to init_counter_refs() which will run on the CPU starting the resume transition (the other CPUs will be taken care of the "online" operations taking place later).

Fixes e2b0d619b400 ("x86, sched: check for counters overflow in frequency invariant accounting")
Signed-off-by Rafael J. Wysocki <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Acked-by Giovanni Gherdovich <[email protected]>
Link https://lkml.kernel.org/r/1803209.Mvru99baaF@kreacher open here
---
 arch/x86/kernel/smpboot.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
arch/x86/kernel/smpboot.c +19 -0
index 8ca66af..117e24f 100644
@@ -56,6 +56,7 @@
 #include <linux/numa.h>
 #include <linux/pgtable.h>
 #include <linux/overflow.h>
+#include <linux/syscore_ops.h>
 
 #include <asm/acpi.h>
 #include <asm/desc.h>

@@ -2083,6 +2084,23 @@ static void init_counter_refs(void)
 	this_cpu_write(arch_prev_mperf, mperf);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static struct syscore_ops freq_invariance_syscore_ops = {
+	.resume = init_counter_refs,
+};
+
+static void register_freq_invariance_syscore_ops(void)
+{
+	/* Bail out if registered already. */
+	if (freq_invariance_syscore_ops.node.prev)
+		return;
+
+	register_syscore_ops(&freq_invariance_syscore_ops);
+}
+#else
+static inline void register_freq_invariance_syscore_ops(void) {}
+#endif
+
 static void init_freq_invariance(bool secondary, bool cppc_ready)
 {
 	bool ret = false;

@@ -2109,6 +2127,7 @@ static void init_freq_invariance(bool secondary, bool cppc_ready)
 	if (ret) {
 		init_counter_refs();
 		static_branch_enable(&arch_scale_freq_key);
+		register_freq_invariance_syscore_ops();
 		pr_info("Estimated ratio of average max frequency by base frequency (times 1024): %llu\n", arch_max_freq_ratio);
 	} else {
 		pr_debug("Couldn't determine max cpu frequency, necessary for scale-invariant accounting.\n");