Messages in this thread | | | Date | Tue, 8 Dec 2020 15:16:10 +0100 | From | Peter Zijlstra <> | Subject | Re: [RFC PATCH v6] sched/fair: select idle cpu from idle cpumask for task wakeup |
| |
On Tue, Dec 08, 2020 at 09:49:57AM +0800, Aubrey Li wrote: > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index c4da7e17b906..b8af602dea79 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -3999,6 +3999,7 @@ void scheduler_tick(void) > rq_lock(rq, &rf); > > update_rq_clock(rq); > + update_idle_cpumask(rq, false);
Does that really need to be done with rq->lock held?
> thermal_pressure = arch_scale_thermal_pressure(cpu_of(rq)); > update_thermal_load_avg(rq_clock_thermal(rq), rq, thermal_pressure);
> @@ -6808,6 +6813,51 @@ balance_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) > } > #endif /* CONFIG_SMP */ > > +/* > + * Update cpu idle state and record this information > + * in sd_llc_shared->idle_cpus_span. > + */ > +void update_idle_cpumask(struct rq *rq, bool set_idle) > +{ > + struct sched_domain *sd; > + int cpu = cpu_of(rq); > + int idle_state; > + > + /* > + * If called from scheduler tick, only update > + * idle cpumask if the CPU is busy, as idle > + * cpumask is also updated on idle entry. > + * > + */ > + if (!set_idle && idle_cpu(cpu)) > + return;
scheduler_tick() already calls idle_cpu() when SMP.
> + /* > + * Also set SCHED_IDLE cpu in idle cpumask to > + * allow SCHED_IDLE cpu as a wakeup target > + */ > + idle_state = set_idle || sched_idle_cpu(cpu); > + /* > + * No need to update idle cpumask if the state > + * does not change. > + */ > + if (rq->last_idle_state == idle_state) > + return; > + > + rcu_read_lock();
This is called with IRQs disabled, surely we can forgo rcu_read_lock() here.
> + sd = rcu_dereference(per_cpu(sd_llc, cpu)); > + if (!sd || !sd->shared) > + goto unlock;
I don't think !sd->shared is possible here.
> + if (idle_state) > + cpumask_set_cpu(cpu, sds_idle_cpus(sd->shared)); > + else > + cpumask_clear_cpu(cpu, sds_idle_cpus(sd->shared)); > + > + rq->last_idle_state = idle_state; > +unlock: > + rcu_read_unlock(); > +} > + > static unsigned long wakeup_gran(struct sched_entity *se) > { > unsigned long gran = sysctl_sched_wakeup_granularity; > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c > index f324dc36fc43..f995660edf2b 100644 > --- a/kernel/sched/idle.c > +++ b/kernel/sched/idle.c > @@ -156,6 +156,11 @@ static void cpuidle_idle_call(void) > return; > } > > + /* > + * The CPU is about to go idle, set it in idle cpumask > + * to be a wake up target. > + */ > + update_idle_cpumask(this_rq(), true);
This should be in do_idle(), right around arch_cpu_idle_enter().
> /* > * The RCU framework needs to be told that we are entering an idle > * section, so no more rcu read side critical sections and one more > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > index 8d1ca65db3b0..db460b20217a 100644 > --- a/kernel/sched/sched.h > +++ b/kernel/sched/sched.h > @@ -1004,6 +1004,7 @@ struct rq { > /* This is used to determine avg_idle's max value */ > u64 max_idle_balance_cost; > #endif /* CONFIG_SMP */ > + unsigned char last_idle_state;
All of that is pointless for UP. Also, is this the best location?
| |