Messages in this thread | | | Subject | Re: [PATCH] sched/fair: Make sched-idle cpu selection consistent throughout | From | Parth Shah <> | Date | Fri, 25 Oct 2019 17:30:34 +0530 |
| |
On 10/25/19 1:41 PM, Viresh Kumar wrote: > On 25-10-19, 12:13, Parth Shah wrote: >> Hi Viresh, >> >> On 10/24/19 12:15 PM, Viresh Kumar wrote: >>> There are instances where we keep searching for an idle CPU despite >>> having a sched-idle cpu already (in find_idlest_group_cpu(), >>> select_idle_smt() and select_idle_cpu() and then there are places where >>> we don't necessarily do that and return a sched-idle cpu as soon as we >>> find one (in select_idle_sibling()). This looks a bit inconsistent and >>> it may be worth having the same policy everywhere. >>> >>> On the other hand, choosing a sched-idle cpu over a idle one shall be >>> beneficial from performance point of view as well, as we don't need to >>> get the cpu online from a deep idle state which is quite a time >>> consuming process and delays the scheduling of the newly wakeup task. >>> >>> This patch tries to simplify code around sched-idle cpu selection and >>> make it consistent throughout. >>> >>> FWIW, tests were done with the help of rt-app (8 SCHED_OTHER and 5 >>> SCHED_IDLE tasks, not bound to any cpu) on ARM platform (octa-core), and >>> no significant difference in scheduling latency of SCHED_OTHER tasks was >>> found. >>> >>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> >>> --- >> >> [...] >> >>> @@ -5755,13 +5749,11 @@ static int select_idle_smt(struct task_struct *p, int target) >>> for_each_cpu(cpu, cpu_smt_mask(target)) { >>> if (!cpumask_test_cpu(cpu, p->cpus_ptr)) >>> continue; >>> - if (available_idle_cpu(cpu)) >>> + if (available_idle_cpu(cpu) || sched_idle_cpu(cpu)) >>> return cpu; >> >> I guess this is a correct approach, but just wondering what if we still >> keep searching for a sched_idle CPU even though we have found an >> available_idle CPU? > > I do believe selecting a sched-idle CPU should almost always be better > (performance wise), unless we have a strong argument against it. And > anyway, the load balancer will get triggered at a later point of time > and will pull away these newly wakeup tasks to idle CPUs. The > advantage we get out of it is that the tasks get serviced a bit > earlier when they first get queued. > > It is really up to the maintainers to see what kind of policy do we > want to adapt here and not a choice I can make :) >
yeah, I agree. I will favor selecting sched-idle first for smaller domains like SMT but would leave on experts. BTW, if sched-idle is given priority then maybe... > @@ -5818,13 +5810,11 @@ static int select_idle_cpu(struct task_struct *p, > struct sched_domain *sd, int t > > for_each_cpu_wrap(cpu, sched_domain_span(sd), target) { > if (!--nr) > - return si_cpu; > + return -1; > if (!cpumask_test_cpu(cpu, p->cpus_ptr)) > continue; > - if (available_idle_cpu(cpu)) > + if (available_idle_cpu(cpu) || sched_idle_cpu(cpu)) > break; ...here too can be optimized I guess.
Thanks, Parth
| |