Re: [PATCH -tip 14/32] sched: migration changes for core scheduling

[Li, Aubrey]

On 2020/11/26 6:57, Balbir Singh wrote:
> 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.

Hmm..., I need another wrapper for CONFIG_SCHED_CORE specific variables.
How about this one?

Thanks,
-Aubrey