On Wed, Jan 27, 2021 at 11:43:22AM +0100, Vincent Guittot wrote:
> > @@ -6149,18 +6161,31 @@ static int select_idle_cpu(struct task_struct *p,
> > struct sched_domain *sd, int t
> > }
> >
> > for_each_cpu_wrap(cpu, cpus, target) {
> > - if (!--nr)
> > - return -1;
> > - if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
> > - break;
> > + if (smt) {
> > + i = select_idle_core(p, cpu, cpus, &idle_cpu);
> > + if ((unsigned int)i < nr_cpumask_bits)
> > + return i;
> > +
> > + } else {
> > + if (!--nr)
> > + return -1;
> > + i = __select_idle_cpu(cpu);
>
> you should use idle_cpu directly instead of this intermediate i variable
>
> + idle_cpu = __select_idle_cpu(cpu);
> + if ((unsigned int)idle_cpu < nr_cpumask_bits)
> + break;
>
> Apart ths small comment above, the patch looks good to me and I
> haven't any performance regression anymore
>
It's matching the code sequence in the SMT block. If we are going to make
that change, then go the full way with this?
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 52a650aa2108..01e40e36c386 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6129,7 +6129,7 @@ static inline int select_idle_core(struct task_struct *p,
int core, struct cpuma
static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int
target)
{
struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
- int i, cpu, idle_cpu = -1, nr = INT_MAX;
+ int cpu, idle_cpu = -1, nr = INT_MAX;
bool smt = test_idle_cores(target, false);
int this = smp_processor_id();
struct sched_domain *this_sd;
@@ -6162,18 +6162,16 @@ static int select_idle_cpu(struct task_struct *p,
struct sched_domain *sd, int t
for_each_cpu_wrap(cpu, cpus, target) {
if (smt) {
- i = select_idle_core(p, cpu, cpus, &idle_cpu);
- if ((unsigned int)i < nr_cpumask_bits)
- return i;
+ idle_cpu = select_idle_core(p, cpu, cpus, &idle_cpu);
+ if ((unsigned int)idle_cpu < nr_cpumask_bits)
+ return idle_cpu;
} else {
if (!--nr)
return -1;
- i = __select_idle_cpu(cpu);
- if ((unsigned int)i < nr_cpumask_bits) {
- idle_cpu = i;
+ idle_cpu = __select_idle_cpu(cpu);
+ if ((unsigned int)idle_cpu < nr_cpumask_bits)
break;
- }
}
}
--
Mel Gorman
SUSE Labs
