All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] More frequency invariance fixes for x86
@ 2020-05-31 18:24 Giovanni Gherdovich
  2020-05-31 18:24 ` [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting Giovanni Gherdovich
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Giovanni Gherdovich @ 2020-05-31 18:24 UTC (permalink / raw)
  To: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Peter Zijlstra, Borislav Petkov, Rafael J . Wysocki
  Cc: x86, linux-pm, linux-kernel, Ricardo Neri, Giovanni Gherdovich

v1 at https://lore.kernel.org/lkml/20200428132450.24901-1-ggherdovich@suse.cz/

changes wrt v1:

- add Peter Zijlstra's code to check for multiplication overflow, see
  https://lore.kernel.org/lkml/20200501133042.GE3762@hirez.programming.kicks-ass.net/
- put all frequence invariant code behind CONFIG_X86_64, as the overflow
  checks need 64 bits operations, see the build error at
  https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/7GDIBOMNVDG5W2XZD4EICE2TUZR3THBN/
- add additional patch to check for when base_freq > turbo_freq,
  suggested by Peter Zijlstra at
  https://lore.kernel.org/lkml/20200501130427.GD3762@hirez.programming.kicks-ass.net/
  
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Cover Letter from v1:

Patch 1/2 prevents a division by zero in case the product
"delta_MPERF * arch_max_freq_ratio" overflows u64, as suggested by Linus at [1].
This patch supersedes the version at [2], as it also disables frequency
invariance when that overflow happens.

Patch 2/2 implements the recommendation by Ricardo Neri to check for an all
zero MSR_TURBO_RATIO_LIMIT and disable freq invariance in that case too.

[1] https://lore.kernel.org/lkml/CAHk-=wiX+NT2yxtdPszH9U_S96MCNQA56GJFXY45mZc47yG5KQ@mail.gmail.com/
[2] https://lore.kernel.org/lkml/20200422144055.18171-1-ggherdovich@suse.cz/
[3] https://lore.kernel.org/lkml/20200424013222.GA26355@ranerica-svr.sc.intel.com/

Giovanni Gherdovich (3):
  x86, sched: check for counters overflow in frequency invariant
    accounting
  x86, sched: Bail out of frequency invariance if turbo frequency is
    unknown
  x86, sched: Bail out of frequency invariance if turbo_freq/base_freq
    gives 0

 arch/x86/include/asm/topology.h |  2 +-
 arch/x86/kernel/smpboot.c       | 50 +++++++++++++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 10 deletions(-)

-- 
2.16.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting
  2020-05-31 18:24 [PATCH v2 0/3] More frequency invariance fixes for x86 Giovanni Gherdovich
@ 2020-05-31 18:24 ` Giovanni Gherdovich
  2020-06-03 14:22   ` Rafael J. Wysocki
                     ` (2 more replies)
  2020-05-31 18:24 ` [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown Giovanni Gherdovich
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 16+ messages in thread
From: Giovanni Gherdovich @ 2020-05-31 18:24 UTC (permalink / raw)
  To: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Peter Zijlstra, Borislav Petkov, Rafael J . Wysocki
  Cc: x86, linux-pm, linux-kernel, Ricardo Neri, Giovanni Gherdovich

The product mcnt * arch_max_freq_ratio can overflows u64.

For context, a large value for arch_max_freq_ratio would be 5000,
corresponding to a turbo_freq/base_freq ratio of 5 (normally it's more like
1500-2000). A large increment frequency for the MPERF counter would be 5GHz
(the base clock of all CPUs on the market today is less than that). With
these figures, a CPU would need to go without a scheduler tick for around 8
days for the u64 overflow to happen. It is unlikely, but the check is
warranted.

Under similar conditions, the difference acnt of two consecutive APERF
readings can overflow as well.

In these circumstances is appropriate to disable frequency invariant
accounting: the feature relies on measures of the clock frequency done at
every scheduler tick, which need to be "fresh" to be at all meaningful.

A note on i386: prior to version 5.1, the GCC compiler didn't have the
builtin function __builtin_mul_overflow. In these GCC versions the macro
check_mul_overflow needs __udivdi3() to do (u64)a/b, which the kernel
doesn't provide. For this reason this change fails to build on i386 if
GCC<5.1, and we protect the entire frequency invariant code behind
CONFIG_X86_64 (special thanks to "kbuild test robot" <lkp@intel.com>).

Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Fixes: 1567c3e3467c ("x86, sched: Add support for frequency invariance")
---
 arch/x86/include/asm/topology.h |  2 +-
 arch/x86/kernel/smpboot.c       | 33 ++++++++++++++++++++++++++++-----
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 79d8d5496330..f4234575f3fd 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -193,7 +193,7 @@ static inline void sched_clear_itmt_support(void)
 }
 #endif /* CONFIG_SCHED_MC_PRIO */
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) && defined(CONFIG_X86_64)
 #include <asm/cpufeature.h>
 
 DECLARE_STATIC_KEY_FALSE(arch_scale_freq_key);
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 2f24c334a938..d660966d7de7 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -55,6 +55,7 @@
 #include <linux/gfp.h>
 #include <linux/cpuidle.h>
 #include <linux/numa.h>
+#include <linux/overflow.h>
 
 #include <asm/acpi.h>
 #include <asm/desc.h>
@@ -1777,6 +1778,7 @@ void native_play_dead(void)
 
 #endif
 
+#ifdef CONFIG_X86_64
 /*
  * APERF/MPERF frequency ratio computation.
  *
@@ -2047,11 +2049,19 @@ static void init_freq_invariance(bool secondary)
 	}
 }
 
+static void disable_freq_invariance_workfn(struct work_struct *work)
+{
+	static_branch_disable(&arch_scale_freq_key);
+}
+
+static DECLARE_WORK(disable_freq_invariance_work,
+		    disable_freq_invariance_workfn);
+
 DEFINE_PER_CPU(unsigned long, arch_freq_scale) = SCHED_CAPACITY_SCALE;
 
 void arch_scale_freq_tick(void)
 {
-	u64 freq_scale;
+	u64 freq_scale = SCHED_CAPACITY_SCALE;
 	u64 aperf, mperf;
 	u64 acnt, mcnt;
 
@@ -2063,19 +2073,32 @@ void arch_scale_freq_tick(void)
 
 	acnt = aperf - this_cpu_read(arch_prev_aperf);
 	mcnt = mperf - this_cpu_read(arch_prev_mperf);
-	if (!mcnt)
-		return;
 
 	this_cpu_write(arch_prev_aperf, aperf);
 	this_cpu_write(arch_prev_mperf, mperf);
 
-	acnt <<= 2*SCHED_CAPACITY_SHIFT;
-	mcnt *= arch_max_freq_ratio;
+	if (check_shl_overflow(acnt, 2*SCHED_CAPACITY_SHIFT, &acnt))
+		goto error;
+
+	if (check_mul_overflow(mcnt, arch_max_freq_ratio, &mcnt) || !mcnt)
+		goto error;
 
 	freq_scale = div64_u64(acnt, mcnt);
+	if (!freq_scale)
+		goto error;
 
 	if (freq_scale > SCHED_CAPACITY_SCALE)
 		freq_scale = SCHED_CAPACITY_SCALE;
 
 	this_cpu_write(arch_freq_scale, freq_scale);
+	return;
+
+error:
+	pr_warn("Scheduler frequency invariance went wobbly, disabling!\n");
+	schedule_work(&disable_freq_invariance_work);
+}
+#else
+static inline void init_freq_invariance(bool secondary)
+{
 }
+#endif /* CONFIG_X86_64 */
-- 
2.16.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown
  2020-05-31 18:24 [PATCH v2 0/3] More frequency invariance fixes for x86 Giovanni Gherdovich
  2020-05-31 18:24 ` [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting Giovanni Gherdovich
@ 2020-05-31 18:24 ` Giovanni Gherdovich
  2020-06-01 23:34   ` Ricardo Neri
                     ` (2 more replies)
  2020-05-31 18:24 ` [PATCH v2 3/3] x86, sched: Bail out of frequency invariance if turbo_freq/base_freq gives 0 Giovanni Gherdovich
  2020-06-03 12:31 ` [PATCH v2 0/3] More frequency invariance fixes for x86 Peter Zijlstra
  3 siblings, 3 replies; 16+ messages in thread
From: Giovanni Gherdovich @ 2020-05-31 18:24 UTC (permalink / raw)
  To: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Peter Zijlstra, Borislav Petkov, Rafael J . Wysocki
  Cc: x86, linux-pm, linux-kernel, Ricardo Neri, Giovanni Gherdovich

There may be CPUs that support turbo boost but don't declare any turbo
ratio, i.e. their MSR_TURBO_RATIO_LIMIT is all zeroes. In that condition
scale-invariant calculations can't be performed.

Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
Suggested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Fixes: 1567c3e3467c ("x86, sched: Add support for frequency invariance")
---
 arch/x86/kernel/smpboot.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index d660966d7de7..fe154c8226ba 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -2001,9 +2001,11 @@ static bool intel_set_max_freq_ratio(void)
 	/*
 	 * Some hypervisors advertise X86_FEATURE_APERFMPERF
 	 * but then fill all MSR's with zeroes.
+	 * Some CPUs have turbo boost but don't declare any turbo ratio
+	 * in MSR_TURBO_RATIO_LIMIT.
 	 */
-	if (!base_freq) {
-		pr_debug("Couldn't determine cpu base frequency, necessary for scale-invariant accounting.\n");
+	if (!base_freq || !turbo_freq) {
+		pr_debug("Couldn't determine cpu base or turbo frequency, necessary for scale-invariant accounting.\n");
 		return false;
 	}
 
-- 
2.16.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 3/3] x86, sched: Bail out of frequency invariance if turbo_freq/base_freq gives 0
  2020-05-31 18:24 [PATCH v2 0/3] More frequency invariance fixes for x86 Giovanni Gherdovich
  2020-05-31 18:24 ` [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting Giovanni Gherdovich
  2020-05-31 18:24 ` [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown Giovanni Gherdovich
@ 2020-05-31 18:24 ` Giovanni Gherdovich
  2020-06-03 14:51   ` Rafael J. Wysocki
  2020-06-16 12:21   ` [tip: sched/core] " tip-bot2 for Giovanni Gherdovich
  2020-06-03 12:31 ` [PATCH v2 0/3] More frequency invariance fixes for x86 Peter Zijlstra
  3 siblings, 2 replies; 16+ messages in thread
From: Giovanni Gherdovich @ 2020-05-31 18:24 UTC (permalink / raw)
  To: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Peter Zijlstra, Borislav Petkov, Rafael J . Wysocki
  Cc: x86, linux-pm, linux-kernel, Ricardo Neri, Giovanni Gherdovich

Be defensive against the case where the processor reports a base_freq
larger than turbo_freq (the ratio would be zero).

Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Fixes: 1567c3e3467c ("x86, sched: Add support for frequency invariance")
---
 arch/x86/kernel/smpboot.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index fe154c8226ba..f619007f46cf 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1976,6 +1976,7 @@ static bool core_set_max_freq_ratio(u64 *base_freq, u64 *turbo_freq)
 static bool intel_set_max_freq_ratio(void)
 {
 	u64 base_freq, turbo_freq;
+	u64 turbo_ratio;
 
 	if (slv_set_max_freq_ratio(&base_freq, &turbo_freq))
 		goto out;
@@ -2009,9 +2010,15 @@ static bool intel_set_max_freq_ratio(void)
 		return false;
 	}
 
-	arch_turbo_freq_ratio = div_u64(turbo_freq * SCHED_CAPACITY_SCALE,
-					base_freq);
+	turbo_ratio = div_u64(turbo_freq * SCHED_CAPACITY_SCALE, base_freq);
+	if (!turbo_ratio) {
+		pr_debug("Non-zero turbo and base frequencies led to a 0 ratio.\n");
+		return false;
+	}
+
+	arch_turbo_freq_ratio = turbo_ratio;
 	arch_set_max_freq_ratio(turbo_disabled());
+
 	return true;
 }
 
-- 
2.16.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown
  2020-05-31 18:24 ` [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown Giovanni Gherdovich
@ 2020-06-01 23:34   ` Ricardo Neri
  2020-07-06 20:19     ` Ira Weiny
  2020-06-03 14:23   ` Rafael J. Wysocki
  2020-06-16 12:21   ` [tip: sched/core] " tip-bot2 for Giovanni Gherdovich
  2 siblings, 1 reply; 16+ messages in thread
From: Ricardo Neri @ 2020-06-01 23:34 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Peter Zijlstra, Borislav Petkov, Rafael J . Wysocki, x86,
	linux-pm, linux-kernel

On Sun, May 31, 2020 at 08:24:52PM +0200, Giovanni Gherdovich wrote:
> There may be CPUs that support turbo boost but don't declare any turbo
> ratio, i.e. their MSR_TURBO_RATIO_LIMIT is all zeroes. In that condition
> scale-invariant calculations can't be performed.
> 
> Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> Suggested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

FWIW,

Tested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] More frequency invariance fixes for x86
  2020-05-31 18:24 [PATCH v2 0/3] More frequency invariance fixes for x86 Giovanni Gherdovich
                   ` (2 preceding siblings ...)
  2020-05-31 18:24 ` [PATCH v2 3/3] x86, sched: Bail out of frequency invariance if turbo_freq/base_freq gives 0 Giovanni Gherdovich
@ 2020-06-03 12:31 ` Peter Zijlstra
  3 siblings, 0 replies; 16+ messages in thread
From: Peter Zijlstra @ 2020-06-03 12:31 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Rafael J . Wysocki, x86, linux-pm, linux-kernel,
	Ricardo Neri

On Sun, May 31, 2020 at 08:24:50PM +0200, Giovanni Gherdovich wrote:
> changes wrt v1:
> 
> - add Peter Zijlstra's code to check for multiplication overflow, see
>   https://lore.kernel.org/lkml/20200501133042.GE3762@hirez.programming.kicks-ass.net/
> - put all frequence invariant code behind CONFIG_X86_64, as the overflow
>   checks need 64 bits operations, see the build error at
>   https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/7GDIBOMNVDG5W2XZD4EICE2TUZR3THBN/
> - add additional patch to check for when base_freq > turbo_freq,
>   suggested by Peter Zijlstra at
>   https://lore.kernel.org/lkml/20200501130427.GD3762@hirez.programming.kicks-ass.net/

Thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting
  2020-05-31 18:24 ` [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting Giovanni Gherdovich
@ 2020-06-03 14:22   ` Rafael J. Wysocki
  2020-06-16 12:21   ` [tip: sched/core] " tip-bot2 for Giovanni Gherdovich
  2020-10-22  8:46   ` [PATCH v2 1/3] " Peter Zijlstra
  2 siblings, 0 replies; 16+ messages in thread
From: Rafael J. Wysocki @ 2020-06-03 14:22 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Peter Zijlstra, Borislav Petkov, Rafael J . Wysocki,
	the arch/x86 maintainers, Linux PM, Linux Kernel Mailing List,
	Ricardo Neri

On Sun, May 31, 2020 at 8:26 PM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
>
> The product mcnt * arch_max_freq_ratio can overflows u64.
>
> For context, a large value for arch_max_freq_ratio would be 5000,
> corresponding to a turbo_freq/base_freq ratio of 5 (normally it's more like
> 1500-2000). A large increment frequency for the MPERF counter would be 5GHz
> (the base clock of all CPUs on the market today is less than that). With
> these figures, a CPU would need to go without a scheduler tick for around 8
> days for the u64 overflow to happen. It is unlikely, but the check is
> warranted.
>
> Under similar conditions, the difference acnt of two consecutive APERF
> readings can overflow as well.
>
> In these circumstances is appropriate to disable frequency invariant
> accounting: the feature relies on measures of the clock frequency done at
> every scheduler tick, which need to be "fresh" to be at all meaningful.
>
> A note on i386: prior to version 5.1, the GCC compiler didn't have the
> builtin function __builtin_mul_overflow. In these GCC versions the macro
> check_mul_overflow needs __udivdi3() to do (u64)a/b, which the kernel
> doesn't provide. For this reason this change fails to build on i386 if
> GCC<5.1, and we protect the entire frequency invariant code behind
> CONFIG_X86_64 (special thanks to "kbuild test robot" <lkp@intel.com>).
>
> Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Fixes: 1567c3e3467c ("x86, sched: Add support for frequency invariance")

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  arch/x86/include/asm/topology.h |  2 +-
>  arch/x86/kernel/smpboot.c       | 33 ++++++++++++++++++++++++++++-----
>  2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index 79d8d5496330..f4234575f3fd 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -193,7 +193,7 @@ static inline void sched_clear_itmt_support(void)
>  }
>  #endif /* CONFIG_SCHED_MC_PRIO */
>
> -#ifdef CONFIG_SMP
> +#if defined(CONFIG_SMP) && defined(CONFIG_X86_64)
>  #include <asm/cpufeature.h>
>
>  DECLARE_STATIC_KEY_FALSE(arch_scale_freq_key);
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 2f24c334a938..d660966d7de7 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -55,6 +55,7 @@
>  #include <linux/gfp.h>
>  #include <linux/cpuidle.h>
>  #include <linux/numa.h>
> +#include <linux/overflow.h>
>
>  #include <asm/acpi.h>
>  #include <asm/desc.h>
> @@ -1777,6 +1778,7 @@ void native_play_dead(void)
>
>  #endif
>
> +#ifdef CONFIG_X86_64
>  /*
>   * APERF/MPERF frequency ratio computation.
>   *
> @@ -2047,11 +2049,19 @@ static void init_freq_invariance(bool secondary)
>         }
>  }
>
> +static void disable_freq_invariance_workfn(struct work_struct *work)
> +{
> +       static_branch_disable(&arch_scale_freq_key);
> +}
> +
> +static DECLARE_WORK(disable_freq_invariance_work,
> +                   disable_freq_invariance_workfn);
> +
>  DEFINE_PER_CPU(unsigned long, arch_freq_scale) = SCHED_CAPACITY_SCALE;
>
>  void arch_scale_freq_tick(void)
>  {
> -       u64 freq_scale;
> +       u64 freq_scale = SCHED_CAPACITY_SCALE;
>         u64 aperf, mperf;
>         u64 acnt, mcnt;
>
> @@ -2063,19 +2073,32 @@ void arch_scale_freq_tick(void)
>
>         acnt = aperf - this_cpu_read(arch_prev_aperf);
>         mcnt = mperf - this_cpu_read(arch_prev_mperf);
> -       if (!mcnt)
> -               return;
>
>         this_cpu_write(arch_prev_aperf, aperf);
>         this_cpu_write(arch_prev_mperf, mperf);
>
> -       acnt <<= 2*SCHED_CAPACITY_SHIFT;
> -       mcnt *= arch_max_freq_ratio;
> +       if (check_shl_overflow(acnt, 2*SCHED_CAPACITY_SHIFT, &acnt))
> +               goto error;
> +
> +       if (check_mul_overflow(mcnt, arch_max_freq_ratio, &mcnt) || !mcnt)
> +               goto error;
>
>         freq_scale = div64_u64(acnt, mcnt);
> +       if (!freq_scale)
> +               goto error;
>
>         if (freq_scale > SCHED_CAPACITY_SCALE)
>                 freq_scale = SCHED_CAPACITY_SCALE;
>
>         this_cpu_write(arch_freq_scale, freq_scale);
> +       return;
> +
> +error:
> +       pr_warn("Scheduler frequency invariance went wobbly, disabling!\n");
> +       schedule_work(&disable_freq_invariance_work);
> +}
> +#else
> +static inline void init_freq_invariance(bool secondary)
> +{
>  }
> +#endif /* CONFIG_X86_64 */
> --
> 2.16.4
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown
  2020-05-31 18:24 ` [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown Giovanni Gherdovich
  2020-06-01 23:34   ` Ricardo Neri
@ 2020-06-03 14:23   ` Rafael J. Wysocki
  2020-06-16 12:21   ` [tip: sched/core] " tip-bot2 for Giovanni Gherdovich
  2 siblings, 0 replies; 16+ messages in thread
From: Rafael J. Wysocki @ 2020-06-03 14:23 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Peter Zijlstra, Borislav Petkov, Rafael J . Wysocki,
	the arch/x86 maintainers, Linux PM, Linux Kernel Mailing List,
	Ricardo Neri

On Sun, May 31, 2020 at 8:26 PM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
>
> There may be CPUs that support turbo boost but don't declare any turbo
> ratio, i.e. their MSR_TURBO_RATIO_LIMIT is all zeroes. In that condition
> scale-invariant calculations can't be performed.
>
> Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> Suggested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> Fixes: 1567c3e3467c ("x86, sched: Add support for frequency invariance")

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  arch/x86/kernel/smpboot.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index d660966d7de7..fe154c8226ba 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -2001,9 +2001,11 @@ static bool intel_set_max_freq_ratio(void)
>         /*
>          * Some hypervisors advertise X86_FEATURE_APERFMPERF
>          * but then fill all MSR's with zeroes.
> +        * Some CPUs have turbo boost but don't declare any turbo ratio
> +        * in MSR_TURBO_RATIO_LIMIT.
>          */
> -       if (!base_freq) {
> -               pr_debug("Couldn't determine cpu base frequency, necessary for scale-invariant accounting.\n");
> +       if (!base_freq || !turbo_freq) {
> +               pr_debug("Couldn't determine cpu base or turbo frequency, necessary for scale-invariant accounting.\n");
>                 return false;
>         }
>
> --
> 2.16.4
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] x86, sched: Bail out of frequency invariance if turbo_freq/base_freq gives 0
  2020-05-31 18:24 ` [PATCH v2 3/3] x86, sched: Bail out of frequency invariance if turbo_freq/base_freq gives 0 Giovanni Gherdovich
@ 2020-06-03 14:51   ` Rafael J. Wysocki
  2020-06-16 12:21   ` [tip: sched/core] " tip-bot2 for Giovanni Gherdovich
  1 sibling, 0 replies; 16+ messages in thread
From: Rafael J. Wysocki @ 2020-06-03 14:51 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Peter Zijlstra, Borislav Petkov, Rafael J . Wysocki,
	the arch/x86 maintainers, Linux PM, Linux Kernel Mailing List,
	Ricardo Neri

On Sun, May 31, 2020 at 8:26 PM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
>
> Be defensive against the case where the processor reports a base_freq
> larger than turbo_freq (the ratio would be zero).
>
> Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Fixes: 1567c3e3467c ("x86, sched: Add support for frequency invariance")

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  arch/x86/kernel/smpboot.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index fe154c8226ba..f619007f46cf 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1976,6 +1976,7 @@ static bool core_set_max_freq_ratio(u64 *base_freq, u64 *turbo_freq)
>  static bool intel_set_max_freq_ratio(void)
>  {
>         u64 base_freq, turbo_freq;
> +       u64 turbo_ratio;
>
>         if (slv_set_max_freq_ratio(&base_freq, &turbo_freq))
>                 goto out;
> @@ -2009,9 +2010,15 @@ static bool intel_set_max_freq_ratio(void)
>                 return false;
>         }
>
> -       arch_turbo_freq_ratio = div_u64(turbo_freq * SCHED_CAPACITY_SCALE,
> -                                       base_freq);
> +       turbo_ratio = div_u64(turbo_freq * SCHED_CAPACITY_SCALE, base_freq);
> +       if (!turbo_ratio) {
> +               pr_debug("Non-zero turbo and base frequencies led to a 0 ratio.\n");
> +               return false;
> +       }
> +
> +       arch_turbo_freq_ratio = turbo_ratio;
>         arch_set_max_freq_ratio(turbo_disabled());
> +
>         return true;
>  }
>
> --
> 2.16.4
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [tip: sched/core] x86, sched: Bail out of frequency invariance if turbo frequency is unknown
  2020-05-31 18:24 ` [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown Giovanni Gherdovich
  2020-06-01 23:34   ` Ricardo Neri
  2020-06-03 14:23   ` Rafael J. Wysocki
@ 2020-06-16 12:21   ` tip-bot2 for Giovanni Gherdovich
  2 siblings, 0 replies; 16+ messages in thread
From: tip-bot2 for Giovanni Gherdovich @ 2020-06-16 12:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Ricardo Neri, Giovanni Gherdovich, Peter Zijlstra (Intel),
	Rafael J. Wysocki, x86, LKML

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

Commit-ID:     51beea8862a3095559862df39554f05042e1195b
Gitweb:        https://git.kernel.org/tip/51beea8862a3095559862df39554f05042e1195b
Author:        Giovanni Gherdovich <ggherdovich@suse.cz>
AuthorDate:    Sun, 31 May 2020 20:24:52 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Mon, 15 Jun 2020 14:10:02 +02:00

x86, sched: Bail out of frequency invariance if turbo frequency is unknown

There may be CPUs that support turbo boost but don't declare any turbo
ratio, i.e. their MSR_TURBO_RATIO_LIMIT is all zeroes. In that condition
scale-invariant calculations can't be performed.

Fixes: 1567c3e3467c ("x86, sched: Add support for frequency invariance")
Suggested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Link: https://lkml.kernel.org/r/20200531182453.15254-3-ggherdovich@suse.cz
---
 arch/x86/kernel/smpboot.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 18d292f..20e1cea 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -2002,9 +2002,11 @@ out:
 	/*
 	 * Some hypervisors advertise X86_FEATURE_APERFMPERF
 	 * but then fill all MSR's with zeroes.
+	 * Some CPUs have turbo boost but don't declare any turbo ratio
+	 * in MSR_TURBO_RATIO_LIMIT.
 	 */
-	if (!base_freq) {
-		pr_debug("Couldn't determine cpu base frequency, necessary for scale-invariant accounting.\n");
+	if (!base_freq || !turbo_freq) {
+		pr_debug("Couldn't determine cpu base or turbo frequency, necessary for scale-invariant accounting.\n");
 		return false;
 	}
 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [tip: sched/core] x86, sched: Bail out of frequency invariance if turbo_freq/base_freq gives 0
  2020-05-31 18:24 ` [PATCH v2 3/3] x86, sched: Bail out of frequency invariance if turbo_freq/base_freq gives 0 Giovanni Gherdovich
  2020-06-03 14:51   ` Rafael J. Wysocki
@ 2020-06-16 12:21   ` tip-bot2 for Giovanni Gherdovich
  1 sibling, 0 replies; 16+ messages in thread
From: tip-bot2 for Giovanni Gherdovich @ 2020-06-16 12:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Giovanni Gherdovich, Peter Zijlstra (Intel),
	Rafael J. Wysocki, x86, LKML

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

Commit-ID:     f4291df103315a696f0b8c4f45ca8ae773c17441
Gitweb:        https://git.kernel.org/tip/f4291df103315a696f0b8c4f45ca8ae773c17441
Author:        Giovanni Gherdovich <ggherdovich@suse.cz>
AuthorDate:    Sun, 31 May 2020 20:24:53 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Mon, 15 Jun 2020 14:10:02 +02:00

x86, sched: Bail out of frequency invariance if turbo_freq/base_freq gives 0

Be defensive against the case where the processor reports a base_freq
larger than turbo_freq (the ratio would be zero).

Fixes: 1567c3e3467c ("x86, sched: Add support for frequency invariance")
Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Link: https://lkml.kernel.org/r/20200531182453.15254-4-ggherdovich@suse.cz
---
 arch/x86/kernel/smpboot.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 20e1cea..518ac6b 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1977,6 +1977,7 @@ static bool core_set_max_freq_ratio(u64 *base_freq, u64 *turbo_freq)
 static bool intel_set_max_freq_ratio(void)
 {
 	u64 base_freq, turbo_freq;
+	u64 turbo_ratio;
 
 	if (slv_set_max_freq_ratio(&base_freq, &turbo_freq))
 		goto out;
@@ -2010,9 +2011,15 @@ out:
 		return false;
 	}
 
-	arch_turbo_freq_ratio = div_u64(turbo_freq * SCHED_CAPACITY_SCALE,
-					base_freq);
+	turbo_ratio = div_u64(turbo_freq * SCHED_CAPACITY_SCALE, base_freq);
+	if (!turbo_ratio) {
+		pr_debug("Non-zero turbo and base frequencies led to a 0 ratio.\n");
+		return false;
+	}
+
+	arch_turbo_freq_ratio = turbo_ratio;
 	arch_set_max_freq_ratio(turbo_disabled());
+
 	return true;
 }
 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [tip: sched/core] x86, sched: check for counters overflow in frequency invariant accounting
  2020-05-31 18:24 ` [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting Giovanni Gherdovich
  2020-06-03 14:22   ` Rafael J. Wysocki
@ 2020-06-16 12:21   ` tip-bot2 for Giovanni Gherdovich
  2020-10-22  8:46   ` [PATCH v2 1/3] " Peter Zijlstra
  2 siblings, 0 replies; 16+ messages in thread
From: tip-bot2 for Giovanni Gherdovich @ 2020-06-16 12:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Giovanni Gherdovich, Peter Zijlstra (Intel),
	Rafael J. Wysocki, x86, LKML

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

Commit-ID:     e2b0d619b400ae326f954a018a1d65d736c237c5
Gitweb:        https://git.kernel.org/tip/e2b0d619b400ae326f954a018a1d65d736c237c5
Author:        Giovanni Gherdovich <ggherdovich@suse.cz>
AuthorDate:    Sun, 31 May 2020 20:24:51 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Mon, 15 Jun 2020 14:10:02 +02:00

x86, sched: check for counters overflow in frequency invariant accounting

The product mcnt * arch_max_freq_ratio can overflows u64.

For context, a large value for arch_max_freq_ratio would be 5000,
corresponding to a turbo_freq/base_freq ratio of 5 (normally it's more like
1500-2000). A large increment frequency for the MPERF counter would be 5GHz
(the base clock of all CPUs on the market today is less than that). With
these figures, a CPU would need to go without a scheduler tick for around 8
days for the u64 overflow to happen. It is unlikely, but the check is
warranted.

Under similar conditions, the difference acnt of two consecutive APERF
readings can overflow as well.

In these circumstances is appropriate to disable frequency invariant
accounting: the feature relies on measures of the clock frequency done at
every scheduler tick, which need to be "fresh" to be at all meaningful.

A note on i386: prior to version 5.1, the GCC compiler didn't have the
builtin function __builtin_mul_overflow. In these GCC versions the macro
check_mul_overflow needs __udivdi3() to do (u64)a/b, which the kernel
doesn't provide. For this reason this change fails to build on i386 if
GCC<5.1, and we protect the entire frequency invariant code behind
CONFIG_X86_64 (special thanks to "kbuild test robot" <lkp@intel.com>).

Fixes: 1567c3e3467c ("x86, sched: Add support for frequency invariance")
Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Link: https://lkml.kernel.org/r/20200531182453.15254-2-ggherdovich@suse.cz
---
 arch/x86/include/asm/topology.h |  2 +-
 arch/x86/kernel/smpboot.c       | 33 +++++++++++++++++++++++++++-----
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 79d8d54..f423457 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -193,7 +193,7 @@ static inline void sched_clear_itmt_support(void)
 }
 #endif /* CONFIG_SCHED_MC_PRIO */
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) && defined(CONFIG_X86_64)
 #include <asm/cpufeature.h>
 
 DECLARE_STATIC_KEY_FALSE(arch_scale_freq_key);
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index ffbd9a3..18d292f 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -56,6 +56,7 @@
 #include <linux/cpuidle.h>
 #include <linux/numa.h>
 #include <linux/pgtable.h>
+#include <linux/overflow.h>
 
 #include <asm/acpi.h>
 #include <asm/desc.h>
@@ -1777,6 +1778,7 @@ void native_play_dead(void)
 
 #endif
 
+#ifdef CONFIG_X86_64
 /*
  * APERF/MPERF frequency ratio computation.
  *
@@ -2048,11 +2050,19 @@ static void init_freq_invariance(bool secondary)
 	}
 }
 
+static void disable_freq_invariance_workfn(struct work_struct *work)
+{
+	static_branch_disable(&arch_scale_freq_key);
+}
+
+static DECLARE_WORK(disable_freq_invariance_work,
+		    disable_freq_invariance_workfn);
+
 DEFINE_PER_CPU(unsigned long, arch_freq_scale) = SCHED_CAPACITY_SCALE;
 
 void arch_scale_freq_tick(void)
 {
-	u64 freq_scale;
+	u64 freq_scale = SCHED_CAPACITY_SCALE;
 	u64 aperf, mperf;
 	u64 acnt, mcnt;
 
@@ -2064,19 +2074,32 @@ void arch_scale_freq_tick(void)
 
 	acnt = aperf - this_cpu_read(arch_prev_aperf);
 	mcnt = mperf - this_cpu_read(arch_prev_mperf);
-	if (!mcnt)
-		return;
 
 	this_cpu_write(arch_prev_aperf, aperf);
 	this_cpu_write(arch_prev_mperf, mperf);
 
-	acnt <<= 2*SCHED_CAPACITY_SHIFT;
-	mcnt *= arch_max_freq_ratio;
+	if (check_shl_overflow(acnt, 2*SCHED_CAPACITY_SHIFT, &acnt))
+		goto error;
+
+	if (check_mul_overflow(mcnt, arch_max_freq_ratio, &mcnt) || !mcnt)
+		goto error;
 
 	freq_scale = div64_u64(acnt, mcnt);
+	if (!freq_scale)
+		goto error;
 
 	if (freq_scale > SCHED_CAPACITY_SCALE)
 		freq_scale = SCHED_CAPACITY_SCALE;
 
 	this_cpu_write(arch_freq_scale, freq_scale);
+	return;
+
+error:
+	pr_warn("Scheduler frequency invariance went wobbly, disabling!\n");
+	schedule_work(&disable_freq_invariance_work);
+}
+#else
+static inline void init_freq_invariance(bool secondary)
+{
 }
+#endif /* CONFIG_X86_64 */

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown
  2020-06-01 23:34   ` Ricardo Neri
@ 2020-07-06 20:19     ` Ira Weiny
  0 siblings, 0 replies; 16+ messages in thread
From: Ira Weiny @ 2020-07-06 20:19 UTC (permalink / raw)
  To: Ricardo Neri
  Cc: Giovanni Gherdovich, Srinivas Pandruvada, Thomas Gleixner,
	Ingo Molnar, Peter Zijlstra, Borislav Petkov, Rafael J . Wysocki,
	x86, linux-pm, linux-kernel

On Mon, Jun 01, 2020 at 04:34:18PM -0700, Ricardo Neri wrote:
> On Sun, May 31, 2020 at 08:24:52PM +0200, Giovanni Gherdovich wrote:
> > There may be CPUs that support turbo boost but don't declare any turbo
> > ratio, i.e. their MSR_TURBO_RATIO_LIMIT is all zeroes. In that condition
> > scale-invariant calculations can't be performed.
> > 
> > Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> > Suggested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> 
> FWIW,
> 
> Tested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

Is this going to make it as a fix to 5.8?

Tested-by: Ira Weiny <ira.weiny@intel.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting
  2020-05-31 18:24 ` [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting Giovanni Gherdovich
  2020-06-03 14:22   ` Rafael J. Wysocki
  2020-06-16 12:21   ` [tip: sched/core] " tip-bot2 for Giovanni Gherdovich
@ 2020-10-22  8:46   ` Peter Zijlstra
  2020-10-22 12:21     ` Giovanni Gherdovich
  2 siblings, 1 reply; 16+ messages in thread
From: Peter Zijlstra @ 2020-10-22  8:46 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Rafael J . Wysocki, x86, linux-pm, linux-kernel,
	Ricardo Neri

On Sun, May 31, 2020 at 08:24:51PM +0200, Giovanni Gherdovich wrote:

Hi Giovanni!

> +error:
> +	pr_warn("Scheduler frequency invariance went wobbly, disabling!\n");
> +	schedule_work(&disable_freq_invariance_work);
> +}

I'm getting reports that we trigger this on resume. Would it make sense
to hook into tsc_{save,restore}_sched_clock_state() (or somewhere near
there) to reset the state (basically call init_counter_refs() again to
ensure we're not having to deal with crazy ?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting
  2020-10-22  8:46   ` [PATCH v2 1/3] " Peter Zijlstra
@ 2020-10-22 12:21     ` Giovanni Gherdovich
  2020-10-22 12:30       ` Peter Zijlstra
  0 siblings, 1 reply; 16+ messages in thread
From: Giovanni Gherdovich @ 2020-10-22 12:21 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Rafael J . Wysocki, x86, linux-pm, linux-kernel,
	Ricardo Neri

On Thu, 2020-10-22 at 10:46 +0200, Peter Zijlstra wrote:
> On Sun, May 31, 2020 at 08:24:51PM +0200, Giovanni Gherdovich wrote:
> 
> Hi Giovanni!
> 
> > +error:
> > +	pr_warn("Scheduler frequency invariance went wobbly, disabling!\n");
> > +	schedule_work(&disable_freq_invariance_work);
> > +}
> 
> I'm getting reports that we trigger this on resume. Would it make sense
> to hook into tsc_{save,restore}_sched_clock_state() (or somewhere near
> there) to reset the state (basically call init_counter_refs() again to
> ensure we're not having to deal with crazy ?

Hello,

right, if the counters keep running while the machine is suspended then the
current code thinks it's a runtime overflow. I'll prepare a patch to fix that,
thanks for the heads-up and the hint.


Giovanni

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting
  2020-10-22 12:21     ` Giovanni Gherdovich
@ 2020-10-22 12:30       ` Peter Zijlstra
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Zijlstra @ 2020-10-22 12:30 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Srinivas Pandruvada, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Rafael J . Wysocki, x86, linux-pm, linux-kernel,
	Ricardo Neri

On Thu, Oct 22, 2020 at 02:21:58PM +0200, Giovanni Gherdovich wrote:
> On Thu, 2020-10-22 at 10:46 +0200, Peter Zijlstra wrote:
> > On Sun, May 31, 2020 at 08:24:51PM +0200, Giovanni Gherdovich wrote:
> > 
> > Hi Giovanni!
> > 
> > > +error:
> > > +	pr_warn("Scheduler frequency invariance went wobbly, disabling!\n");
> > > +	schedule_work(&disable_freq_invariance_work);
> > > +}
> > 
> > I'm getting reports that we trigger this on resume. Would it make sense
> > to hook into tsc_{save,restore}_sched_clock_state() (or somewhere near
> > there) to reset the state (basically call init_counter_refs() again to
> > ensure we're not having to deal with crazy ?
> 
> Hello,
> 
> right, if the counters keep running while the machine is suspended then the
> current code thinks it's a runtime overflow. I'll prepare a patch to fix that,
> thanks for the heads-up and the hint.

Either keep running, get reset, get scrambled, everything is possible.

Thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-10-22 12:30 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-31 18:24 [PATCH v2 0/3] More frequency invariance fixes for x86 Giovanni Gherdovich
2020-05-31 18:24 ` [PATCH v2 1/3] x86, sched: check for counters overflow in frequency invariant accounting Giovanni Gherdovich
2020-06-03 14:22   ` Rafael J. Wysocki
2020-06-16 12:21   ` [tip: sched/core] " tip-bot2 for Giovanni Gherdovich
2020-10-22  8:46   ` [PATCH v2 1/3] " Peter Zijlstra
2020-10-22 12:21     ` Giovanni Gherdovich
2020-10-22 12:30       ` Peter Zijlstra
2020-05-31 18:24 ` [PATCH v2 2/3] x86, sched: Bail out of frequency invariance if turbo frequency is unknown Giovanni Gherdovich
2020-06-01 23:34   ` Ricardo Neri
2020-07-06 20:19     ` Ira Weiny
2020-06-03 14:23   ` Rafael J. Wysocki
2020-06-16 12:21   ` [tip: sched/core] " tip-bot2 for Giovanni Gherdovich
2020-05-31 18:24 ` [PATCH v2 3/3] x86, sched: Bail out of frequency invariance if turbo_freq/base_freq gives 0 Giovanni Gherdovich
2020-06-03 14:51   ` Rafael J. Wysocki
2020-06-16 12:21   ` [tip: sched/core] " tip-bot2 for Giovanni Gherdovich
2020-06-03 12:31 ` [PATCH v2 0/3] More frequency invariance fixes for x86 Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.