From: Peter Zijlstra <peterz@infradead.org>
To: Quentin Perret <quentin.perret@arm.com>
Cc: "Wang, Vincent (王争)" <Vincent.Wang@unisoc.com>,
"Zhang, Chunyan (张春艳)" <Chunyan.Zhang@unisoc.com>,
"Ingo Molnar" <mingo@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Chunyan Zhang" <zhang.lyra@gmail.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>
Subject: [PATCH] sched/cpufreq: Fix 32bit math overflow
Date: Tue, 5 Mar 2019 09:32:02 +0100 [thread overview]
Message-ID: <20190305083202.GU32494@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20190304191059.mzwid3udqwtzejww@queper01-lin>
On Mon, Mar 04, 2019 at 07:11:01PM +0000, Quentin Perret wrote:
> So yeah, that works for me.
Chunyan, Vincent; can you verify the below cures your ill?
---
Subject: sched/cpufreq: Fix 32bit math overflow
Vincent Wang reported that get_next_freq() has a mult overflow issue on
32bit platforms in the IOWAIT boost case, since in that case {util,max}
are in freq units instead of capacity units.
Solve this by moving the IOWAIT boost to capacity units. And since this
means @max is constant; simplify the code.
Cc: Chunyan Zhang <chunyan.zhang@unisoc.com>
Reported-by: Vincent Wang <vincent.wang@unisoc.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/sched/cpufreq_schedutil.c | 58 +++++++++++++++++-----------------------
1 file changed, 24 insertions(+), 34 deletions(-)
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 2efe629425be..72b62ac1c7c2 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -48,10 +48,10 @@ struct sugov_cpu {
bool iowait_boost_pending;
unsigned int iowait_boost;
- unsigned int iowait_boost_max;
u64 last_update;
unsigned long bw_dl;
+ unsigned long min;
unsigned long max;
/* The field below is for single-CPU policies only: */
@@ -303,8 +303,7 @@ static bool sugov_iowait_reset(struct sugov_cpu *sg_cpu, u64 time,
if (delta_ns <= TICK_NSEC)
return false;
- sg_cpu->iowait_boost = set_iowait_boost
- ? sg_cpu->sg_policy->policy->min : 0;
+ sg_cpu->iowait_boost = set_iowait_boost ? sg_cpu->min : 0;
sg_cpu->iowait_boost_pending = set_iowait_boost;
return true;
@@ -344,14 +343,12 @@ static void sugov_iowait_boost(struct sugov_cpu *sg_cpu, u64 time,
/* Double the boost at each request */
if (sg_cpu->iowait_boost) {
- sg_cpu->iowait_boost <<= 1;
- if (sg_cpu->iowait_boost > sg_cpu->iowait_boost_max)
- sg_cpu->iowait_boost = sg_cpu->iowait_boost_max;
+ sg_cpu->iowait_boost = min(sg_cpu->iowait_boost << 1, SCHED_CAPACITY_SCALE);
return;
}
/* First wakeup after IO: start with minimum boost */
- sg_cpu->iowait_boost = sg_cpu->sg_policy->policy->min;
+ sg_cpu->iowait_boost = sg_cpu->min;
}
/**
@@ -373,47 +370,38 @@ static void sugov_iowait_boost(struct sugov_cpu *sg_cpu, u64 time,
* This mechanism is designed to boost high frequently IO waiting tasks, while
* being more conservative on tasks which does sporadic IO operations.
*/
-static void sugov_iowait_apply(struct sugov_cpu *sg_cpu, u64 time,
- unsigned long *util, unsigned long *max)
+static unsigned long sugov_iowait_apply(struct sugov_cpu *sg_cpu, u64 time,
+ unsigned long util, unsigned long max)
{
- unsigned int boost_util, boost_max;
+ unsigned long boost;
/* No boost currently required */
if (!sg_cpu->iowait_boost)
- return;
+ return util;
/* Reset boost if the CPU appears to have been idle enough */
if (sugov_iowait_reset(sg_cpu, time, false))
- return;
+ return util;
- /*
- * An IO waiting task has just woken up:
- * allow to further double the boost value
- */
- if (sg_cpu->iowait_boost_pending) {
- sg_cpu->iowait_boost_pending = false;
- } else {
+ if (!sg_cpu->iowait_boost_pending) {
/*
- * Otherwise: reduce the boost value and disable it when we
- * reach the minimum.
+ * No boost pending; reduce the boost value.
*/
sg_cpu->iowait_boost >>= 1;
- if (sg_cpu->iowait_boost < sg_cpu->sg_policy->policy->min) {
+ if (sg_cpu->iowait_boost < sg_cpu->min) {
sg_cpu->iowait_boost = 0;
- return;
+ return util;
}
}
+ sg_cpu->iowait_boost_pending = false;
+
/*
- * Apply the current boost value: a CPU is boosted only if its current
- * utilization is smaller then the current IO boost level.
+ * @util is already in capacity scale; convert iowait_boost
+ * into the same scale so we can compare.
*/
- boost_util = sg_cpu->iowait_boost;
- boost_max = sg_cpu->iowait_boost_max;
- if (*util * boost_max < *max * boost_util) {
- *util = boost_util;
- *max = boost_max;
- }
+ boost = (sg_cpu->iowait_boost * max) >> SCHED_CAPACITY_SHIFT;
+ return max(boost, util);
}
#ifdef CONFIG_NO_HZ_COMMON
@@ -460,7 +448,7 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
util = sugov_get_util(sg_cpu);
max = sg_cpu->max;
- sugov_iowait_apply(sg_cpu, time, &util, &max);
+ util = sugov_iowait_apply(sg_cpu, time, util, max);
next_f = get_next_freq(sg_policy, util, max);
/*
* Do not reduce the frequency if the CPU has not been idle
@@ -500,7 +488,7 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
j_util = sugov_get_util(j_sg_cpu);
j_max = j_sg_cpu->max;
- sugov_iowait_apply(j_sg_cpu, time, &j_util, &j_max);
+ j_util = sugov_iowait_apply(j_sg_cpu, time, j_util, j_max);
if (j_util * max > j_max * util) {
util = j_util;
@@ -837,7 +825,9 @@ static int sugov_start(struct cpufreq_policy *policy)
memset(sg_cpu, 0, sizeof(*sg_cpu));
sg_cpu->cpu = cpu;
sg_cpu->sg_policy = sg_policy;
- sg_cpu->iowait_boost_max = policy->cpuinfo.max_freq;
+ sg_cpu->min =
+ (SCHED_CAPACITY_SCALE * policy->cpuinfo.min_freq) /
+ policy->cpuinfo.max_freq;
}
for_each_cpu(cpu, policy->cpus) {
next prev parent reply other threads:[~2019-03-05 8:32 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-22 10:37 [PATCH V4] sched/cpufreq: initialize iowait_boost_max and iowait_boost with cpu capacity Chunyan Zhang
2019-02-22 10:59 ` Quentin Perret
2019-03-04 7:35 ` 答复: " Wang, Vincent (王争)
2019-03-04 13:58 ` Quentin Perret
2019-03-04 15:26 ` Peter Zijlstra
2019-03-04 16:48 ` Quentin Perret
2019-03-04 17:40 ` Peter Zijlstra
2019-03-04 17:44 ` Peter Zijlstra
2019-03-04 17:50 ` Quentin Perret
2019-03-04 18:47 ` Peter Zijlstra
2019-03-04 19:11 ` Quentin Perret
2019-03-05 8:32 ` Peter Zijlstra [this message]
2019-03-05 10:55 ` [PATCH] sched/cpufreq: Fix 32bit math overflow Rafael J. Wysocki
2019-03-06 2:01 ` Chunyan Zhang
2019-03-06 7:50 ` Chunyan Zhang
2019-03-09 14:35 ` [tip:sched/urgent] sched/cpufreq: Fix 32-bit " tip-bot for Peter Zijlstra
2019-03-19 11:10 ` tip-bot for Peter Zijlstra
2019-03-04 17:40 ` 答复: [PATCH V4] sched/cpufreq: initialize iowait_boost_max and iowait_boost with cpu capacity Vincent Guittot
2019-03-04 17:49 ` Peter Zijlstra
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=20190305083202.GU32494@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=Chunyan.Zhang@unisoc.com \
--cc=Vincent.Wang@unisoc.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=quentin.perret@arm.com \
--cc=rjw@rjwysocki.net \
--cc=zhang.lyra@gmail.com \
/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;
as well as URLs for NNTP newsgroup(s).