2020-07-24 00:29:06

by Jiang Biao

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

From: Jiang Biao <[email protected]>

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 <[email protected]>
---
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



2020-07-24 07:26:28

by Vincent Guittot

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

On Fri, 24 Jul 2020 at 01:39, Jiang Biao <[email protected]> wrote:
>
> From: Jiang Biao <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
>

2020-07-24 08:15:46

by Jiang Biao

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

On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
<[email protected]> wrote:
>
> On Fri, 24 Jul 2020 at 01:39, Jiang Biao <[email protected]> wrote:
> >
> > From: Jiang Biao <[email protected]>
> >
> > 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

2020-07-24 10:34:40

by Vincent Guittot

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

On Fri, 24 Jul 2020 at 10:12, Jiang Biao <[email protected]> wrote:
>
> On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
> <[email protected]> wrote:
> >
> > On Fri, 24 Jul 2020 at 01:39, Jiang Biao <[email protected]> wrote:
> > >
> > > From: Jiang Biao <[email protected]>
> > >
> > > 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

2020-07-24 11:54:24

by Jiang Biao

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

On Fri, 24 Jul 2020 at 18:34, Vincent Guittot
<[email protected]> wrote:
>
> On Fri, 24 Jul 2020 at 10:12, Jiang Biao <[email protected]> wrote:
> >
> > On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
> > <[email protected]> wrote:
> > >
> > > On Fri, 24 Jul 2020 at 01:39, Jiang Biao <[email protected]> wrote:
> > > >
> > > > From: Jiang Biao <[email protected]>
> > > >
> > > > 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

2020-07-24 12:38:08

by Ingo Molnar

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


* Jiang Biao <[email protected]> wrote:

> On Fri, 24 Jul 2020 at 18:34, Vincent Guittot
> <[email protected]> wrote:
> >
> > On Fri, 24 Jul 2020 at 10:12, Jiang Biao <[email protected]> wrote:
> > >
> > > On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
> > > <[email protected]> wrote:
> > > >
> > > > On Fri, 24 Jul 2020 at 01:39, Jiang Biao <[email protected]> wrote:
> > > > >
> > > > > From: Jiang Biao <[email protected]>
> > > > >
> > > > > 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

2020-07-24 12:43:32

by Jiang Biao

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

On Fri, 24 Jul 2020 at 20:36, Ingo Molnar <[email protected]> wrote:
>
>
> * Jiang Biao <[email protected]> wrote:
>
> > On Fri, 24 Jul 2020 at 18:34, Vincent Guittot
> > <[email protected]> wrote:
> > >
> > > On Fri, 24 Jul 2020 at 10:12, Jiang Biao <[email protected]> wrote:
> > > >
> > > > On Fri, 24 Jul 2020 at 15:24, Vincent Guittot
> > > > <[email protected]> wrote:
> > > > >
> > > > > On Fri, 24 Jul 2020 at 01:39, Jiang Biao <[email protected]> wrote:
> > > > > >
> > > > > > From: Jiang Biao <[email protected]>
> > > > > >
> > > > > > 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