sched/cpufreq: Fix 32-bit math overflow

Vincent Wang reported that get_next_freq() has a mult overflow bug on
32-bit 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.

Reported-by: Vincent Wang <vincent.wang@unisoc.com>
Tested-by: Vincent Wang <vincent.wang@unisoc.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Quentin Perret <quentin.perret@arm.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Rik van Riel <riel@surriel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20190305083202.GU32494@hirez.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 033ec7c..1ccf77f 100644 (file)
--- 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,13 @@ 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_t(unsigned int, 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 +371,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 +449,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 +489,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 +826,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) {