Re: [PATCH] sched/fair: Make SCHED_IDLE se be preempted in strict hierarchy

[Josh Don]

Hi Tianchen,

Thanks for these fixes.

>  #endif /* CONFIG_FAIR_GROUP_SCHED */
> @@ -8382,16 +8382,7 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>         if (test_tsk_need_resched(curr))
>                 return;
>
> -       /* Idle tasks are by definition preempted by non-idle tasks. */
> -       if (unlikely(task_has_idle_policy(curr)) &&
> -           likely(!task_has_idle_policy(p)))
> -               goto preempt;
> -
> -       /*
> -        * Batch and idle tasks do not preempt non-idle tasks (their preemption
> -        * is driven by the tick):
> -        */
> -       if (unlikely(p->policy != SCHED_NORMAL) || !sched_feat(WAKEUP_PREEMPTION))
> +       if (!sched_feat(WAKEUP_PREEMPTION))
>                 return;
>
>         find_matching_se(&se, &pse);
> @@ -8408,6 +8399,12 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>                 goto preempt;
>         if (cse_is_idle != pse_is_idle)
>                 return;

nit: add newline

> +       /*
> +        * Batch tasks do not preempt non-idle tasks (their preemption
> +        * is driven by the tick):
> +        */
> +       if (unlikely(pse == &p->se && p->policy == SCHED_BATCH))
> +               return;

I think it is worth extending that comment to explain why we don't
also check policy for SCHED_IDLE here (since the cse_is_idle checks
above boil down to checking for task idle policy in this special case
where pse is a task entity). The comment above the pse_is_idle checks
only talks about idle group preemption, when here we're relying on it
for idle task preemption.

>
>         cfs_rq = cfs_rq_of(se);
>         update_curr(cfs_rq);
> --
> 2.39.3
>

Reviewed-by: Josh Don <joshdon@xxxxxxxxxx>