Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190612163345.GB26997@sinkpad> <635c01b0-d8f3-561b-5396-10c75ed03712@oracle.com>
 <20190613032246.GA17752@sinkpad> <CAERHkrsMFjjBpPZS7jDhzbob4PSmiPj83OfqEeiKgaDAU3ajOA@mail.gmail.com>
 <20190619183302.GA6775@sinkpad> <20190718100714.GA469@aaronlu>
 <CAERHkrtvLKxrpvfw04urAuougsYOWnNw4-H1vUDFx27Dvy0=Ww@mail.gmail.com>
 <20190725143003.GA992@aaronlu> <20190726152101.GA27884@sinkpad>
 <7dc86e3c-aa3f-905f-3745-01181a3b0dac@linux.intel.com> <20190802153715.GA18075@sinkpad>
 <eec72c2d533b7600c63de3c8001cc6ab9e915afe.camel@suse.com> <69cd9bca-da28-1d35-3913-1efefe0c1c22@linux.intel.com>
 <fab8eabb-1cfa-9bf6-02af-3afdff3f955d@linux.intel.com>
In-Reply-To: <fab8eabb-1cfa-9bf6-02af-3afdff3f955d@linux.intel.com>
From:   Aubrey Li <aubrey.intel@gmail.com>
Date:   Wed, 25 Sep 2019 10:40:09 +0800
Message-ID: <CAERHkrurxjDvktGD+Xfjx7Bo5JtTCuqdYToQaDJTNbbRufFWMw@mail.gmail.com>
Subject: Re: [RFC PATCH v3 00/16] Core scheduling v3
To:     Tim Chen <tim.c.chen@linux.intel.com>
Cc:     Dario Faggioli <dfaggioli@suse.com>,
        Julien Desfossez <jdesfossez@digitalocean.com>,
        "Li, Aubrey" <aubrey.li@linux.intel.com>,
        Aaron Lu <aaron.lu@linux.alibaba.com>,
        Subhra Mazumdar <subhra.mazumdar@oracle.com>,
        Vineeth Remanan Pillai <vpillai@digitalocean.com>,
        Nishanth Aravamudan <naravamudan@digitalocean.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Paul Turner <pjt@google.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
        =?UTF-8?B?RnLDqWTDqXJpYyBXZWlzYmVja2Vy?= <fweisbec@gmail.com>,
        Kees Cook <keescook@chromium.org>,
        Greg Kerr <kerrnel@google.com>, Phil Auld <pauld@redhat.com>,
        Valentin Schneider <valentin.schneider@arm.com>,
        Mel Gorman <mgorman@techsingularity.net>,
        Pawan Gupta <pawan.kumar.gupta@linux.intel.com>,
        Paolo Bonzini <pbonzini@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Sep 7, 2019 at 2:30 AM Tim Chen <tim.c.chen@linux.intel.com> wrote:
> +static inline s64 core_sched_imbalance_delta(int src_cpu, int dst_cpu,
> +                       int src_sibling, int dst_sibling,
> +                       struct task_group *tg, u64 task_load)
> +{
> +       struct sched_entity *se, *se_sibling, *dst_se, *dst_se_sibling;
> +       s64 excess, deficit, old_mismatch, new_mismatch;
> +
> +       if (src_cpu == dst_cpu)
> +               return -1;
> +
> +       /* XXX SMT4 will require additional logic */
> +
> +       se = tg->se[src_cpu];
> +       se_sibling = tg->se[src_sibling];
> +
> +       excess = se->avg.load_avg - se_sibling->avg.load_avg;
> +       if (src_sibling == dst_cpu) {
> +               old_mismatch = abs(excess);
> +               new_mismatch = abs(excess - 2*task_load);
> +               return old_mismatch - new_mismatch;
> +       }
> +
> +       dst_se = tg->se[dst_cpu];
> +       dst_se_sibling = tg->se[dst_sibling];
> +       deficit = dst_se->avg.load_avg - dst_se_sibling->avg.load_avg;
> +
> +       old_mismatch = abs(excess) + abs(deficit);
> +       new_mismatch = abs(excess - (s64) task_load) +
> +                      abs(deficit + (s64) task_load);

If I understood correctly, these formulas made an assumption that the task
being moved to the destination is matched the destination's core cookie. so if
the task is not matched with dst's core cookie and still have to stay
in the runqueue
then the formula becomes not correct.

>  /**
>   * update_sg_lb_stats - Update sched_group's statistics for load balancing.
>   * @env: The load balancing environment.
> @@ -8345,7 +8492,8 @@ static inline void update_sg_lb_stats(struct lb_env *env,
>                 else
>                         load = source_load(i, load_idx);
>
> -               sgs->group_load += load;

Why is this load update line removed?

> +               core_sched_imbalance_scan(sgs, i, env->dst_cpu);
> +
>                 sgs->group_util += cpu_util(i);
>                 sgs->sum_nr_running += rq->cfs.h_nr_running;
>

Thanks,
-Aubrey