Re: [PATCH 04/18 v2] sched/eevdf: Compare min slice during wake_affine

[Kayra Cizmeci <kayracizmeci@gmail.com>]

> Add a new level in wake affine where we check on which CPU the task
> would most probably run 1st between this and prev CPUs.

> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>


> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index ad72b8536d6c..eeac0aaba3cd 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8422,6 +8422,9 @@ static int wake_wide(struct task_struct *p)
>   * wake_affine_idle() - only considers 'now', it check if the waking CPU is
>   *			cache-affine and is (or	will be) idle.
>   *
> + * wake_affine_slice() - only considers 'now', it check if the waking CPU can
> + *			 be preempted becaus using longerslice.
> + *
>   * wake_affine_weight() - considers the weight to reflect the average
>   *			  scheduling latency of the CPUs. This seems to work
>   *			  for the overloaded case.
> 

Nice. A typo. 'becaus'. 'be' wanted to be part of Santa Claus instead of 'cause'.


> +static int
> +wake_affine_slice(struct task_struct *p, int this_cpu, int prev_cpu)
> +{
> +	struct sched_entity *se = &p->se;
> +
> +	if (se->slice < get_rq_min_slice(cpu_rq(prev_cpu)))
> +		return prev_cpu;
> +
> +	if (se->slice < get_rq_min_slice(cpu_rq(this_cpu)))
> +		return this_cpu;
> +
> +	return nr_cpumask_bits;
> +}
> +
>  static int
>  wake_affine_weight(struct sched_domain *sd, struct task_struct *p,
>  		   int this_cpu, int prev_cpu, int sync)
> @@ -8508,6 +8525,9 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p,
>  	if (sched_feat(WA_IDLE))
>  		target = wake_affine_idle(this_cpu, prev_cpu, sync);
>  
> +	if (sched_feat(PREEMPT_SHORT) && target == nr_cpumask_bits)
> +		target = wake_affine_slice(p, this_cpu, prev_cpu);
> +
> 	if (sched_feat(WA_WEIGHT) && target == nr_cpumask_bits)
> 		target = wake_affine_weight(sd, p, this_cpu, prev_cpu, sync);
>

Also,

Scene (Why I always start like this? Answer is... IDK neither. :>):

CPU0 has an RT task running named TA, and CPU1 has a fair task running
named TB that has 100 ms slice.

cfs_rq_min_slice() looks only to rq's cfs_rq skipping others like dl
and rt. The value that's coming from cfs_rq_min_slice() is then checked if it
equals to the currently saved value.

So if the cfs_rq is empty, cfs_rq_min_slice() just returns the starting value of min,
that is ~0ULL. And we write this.

In the case of CPU0 and CPU1 the behavior will change whenether or not which one
of these is prev or this CPU. If CPU0 is prev it will be chosen, if not CPU1 will.

IDK if this is tolerated tho. But shouldn't this be changed?

Or Am I getting something wrong?

(To y'all that are currently attending to LPC, enjoy! I'm sadly only enjoying my room.)

Thanks,
Kayra :>