lkml.org 
[lkml]   [2019]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH AUTOSEL 5.0 27/67] sched/cpufreq: Fix 32-bit math overflow
    Date
    From: Peter Zijlstra <peterz@infradead.org>

    [ Upstream commit a23314e9d88d89d49e69db08f60b7caa470f04e1 ]

    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>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    kernel/sched/cpufreq_schedutil.c | 59 ++++++++++++++------------------
    1 file changed, 25 insertions(+), 34 deletions(-)

    diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
    index 033ec7c45f13..1ccf77f6d346 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,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) {
    --
    2.19.1
    \
     
     \ /
      Last update: 2019-03-30 02:11    [W:4.280 / U:0.648 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site