On Wed, Nov 25, 2020 at 11:12:53AM +0800, Li, Aubrey wrote:
> On 2020/11/24 23:42, Peter Zijlstra wrote:
> > 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.
> >
>
> Okay, I pasted the refined patch here.
> @Joel, please let me know if you want me to send it in a separated thread.
>
You still have a bunch of #ifdefs, can't we just do
#ifndef CONFIG_SCHED_CORE
static inline bool sched_core_enabled(struct rq *rq)
{
return false;
}
#endif
and frankly I think even that is not needed because there is a jump
label __sched_core_enabled that tells us if sched_core is enabled or
not.
Balbir
