Re: [RFC PATCH 11/13] sched: migration changes for core scheduling

Joel Fernandes
2020-06-12T13:21:32Z

On Wed, Mar 04, 2020 at 05:00:01PM +0000, vpillai wrote:
> From: Aubrey Li <aubrey.li@intel.com>
>
> - Don't migrate if there is a cookie mismatch
> Load balance tries to move task from busiest CPU to the
> destination CPU. When core scheduling is enabled, if the
> task's cookie does not match with the destination CPU's
> core cookie, this task will be skipped by this CPU. This
> mitigates the forced idle time on the destination CPU.
>
> - Select cookie matched idle CPU
> In the fast path of task wakeup, select the first cookie matched
> idle CPU instead of the first idle CPU.
>
> - Find cookie matched idlest CPU
> In the slow path of task wakeup, find the idlest CPU whose core
> cookie matches with task's cookie
>
> - Don't migrate task if cookie not match
> For the NUMA load balance, don't migrate task to the CPU whose
> core cookie does not match with task's cookie
>
> Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
> Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
> Signed-off-by: Vineeth Remanan Pillai <vpillai@digitalocean.com>
> ---
> kernel/sched/fair.c | 55 +++++++++++++++++++++++++++++++++++++++++---
> kernel/sched/sched.h | 29 +++++++++++++++++++++++
> 2 files changed, 81 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 1c9a80d8dbb8..f42ceecb749f 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1789,6 +1789,15 @@ static void task_numa_find_cpu(struct task_numa_env *env,
> if (!cpumask_test_cpu(cpu, env->p->cpus_ptr))
> continue;
>
> +#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
> +
> env->dst_cpu = cpu;
> task_numa_compare(env, taskimp, groupimp, maymove);
> }
> @@ -5660,8 +5669,13 @@ find_idlest_group_cpu(struct sched_group *group, struct task_struct *p, int this
>
> /* Traverse only the allowed CPUs */
> for_each_cpu_and(i, sched_group_span(group), p->cpus_ptr) {
> + struct rq *rq = cpu_rq(i);
> +
> +#ifdef CONFIG_SCHED_CORE
> + if (!sched_core_cookie_match(rq, p))
> + continue;
> +#endif
> if (available_idle_cpu(i)) {
> - struct rq *rq = cpu_rq(i);
> struct cpuidle_state *idle = idle_get_state(rq);
> if (idle && idle->exit_latency < min_exit_latency) {
> /*
> @@ -5927,8 +5941,14 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t
> return si_cpu;
> if (!cpumask_test_cpu(cpu, p->cpus_ptr))
> continue;
> +#ifdef CONFIG_SCHED_CORE
> + if (available_idle_cpu(cpu) &&
> + sched_core_cookie_match(cpu_rq(cpu), p))
> + break;
> +#else

select_idle_cpu() is called only if no idle core could be found in the LLC by
select_idle_core().

So, would it be better here to just do the cookie equality check directly
instead of calling the sched_core_cookie_match() helper? More so, because
select_idle_sibling() is a fastpath.

AFAIR, that's what v4 did:

if (available_idle_cpu(cpu))
#ifdef CONFIG_SCHED_CORE
if (sched_core_enabled(cpu_rq(cpu)) &&
(p->core_cookie == cpu_rq(cpu)->core->core_cookie))
break;
#else
break;
#endif

Thoughts? thanks,

- Joel