linux-kernel - Re: [PATCH] sched/fair: Allow non-idle task to preempt idle task directly

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: <CAKfTPtCNLELdiQXCdbxrXotqCkRV-2mKK7uVXQv---mbnekaUQ@mail.gmail.com>
Date:   Fri, 1 Apr 2022 15:09:59 +0200
From:   Vincent Guittot <vincent.guittot@...aro.org>
To:     zhangsong <zhangsong34@...wei.com>
Cc:     peterz@...radead.org, mingo@...hat.com, juri.lelli@...hat.com,
        dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
        mgorman@...e.de, bristot@...hat.com, linux-kernel@...r.kernel.org,
        zhangsong <zhangsong34@...il.com>
Subject: Re: [PATCH] sched/fair: Allow non-idle task to preempt idle task directly

On Fri, 1 Apr 2022 at 11:13, zhangsong <zhangsong34@...wei.com> wrote:
>
> From: zhangsong <zhangsong34@...il.com>
>
> In check_preempt_tick(), the sched idle task may exectue at least
> `sysctl_sched_min_granularity` time but any other cfs tasks cannot
> preempt it. So it is nessesary to ignore the `sysctl_sched_min_granularity`
> resctriction for sched idle task preemption.

Could you explain why you need to remove this condition for sched_idle ?
sched_idle tasks are already preempted at wakeup by others. And they
run while others are runnable only if they has not run for a very long
time compares to other. The ideal_runtime of a sched_idle task is
capped to 750us min to ensure a minimum progress. But this will happen
not more than once  every 256ms and most probably even less often.

>
> Signed-off-by: zhangsong <zhangsong34@...il.com>
> ---
>  kernel/sched/fair.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d4bd299d6..edcb33440 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4477,6 +4477,15 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
>         struct sched_entity *se;
>         s64 delta;
>
> +       se = __pick_first_entity(cfs_rq);
> +
> +       if ((cfs_rq->last && se_is_idle(cfs_rq->last) - se_is_idle(curr) < 0) ||
> +           (cfs_rq->next && se_is_idle(cfs_rq->last) - se_is_idle(curr) < 0) ||
> +           se_is_idle(se) - se_is_idle(curr) < 0) {
> +               resched_curr(rq_of(cfs_rq));
> +               return;

Why all these complex conditions ?
if (se_is_idle(curr)) should be enough


> +       }
> +
>         ideal_runtime = sched_slice(cfs_rq, curr);
>         delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime;
>         if (delta_exec > ideal_runtime) {
> @@ -4497,7 +4506,6 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr)
>         if (delta_exec < sysctl_sched_min_granularity)
>                 return;
>
> -       se = __pick_first_entity(cfs_rq);
>         delta = curr->vruntime - se->vruntime;
>
>         if (delta < 0)
> --
> 2.27.0
>