Messages in this thread Patch in this message | | | Date | Mon, 14 Dec 2020 10:31:22 +0100 | From | Peter Zijlstra <> | Subject | Re: [RFC PATCH v7] sched/fair: select idle cpu from idle cpumask for task wakeup |
| |
On Mon, Dec 14, 2020 at 09:11:29AM +0100, Vincent Guittot wrote: > On Fri, 11 Dec 2020 at 23:50, Mel Gorman <mgorman@techsingularity.net> wrote:
> > I originally did something like that on purpose too but Vincent called > > it out so it is worth mentioning now to avoid surprises. That said, I'm > > at the point where anything SIS-related simply needs excessive exposure > > because no matter what it does, someone is unhappy with it. > > Yeah, I don't like extending the idle core search loop for something > that is not related to the idle core search. This adds more work out > of control of the sis_prop. So I'm still against hiding some idle cpu > search in idle core search.
The idea, of course, is to do less. The current code is pretty crap in that it will do a whole bunch of things multiple times.
Also, a possible follow up, would be something like the below (and remove all the sds->has_idle_cores crud), which brings core scanning under SIS_PROP.
But it all needs lots of benchmarking :/
--- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6095,6 +6095,9 @@ static inline bool test_idle_cores(int c static inline int __select_idle_core(int core, struct cpumask *cpus, int *idle_cpu) { + if (idle_cpu && (available_idle_cpu(core) || sched_idle_cpu(cpu)) + *idle_cpu = core; + return -1; } @@ -6109,7 +6112,6 @@ static int select_idle_cpu(struct task_s { struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask); int this = smp_processor_id(); - bool smt = test_idle_cores(this, false); int i, cpu, idle_cpu = -1, nr = INT_MAX; struct sched_domain *this_sd; u64 time; @@ -6120,7 +6122,7 @@ static int select_idle_cpu(struct task_s cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr); - if (sched_feat(SIS_PROP) && !smt) { + if (sched_feat(SIS_PROP)) { u64 avg_cost, avg_idle, span_avg; /* @@ -6140,26 +6142,17 @@ static int select_idle_cpu(struct task_s } for_each_cpu_wrap(cpu, cpus, target) { - if (smt) { - i = __select_idle_core(cpu, cpus, &idle_cpu); - if ((unsigned)i < nr_cpumask_bits) - return i; - - } else { - if (!--nr) - return -1; - - if (available_idle_cpu(cpu) || sched_idle_cpu(cpu)) { - idle_cpu = cpu; - break; - } + if (!--nr) + break; + + i = __select_idle_core(cpu, cpus, &idle_cpu); + if ((unsigned)i < nr_cpumask_bits) { + idle_cpu = i; + break; } } - if (smt) - set_idle_cores(this, false); - - if (sched_feat(SIS_PROP) && !smt) { + if (sched_feat(SIS_PROP)) { time = cpu_clock(this) - time; update_avg(&this_sd->avg_scan_cost, time); }
| |