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

[Vincent Guittot]

On Fri, 1 Apr 2022 at 11:13, zhangsong <zhangsong34@xxxxxxxxxx> wrote:
>
> From: zhangsong <zhangsong34@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
>  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
>