Re: [PATCH 1/2] sched/fair: skip newidle update stats

[Vincent Guittot]

On Fri, 12 Nov 2021 at 15:55, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
>
> Subject: sched/fair: Reflow newidle_balance()
> From: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Date: Fri Nov 12 15:46:08 CET 2021
>
> The control flow in newidle_balance() is a little convoluted, attempt
> simplification.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> ---
>  kernel/sched/fair.c |   21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
>
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10858,10 +10858,10 @@ static inline void nohz_newidle_balance(
>  static int newidle_balance(struct rq *this_rq, struct rq_flags *rf)
>  {
>         unsigned long next_balance = jiffies + HZ;
> +       int pulled_task = 0, timeout = 0;
>         int this_cpu = this_rq->cpu;
>         u64 t0, t1, curr_cost = 0;
>         struct sched_domain *sd;
> -       int pulled_task = 0, early_stop = 0;
>
>         update_misfit_status(NULL, this_rq);
>
> @@ -10889,17 +10889,9 @@ static int newidle_balance(struct rq *th
>         if (!READ_ONCE(this_rq->rd->overload) ||
>             (sd && this_rq->avg_idle < sd->max_newidle_lb_cost)) {
>
> -               if (sd) {
> +               if (sd)
>                         update_next_balance(sd, &next_balance);
>
> -                       /*
> -                        * We skip new idle LB because there is not enough
> -                        * time before next wake up. Make sure that we will
> -                        * not kick NOHZ_NEWILB_KICK
> -                        */
> -                       early_stop = 1;
> -               }
> -
>                 goto out;
>         }
>
> @@ -10922,7 +10914,7 @@ static int newidle_balance(struct rq *th
>                 update_next_balance(sd, &next_balance);
>
>                 if (this_rq->avg_idle < curr_cost + sd->max_newidle_lb_cost) {
> -                       early_stop = 1;
> +                       timeout = 1;
>                         break;
>                 }
>
> @@ -10967,6 +10959,11 @@ static int newidle_balance(struct rq *th
>         if (this_rq->nr_running != this_rq->cfs.h_nr_running)
>                 pulled_task = -1;
>
> +       if (pulled_task || timeout)
> +               goto out;
> +
> +       nohz_newidle_balance(this_rq);

maybe

       if (!pulled_task && !timeout)
               nohz_newidle_balance(this_rq);

> +
>  out:
>         /* Move the next balance forward */
>         if (time_after(this_rq->next_balance, next_balance))
> @@ -10974,8 +10971,6 @@ static int newidle_balance(struct rq *th
>
>         if (pulled_task)
>                 this_rq->idle_stamp = 0;
> -       else if (!early_stop)
> -               nohz_newidle_balance(this_rq);
>
>         return pulled_task;
>  }