lkml.org 
[lkml]   [2020]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH] sched/fair: Fix nohz.next_balance update
    commit c5afb6a87f23 ("sched/fair: Fix nohz.next_balance update")
    During idle load balance, this_cpu(ilb) do load balance for the other
    idle CPUs, also gather the earliest (nohz.)next_balance.

    Since commit:
    'b7031a02ec75 ("sched/fair: Add NOHZ_STATS_KICK")'

    We update nohz.next_balance like this:

    _nohz_idle_balance() {
    for_each_cpu(nohz.idle_cpus_mask) {
    rebalance_domains() {
    update nohz.next_balance <-- compare and update
    }
    }
    rebalance_domains(this_cpu) {
    update nohz.next_balance <-- compare and update
    }
    update nohz.next_balance <-- unconditionally update
    }

    For instance, nohz.idle_cpus_mask spans {cpu2,3,5,8}, and this_cpu is
    cpu5. After the above loop we could gather the earliest *next_balance*
    among {cpu2,3,8}, then rebalance_domains(this_cpu) update
    nohz.next_balance with this_rq->next_balance, but finally overwrite
    nohz.next_balance with the earliest *next_balance* among {cpu2,3,8},
    we may end up with not getting the earliest next_balance.

    Since we can gather all the updated rq->next_balance, including this_cpu,
    in _nohz_idle_balance(), it's safe to remove the extra lines in
    rebalance_domains() which are originally intended for this_cpu. And
    finally the updating only happen in _nohz_idle_balance().

    Signed-off-by: Peng Liu <iwtbavbm@gmail.com>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Juri Lelli <juri.lelli@redhat.com>
    Cc: Vincent Guittot <vincent.guittot@linaro.org>
    Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
    Cc: Steven Rostedt <rostedt@goodmis.org>
    Cc: Ben Segall <bsegall@google.com>
    Cc: Mel Gorman <mgorman@suse.de>
    Cc: Valentin Schneider <valentin.schneider@arm.com>
    ---
    kernel/sched/fair.c | 24 ++++++++----------------
    1 file changed, 8 insertions(+), 16 deletions(-)

    diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
    index 02f323b85b6d..1d0cf33fefad 100644
    --- a/kernel/sched/fair.c
    +++ b/kernel/sched/fair.c
    @@ -9943,22 +9943,8 @@ static void rebalance_domains(struct rq *rq, enum cpu_idle_type idle)
    * When the cpu is attached to null domain for ex, it will not be
    * updated.
    */
    - if (likely(update_next_balance)) {
    + if (likely(update_next_balance))
    rq->next_balance = next_balance;
    -
    -#ifdef CONFIG_NO_HZ_COMMON
    - /*
    - * If this CPU has been elected to perform the nohz idle
    - * balance. Other idle CPUs have already rebalanced with
    - * nohz_idle_balance() and nohz.next_balance has been
    - * updated accordingly. This CPU is now running the idle load
    - * balance for itself and we need to update the
    - * nohz.next_balance accordingly.
    - */
    - if ((idle == CPU_IDLE) && time_after(nohz.next_balance, rq->next_balance))
    - nohz.next_balance = rq->next_balance;
    -#endif
    - }
    }

    static inline int on_null_domain(struct rq *rq)
    @@ -10321,9 +10307,15 @@ static bool _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
    has_blocked_load |= this_rq->has_blocked_load;
    }

    - if (flags & NOHZ_BALANCE_KICK)
    + if (flags & NOHZ_BALANCE_KICK) {
    rebalance_domains(this_rq, CPU_IDLE);

    + if (time_after(next_balance, this_rq->next_balance)) {
    + next_balance = this_rq->next_balance;
    + update_next_balance = 1;
    + }
    + }
    +
    WRITE_ONCE(nohz.next_blocked,
    now + msecs_to_jiffies(LOAD_AVG_PERIOD));

    --
    2.17.1
    \
     
     \ /
      Last update: 2020-05-03 10:34    [W:3.636 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site