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.
