linux-kernel - Re: [PATCH v1] sched/cfs: Fix pick_next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKfTPtBn0Bh4DjSs_nATYPvRt5Ks2Dg8n0TH7vuxtFfKFTxG5A@mail.gmail.com>
Date:   Wed, 1 Jul 2020 12:47:29 +0200
From:   Vincent Guittot <vincent.guittot@...aro.org>
To:     Zijun Hu <zijuhu@...eaurora.org>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] sched/cfs: Fix pick_next_entity() implementation error

On Wed, 1 Jul 2020 at 12:07, Zijun Hu <zijuhu@...eaurora.org> wrote:
>
> sched_entity @se not static @left should be compared
> to pick up @cfs_rq->next.

Could you elaborate why ?

left is the leftmost sched_entity and the one that should be used.

se != left means that left should be skipped after a yield and the
next se in the rbtree is not "far" from left although it has higher
vruntime

if we finally want to use last or next instead of se, we must ensure
that they are still not "far" from left otherwise you can promote a
sched entity that ends up having a really high vruntime

>
> Signed-off-by: Zijun Hu <zijuhu@...eaurora.org>
> ---
>  kernel/sched/fair.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 658aa7a2ae6f..4790f2d851ad 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4452,13 +4452,13 @@ pick_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *curr)
>         /*
>          * Prefer last buddy, try to return the CPU to a preempted task.
>          */
> -       if (cfs_rq->last && wakeup_preempt_entity(cfs_rq->last, left) < 1)
> +       if (cfs_rq->last && wakeup_preempt_entity(cfs_rq->last, se) < 1)
>                 se = cfs_rq->last;
>
>         /*
>          * Someone really wants this to run. If it's not unfair, run it.
>          */
> -       if (cfs_rq->next && wakeup_preempt_entity(cfs_rq->next, left) < 1)
> +       if (cfs_rq->next && wakeup_preempt_entity(cfs_rq->next, se) < 1)
>                 se = cfs_rq->next;
>
>         clear_buddies(cfs_rq, se);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
>