Re: [RFC PATCH] sched/fair: Choose the CPU where short task is running during wake up

[Gautham R. Shenoy]

Hello Prateek,

On Mon, Sep 26, 2022 at 11:20:16AM +0530, K Prateek Nayak wrote:[

[..snip..]

> > @@ -6050,7 +6063,8 @@ wake_affine_idle(int this_cpu, int prev_cpu, int sync)
> >  	if (available_idle_cpu(this_cpu) && cpus_share_cache(this_cpu, prev_cpu))
> >  		return available_idle_cpu(prev_cpu) ? prev_cpu : this_cpu;
> >  
> > -	if (sync && cpu_rq(this_cpu)->nr_running == 1)
> > +	if ((sync && cpu_rq(this_cpu)->nr_running == 1) ||
> > +	    is_short_task(cpu_curr(this_cpu)))
> 
> This change seems to optimize for affine wakeup which benefits
> tasks with producer-consumer pattern but is not ideal for Stream.
> Currently the logic ends will do an affine wakeup even if sync
> flag is not set:
> 
>           stream-4135    [029] d..2.   353.580953: sched_waking: comm=stream pid=4129 prio=120 target_cpu=082
>           stream-4135    [029] d..2.   353.580957: select_task_rq_fair: wake_affine_idle: Select this_cpu: sync(0) rq->nr_running(1) is_short_task(1)
>           stream-4135    [029] d..2.   353.580960: sched_migrate_task: comm=stream pid=4129 prio=120 orig_cpu=82 dest_cpu=30
>           <idle>-0       [030] dNh2.   353.580993: sched_wakeup: comm=stream pid=4129 prio=120 target_cpu=030
> 
> I believe a consideration should be made for the sync flag when
> going for an affine wakeup. Also the check for short running could
> be at the end after checking if prev_cpu is an available_idle_cpu.

We need to check if moving the is_short_task() to a later point after
checking the availability of the previous CPU solve the problem for
the workloads which showed regressions on AMD EPYC systems.

> --
> Thanks and Regards,
> Prateek

--
Thanks and Regards
gautham.