[PATCH] sched/fair: consider sched-idle CPU when selecting idle core

[PATCH] sched/fair: consider sched-idle CPU when selecting idle core

[Jiang Biao]

From: Jiang Biao <benbjiang@tencent.com>

Sched-idle CPU has been considered in select_idle_cpu and
select_idle_smt, it also needs to be considered in select_idle_core to
be consistent and keep the same *idle* policy.

Signed-off-by: Jiang Biao <benbjiang@tencent.com>
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 04fa8dbcfa4d..f430a9820d08 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6014,7 +6014,7 @@ void __update_idle_core(struct rq *rq)
 		if (cpu == core)
 			continue;
 
-		if (!available_idle_cpu(cpu))
+		if (!available_idle_cpu(cpu) && !sched_idle_cpu(cpu))
 			goto unlock;
 	}
 
@@ -6045,7 +6045,7 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int
 		bool idle = true;
 
 		for_each_cpu(cpu, cpu_smt_mask(core)) {
-			if (!available_idle_cpu(cpu)) {
+			if (!available_idle_cpu(cpu) && !sched_idle_cpu(cpu)) {
 				idle = false;
 				break;
 			}
-- 
2.21.0

Re: [PATCH] sched/fair: consider sched-idle CPU when selecting idle core

[Vincent Guittot]

On Fri, 24 Jul 2020 at 01:39, Jiang Biao <humjb_1983@163.com> wrote:
>
> From: Jiang Biao <benbjiang@tencent.com>
>
> Sched-idle CPU has been considered in select_idle_cpu and
> select_idle_smt, it also needs to be considered in select_idle_core to
> be consistent and keep the same *idle* policy.

In the case of select_idle_core, we are looking for a core that is
fully idle but if one CPU of the core is running a sched_idle task,
the core will not be idle and we might end up having  the wakeup task
on a CPU and a sched_idle task on another CPU of the core which is not
what we want

>
> Signed-off-by: Jiang Biao <benbjiang@tencent.com>
> ---
>  kernel/sched/fair.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 04fa8dbcfa4d..f430a9820d08 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6014,7 +6014,7 @@ void __update_idle_core(struct rq *rq)
>                 if (cpu == core)
>                         continue;
>
> -               if (!available_idle_cpu(cpu))
> +               if (!available_idle_cpu(cpu) && !sched_idle_cpu(cpu))
>                         goto unlock;
>         }
>
> @@ -6045,7 +6045,7 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int
>                 bool idle = true;
>
>                 for_each_cpu(cpu, cpu_smt_mask(core)) {
> -                       if (!available_idle_cpu(cpu)) {
> +                       if (!available_idle_cpu(cpu) && !sched_idle_cpu(cpu)) {
>                                 idle = false;
>                                 break;
>                         }
> --
> 2.21.0
>
>

Re: [PATCH] sched/fair: consider sched-idle CPU when selecting idle core

[Jiang Biao]

On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> On Fri, 24 Jul 2020 at 01:39, Jiang Biao <humjb_1983@163.com> wrote:
> >
> > From: Jiang Biao <benbjiang@tencent.com>
> >
> > Sched-idle CPU has been considered in select_idle_cpu and
> > select_idle_smt, it also needs to be considered in select_idle_core to
> > be consistent and keep the same *idle* policy.
>
> In the case of select_idle_core, we are looking for a core that is
> fully idle but if one CPU of the core is running a sched_idle task,
> the core will not be idle and we might end up having  the wakeup task
> on a CPU and a sched_idle task on another CPU of the core which is not
> what we want
Got it. sched_idle task may interfere its sibling, which brings me
another question,
If there's a core with smt1 running sched_idle task and smt2 idle,
selecting smt1
rather than smt2 should be more helpful for wakee task, because wakee task
could suppress the sched_idle task without neighbour interfering.
And there seems to be no consideration about that currently.
Is it worth improving that?

Thanks a lot.

Regards,
Jiang

Re: [PATCH] sched/fair: consider sched-idle CPU when selecting idle core

[Vincent Guittot]

On Fri, 24 Jul 2020 at 10:12, Jiang Biao <benbjiang@gmail.com> wrote:
>
> On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
> >
> > On Fri, 24 Jul 2020 at 01:39, Jiang Biao <humjb_1983@163.com> wrote:
> > >
> > > From: Jiang Biao <benbjiang@tencent.com>
> > >
> > > Sched-idle CPU has been considered in select_idle_cpu and
> > > select_idle_smt, it also needs to be considered in select_idle_core to
> > > be consistent and keep the same *idle* policy.
> >
> > In the case of select_idle_core, we are looking for a core that is
> > fully idle but if one CPU of the core is running a sched_idle task,
> > the core will not be idle and we might end up having  the wakeup task
> > on a CPU and a sched_idle task on another CPU of the core which is not
> > what we want
> Got it. sched_idle task may interfere its sibling, which brings me
> another question,
> If there's a core with smt1 running sched_idle task and smt2 idle,
> selecting smt1
> rather than smt2 should be more helpful for wakee task, because wakee task
> could suppress the sched_idle task without neighbour interfering.

But the sched_idle will then probably quickly move on the idle smt2

> And there seems to be no consideration about that currently.
> Is it worth improving that?

This will complexify and extend the duration of the search loop  and
as mentioned above, it will most probably be a nop at the end because
of sched_idle task moving on smt2

>
> Thanks a lot.
>
> Regards,
> Jiang

Re: [PATCH] sched/fair: consider sched-idle CPU when selecting idle core

[Jiang Biao]

On Fri, 24 Jul 2020 at 18:34, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> On Fri, 24 Jul 2020 at 10:12, Jiang Biao <benbjiang@gmail.com> wrote:
> >
> > On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
> > <vincent.guittot@linaro.org> wrote:
> > >
> > > On Fri, 24 Jul 2020 at 01:39, Jiang Biao <humjb_1983@163.com> wrote:
> > > >
> > > > From: Jiang Biao <benbjiang@tencent.com>
> > > >
> > > > Sched-idle CPU has been considered in select_idle_cpu and
> > > > select_idle_smt, it also needs to be considered in select_idle_core to
> > > > be consistent and keep the same *idle* policy.
> > >
> > > In the case of select_idle_core, we are looking for a core that is
> > > fully idle but if one CPU of the core is running a sched_idle task,
> > > the core will not be idle and we might end up having  the wakeup task
> > > on a CPU and a sched_idle task on another CPU of the core which is not
> > > what we want
> > Got it. sched_idle task may interfere its sibling, which brings me
> > another question,
> > If there's a core with smt1 running sched_idle task and smt2 idle,
> > selecting smt1
> > rather than smt2 should be more helpful for wakee task, because wakee task
> > could suppress the sched_idle task without neighbour interfering.
>
> But the sched_idle will then probably quickly move on the idle smt2
>
> > And there seems to be no consideration about that currently.
> > Is it worth improving that?
>
> This will complexify and extend the duration of the search loop  and
> as mentioned above, it will most probably be a nop at the end because
> of sched_idle task moving on smt2
Indeed, the complexity is not worth.
Thanks for the explanation.

Regards,
Jiang
>
> >
> > Thanks a lot.
> >
> > Regards,
> > Jiang

Re: [PATCH] sched/fair: consider sched-idle CPU when selecting idle core

[Ingo Molnar]

* Jiang Biao <benbjiang@gmail.com> wrote:

> On Fri, 24 Jul 2020 at 18:34, Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
> >
> > On Fri, 24 Jul 2020 at 10:12, Jiang Biao <benbjiang@gmail.com> wrote:
> > >
> > > On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
> > > <vincent.guittot@linaro.org> wrote:
> > > >
> > > > On Fri, 24 Jul 2020 at 01:39, Jiang Biao <humjb_1983@163.com> wrote:
> > > > >
> > > > > From: Jiang Biao <benbjiang@tencent.com>
> > > > >
> > > > > Sched-idle CPU has been considered in select_idle_cpu and
> > > > > select_idle_smt, it also needs to be considered in select_idle_core to
> > > > > be consistent and keep the same *idle* policy.
> > > >
> > > > In the case of select_idle_core, we are looking for a core that is
> > > > fully idle but if one CPU of the core is running a sched_idle task,
> > > > the core will not be idle and we might end up having  the wakeup task
> > > > on a CPU and a sched_idle task on another CPU of the core which is not
> > > > what we want
> > > Got it. sched_idle task may interfere its sibling, which brings me
> > > another question,
> > > If there's a core with smt1 running sched_idle task and smt2 idle,
> > > selecting smt1
> > > rather than smt2 should be more helpful for wakee task, because wakee task
> > > could suppress the sched_idle task without neighbour interfering.
> >
> > But the sched_idle will then probably quickly move on the idle smt2
> >
> > > And there seems to be no consideration about that currently.
> > > Is it worth improving that?
> >
> > This will complexify and extend the duration of the search loop  and
> > as mentioned above, it will most probably be a nop at the end because
> > of sched_idle task moving on smt2
> Indeed, the complexity is not worth.
> Thanks for the explanation.

BTW., if you disagree then you could add a bit of debug 
instrumentation to measure to what extent it's a nop at the end of the 
search loop, to turn the "most probably" statement into a specific 
number.

Thanks,

	Ingo

Re: [PATCH] sched/fair: consider sched-idle CPU when selecting idle core

[Jiang Biao]

On Fri, 24 Jul 2020 at 20:36, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Jiang Biao <benbjiang@gmail.com> wrote:
>
> > On Fri, 24 Jul 2020 at 18:34, Vincent Guittot
> > <vincent.guittot@linaro.org> wrote:
> > >
> > > On Fri, 24 Jul 2020 at 10:12, Jiang Biao <benbjiang@gmail.com> wrote:
> > > >
> > > > On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
> > > > <vincent.guittot@linaro.org> wrote:
> > > > >
> > > > > On Fri, 24 Jul 2020 at 01:39, Jiang Biao <humjb_1983@163.com> wrote:
> > > > > >
> > > > > > From: Jiang Biao <benbjiang@tencent.com>
> > > > > >
> > > > > > Sched-idle CPU has been considered in select_idle_cpu and
> > > > > > select_idle_smt, it also needs to be considered in select_idle_core to
> > > > > > be consistent and keep the same *idle* policy.
> > > > >
> > > > > In the case of select_idle_core, we are looking for a core that is
> > > > > fully idle but if one CPU of the core is running a sched_idle task,
> > > > > the core will not be idle and we might end up having  the wakeup task
> > > > > on a CPU and a sched_idle task on another CPU of the core which is not
> > > > > what we want
> > > > Got it. sched_idle task may interfere its sibling, which brings me
> > > > another question,
> > > > If there's a core with smt1 running sched_idle task and smt2 idle,
> > > > selecting smt1
> > > > rather than smt2 should be more helpful for wakee task, because wakee task
> > > > could suppress the sched_idle task without neighbour interfering.
> > >
> > > But the sched_idle will then probably quickly move on the idle smt2
> > >
> > > > And there seems to be no consideration about that currently.
> > > > Is it worth improving that?
> > >
> > > This will complexify and extend the duration of the search loop  and
> > > as mentioned above, it will most probably be a nop at the end because
> > > of sched_idle task moving on smt2
> > Indeed, the complexity is not worth.
> > Thanks for the explanation.
>
> BTW., if you disagree then you could add a bit of debug
> instrumentation to measure to what extent it's a nop at the end of the
> search loop, to turn the "most probably" statement into a specific
> number.
>
> Thanks,
>
>         Ingo
Ok, I'll try.
Thanks for your reply.

Regards,
Jiang