Messages in this thread |  | | Date | Tue, 24 Nov 2020 16:42:37 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH -tip 14/32] sched: migration changes for core scheduling |
| |
On Mon, Nov 23, 2020 at 12:36:10PM +0800, Li, Aubrey wrote: > >> +#ifdef CONFIG_SCHED_CORE > >> + /* > >> + * Skip this cpu if source task's cookie does not match > >> + * with CPU's core cookie. > >> + */ > >> + if (!sched_core_cookie_match(cpu_rq(cpu), env->p)) > >> + continue; > >> +#endif > >> + > > > > Any reason this is under an #ifdef? In sched_core_cookie_match() won't > > the check for sched_core_enabled() do the right thing even when > > CONFIG_SCHED_CORE is not enabed?> > Yes, sched_core_enabled works properly when CONFIG_SCHED_CORE is not > enabled. But when CONFIG_SCHED_CORE is not enabled, it does not make > sense to leave a core scheduler specific function here even at compile > time. Also, for the cases in hot path, this saves CPU cycles to avoid > a judgment.
No, that's nonsense. If it works, remove the #ifdef. Less (#ifdef) is more.
> >> +static inline bool sched_core_cookie_match(struct rq *rq, struct task_struct *p) > >> +{ > >> + bool idle_core = true; > >> + int cpu; > >> + > >> + /* Ignore cookie match if core scheduler is not enabled on the CPU. */ > >> + if (!sched_core_enabled(rq)) > >> + return true; > >> + > >> + for_each_cpu(cpu, cpu_smt_mask(cpu_of(rq))) { > >> + if (!available_idle_cpu(cpu)) { > > > > I was looking at this snippet and comparing this to is_core_idle(), the > > major difference is the check for vcpu_is_preempted(). Do we want to > > call the core as non idle if any vcpu was preempted on this CPU? > > Yes, if there is a VCPU was preempted on this CPU, better not place task > on this core as the VCPU may be holding a spinlock and wants to be executed > again ASAP.
If you're doing core scheduling on vcpus, you deserve all the pain possible.
|  |