mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Yair Podemsky <ypodemsk@redhat.com>
To: x86@kernel.org, tglx@linutronix.de, mingo@redhat.com,
	peterz@infradead.org, rafael.j.wysocki@intel.com,
	pauld@redhat.com, frederic@kernel.org, ggherdovich@suse.cz,
	linux-kernel@vger.kernel.org, lenb@kernel.org,
	vschneid@redhat.com, jlelli@redhat.com, mtosatti@redhat.com,
	ppandit@redhat.com, alougovs@redhat.com, lcapitul@redhat.com,
	nsaenz@kernel.org
Cc: ypodemsk@redhat.com
Subject: [PATCH] x86/aperfmperf: Fix arch_scale_freq_tick() on tickless systems
Date: Thu,  4 Aug 2022 16:17:28 +0300	[thread overview]
Message-ID: <20220804131728.58513-1-ypodemsk@redhat.com> (raw)

In order for the scheduler to be frequency invariant we measure the
ratio between the maximum cpu frequency and the actual cpu frequency.
During long tickless periods of time the calculations that keep track
of that might overflow, in the function scale_freq_tick():

if (check_shl_overflow(acnt, 2*SCHED_CAPACITY_SHIFT, &acnt))
»       goto error;

eventually forcing the kernel to disable the feature with the
message "Scheduler frequency invariance went wobbly, disabling!".
Let's avoid that by detecting long tickless periods and bypassing
the calculation for that tick.

This calculation updates the value of arch_freq_scale, used by the
capacity-aware scheduler to correct cpu duty cycles:
task_util_freq_inv(p) = duty_cycle(p) * (curr_frequency(cpu) /
max_frequency(cpu))

However Consider a long tickless period, It takes should take 60 minutes
for a tickless CPU running at 5GHz to trigger the acnt overflow,
pick 10 minutes as a staleness threshold to be on the safe side,
In our testing it took over 30 minutes for the overflow to happen,
but since it's frequency/platform dependent we choose a smaller value
to be on the safe side.

Fixes: e2b0d619b400 ("x86, sched: check for counters overflow in frequency invariant accounting")
Signed-off-by: Yair Podemsky <ypodemsk@redhat.com>
---
 arch/x86/kernel/cpu/aperfmperf.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/aperfmperf.c b/arch/x86/kernel/cpu/aperfmperf.c
index 1f60a2b27936..dfe356034a60 100644
--- a/arch/x86/kernel/cpu/aperfmperf.c
+++ b/arch/x86/kernel/cpu/aperfmperf.c
@@ -23,6 +23,13 @@
 
 #include "cpu.h"
 
+/*
+ * Samples older then 10 minutes should not be proccessed,
+ * This time is long enough to prevent unneeded drops of data
+ * But short enough to prevent overflows
+ */
+#define MAX_SAMPLE_AGE_NOHZ	((unsigned long)HZ * 600)
+
 struct aperfmperf {
 	seqcount_t	seq;
 	unsigned long	last_update;
@@ -373,6 +380,7 @@ static inline void scale_freq_tick(u64 acnt, u64 mcnt) { }
 void arch_scale_freq_tick(void)
 {
 	struct aperfmperf *s = this_cpu_ptr(&cpu_samples);
+	unsigned long last  = s->last_update;
 	u64 acnt, mcnt, aperf, mperf;
 
 	if (!cpu_feature_enabled(X86_FEATURE_APERFMPERF))
@@ -392,7 +400,12 @@ void arch_scale_freq_tick(void)
 	s->mcnt = mcnt;
 	raw_write_seqcount_end(&s->seq);
 
-	scale_freq_tick(acnt, mcnt);
+	/*
+	 * Avoid calling scale_freq_tick() when the last update was too long ago,
+	 * as it might overflow during calulation.
+	 */
+	if ((jiffies - last) <= MAX_SAMPLE_AGE_NOHZ)
+		scale_freq_tick(acnt, mcnt);
 }
 
 /*
-- 
2.31.1


             reply	other threads:[~2022-08-04 13:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-04 13:17 Yair Podemsky [this message]
2022-09-06 12:16 ` ypodemsk
2022-09-06 14:54 ` Peter Zijlstra
2022-09-06 16:17   ` Valentin Schneider
2022-10-19 11:31     ` ypodemsk
2022-11-02  8:55       ` ypodemsk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220804131728.58513-1-ypodemsk@redhat.com \
    --to=ypodemsk@redhat.com \
    --cc=alougovs@redhat.com \
    --cc=frederic@kernel.org \
    --cc=ggherdovich@suse.cz \
    --cc=jlelli@redhat.com \
    --cc=lcapitul@redhat.com \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=nsaenz@kernel.org \
    --cc=pauld@redhat.com \
    --cc=peterz@infradead.org \
    --cc=ppandit@redhat.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=tglx@linutronix.de \
    --cc=vschneid@redhat.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®