Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220916080305.29574-6-vincent.guittot@linaro.org> <20220916120245.2951-1-hdanton@sina.com>
In-Reply-To: <20220916120245.2951-1-hdanton@sina.com>
From:   Vincent Guittot <vincent.guittot@linaro.org>
Date:   Fri, 16 Sep 2022 15:36:53 +0200
Message-ID: <CAKfTPtDV+ACXtQGH38sXwFVJfq+EzyKO1mCMZ68Q+GC5kw7-ww@mail.gmail.com>
Subject: Re: [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup
To:     Hillf Danton <hdanton@sina.com>
Cc:     peterz@infradead.org, mgorman@suse.de, valentin.schneider@arm.com,
        parth@linux.ibm.com, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Hillf,

On Fri, 16 Sept 2022 at 14:03, Hillf Danton <hdanton@sina.com> wrote:
>
> Hello Vincent
>
> On 16 Sep 2022 10:03:02 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote:
> >
> > @@ -4606,6 +4608,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> >
> >       se = __pick_first_entity(cfs_rq);
> >       delta = curr->vruntime - se->vruntime;
> > +     delta -= wakeup_latency_gran(curr, se);
> >
> >       if (delta < 0)
> >               return;
>
> What is derived from the latency nice you added is the runtime granulaity
> which has a role in preempting the current task.
>
> Given the same defination of latency nice as the nice, the runtime granularity
> can be computed without introducing the latency nice.
>
> Only for thoughts now.
>
> Hillf
>
> +++ b/kernel/sched/fair.c
> @@ -4569,7 +4569,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, st
>  static void
>  check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
>  {
> -       unsigned long ideal_runtime, delta_exec;
> +       unsigned long ideal_runtime, delta_exec, granu;
>         struct sched_entity *se;
>         s64 delta;
>
> @@ -4594,6 +4594,14 @@ check_preempt_tick(struct cfs_rq *cfs_rq
>                 return;
>
>         se = __pick_first_entity(cfs_rq);
> +
> +       granu = sysctl_sched_min_granularity +
> +               (ideal_runtime - sysctl_sched_min_granularity) *
> +               (se->latency_nice + 20) / LATENCY_NICE_WIDTH;

There is no latency_nice field in se but a latency_offset instead

Also at this step, we are sure that curr has run at least
sysctl_sched_min_granularity and we want now to compare curr vruntime
with first se's one. We take the latency offset into account to make
sure we will not preempt curr too early

> +
> +       if (delta_exec < granu)
> +               return;
> +
>         delta = curr->vruntime - se->vruntime;
>
>         if (delta < 0)