lkml.org 
[lkml]   [2020]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH -tip 14/32] sched: migration changes for core scheduling
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

\
 
 \ /
  Last update: 2020-11-25 23:59    [W:0.156 / U:0.280 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site