[PATCH] kernel/sched/topology: Clarify root domain(s) debug string

[PATCH] kernel/sched/topology: Clarify root domain(s) debug string

[Juri Lelli]

When scheduler debug is enabled, building scheduling domains outputs
information about how the domains are laid out and to which root domain
each CPU (or sets of CPUs) belongs, e.g.:

 CPU0 attaching sched-domain(s):
  domain-0: span=0-5 level=MC
   groups: 0:{ span=0 }, 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }
 CPU1 attaching sched-domain(s):
  domain-0: span=0-5 level=MC
   groups: 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }, 0:{ span=0 }

 [...]

 span: 0-5 (max cpu_capacity = 1024)

The fact that latest line refers to CPUs 0-5 root domain doesn't however look
immediately obvious to me: one might wonder why span 0-5 is reported "again".

Make it more clear by adding "root domain" to it, as to end with the
following.

 CPU0 attaching sched-domain(s):
  domain-0: span=0-5 level=MC
   groups: 0:{ span=0 }, 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }
 CPU1 attaching sched-domain(s):
  domain-0: span=0-5 level=MC
   groups: 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }, 0:{ span=0 }

 [...]

 root domain span: 0-5 (max cpu_capacity = 1024)

Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/sched/topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 64cc564f5255..61a1125c1ae4 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1708,7 +1708,7 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
 	rcu_read_unlock();
 
 	if (rq && sched_debug_enabled) {
-		pr_info("span: %*pbl (max cpu_capacity = %lu)\n",
+		pr_info("root domain span: %*pbl (max cpu_capacity = %lu)\n",
 			cpumask_pr_args(cpu_map), rq->rd->max_cpu_capacity);
 	}
 
-- 
2.14.3

[tip:sched/core] sched/topology: Clarify root domain(s) debug string

[tip-bot for Juri Lelli]

Commit-ID:  bf5015a50f1fdb248b48405b67cae24dc02605d6
Gitweb:     https://git.kernel.org/tip/bf5015a50f1fdb248b48405b67cae24dc02605d6
Author:     Juri Lelli <juri.lelli@redhat.com>
AuthorDate: Thu, 24 May 2018 17:29:36 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 25 May 2018 08:03:38 +0200

sched/topology: Clarify root domain(s) debug string

When scheduler debug is enabled, building scheduling domains outputs
information about how the domains are laid out and to which root domain
each CPU (or sets of CPUs) belongs, e.g.:

 CPU0 attaching sched-domain(s):
  domain-0: span=0-5 level=MC
   groups: 0:{ span=0 }, 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }
 CPU1 attaching sched-domain(s):
  domain-0: span=0-5 level=MC
   groups: 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }, 0:{ span=0 }

 [...]

 span: 0-5 (max cpu_capacity = 1024)

The fact that latest line refers to CPUs 0-5 root domain doesn't however look
immediately obvious to me: one might wonder why span 0-5 is reported "again".

Make it more clear by adding "root domain" to it, as to end with the
following:

 CPU0 attaching sched-domain(s):
  domain-0: span=0-5 level=MC
   groups: 0:{ span=0 }, 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }
 CPU1 attaching sched-domain(s):
  domain-0: span=0-5 level=MC
   groups: 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }, 0:{ span=0 }

 [...]

 root domain span: 0-5 (max cpu_capacity = 1024)

Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20180524152936.17611-1-juri.lelli@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 64cc564f5255..61a1125c1ae4 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1708,7 +1708,7 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
 	rcu_read_unlock();
 
 	if (rq && sched_debug_enabled) {
-		pr_info("span: %*pbl (max cpu_capacity = %lu)\n",
+		pr_info("root domain span: %*pbl (max cpu_capacity = %lu)\n",
 			cpumask_pr_args(cpu_map), rq->rd->max_cpu_capacity);
 	}
 

Re: [PATCH] kernel/sched/topology: Clarify root domain(s) debug string

[Dietmar Eggemann]

On 05/24/2018 05:29 PM, Juri Lelli wrote:
> When scheduler debug is enabled, building scheduling domains outputs
> information about how the domains are laid out and to which root domain
> each CPU (or sets of CPUs) belongs, e.g.:
> 
>   CPU0 attaching sched-domain(s):
>    domain-0: span=0-5 level=MC
>     groups: 0:{ span=0 }, 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }
>   CPU1 attaching sched-domain(s):
>    domain-0: span=0-5 level=MC
>     groups: 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }, 0:{ span=0 }
> 
>   [...]
> 
>   span: 0-5 (max cpu_capacity = 1024)
> 
> The fact that latest line refers to CPUs 0-5 root domain doesn't however look
> immediately obvious to me: one might wonder why span 0-5 is reported "again".
> 
> Make it more clear by adding "root domain" to it, as to end with the
> following.
> 
>   CPU0 attaching sched-domain(s):
>    domain-0: span=0-5 level=MC
>     groups: 0:{ span=0 }, 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }
>   CPU1 attaching sched-domain(s):
>    domain-0: span=0-5 level=MC
>     groups: 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }, 0:{ span=0 }
> 
>   [...]
> 
>   root domain span: 0-5 (max cpu_capacity = 1024)
> 
> Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Patrick Bellasi <patrick.bellasi@arm.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>   kernel/sched/topology.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 64cc564f5255..61a1125c1ae4 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1708,7 +1708,7 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
>   	rcu_read_unlock();
>   
>   	if (rq && sched_debug_enabled) {
> -		pr_info("span: %*pbl (max cpu_capacity = %lu)\n",
> +		pr_info("root domain span: %*pbl (max cpu_capacity = %lu)\n",
>   			cpumask_pr_args(cpu_map), rq->rd->max_cpu_capacity);
>   	}
>   
> 

Looks good to me. Probably especially helpful when setting up exclusive 
cpusets.

Juno with big and little exclusive cpuset:

...
[  124.231333] CPU1 attaching sched-domain(s):
[  124.235482]  domain-0: span=1-2 level=MC
[  124.239382]   groups: 1:{ span=1 }, 2:{ span=2 }
[  124.243969] CPU2 attaching sched-domain(s):
[  124.248112]  domain-0: span=1-2 level=MC
[  124.251998]   groups: 2:{ span=2 }, 1:{ span=1 }
[  124.256585] root domain span: 1-2 (max cpu_capacity = 1024)
[  124.262150] CPU0 attaching sched-domain(s):
[  124.266307]  domain-0: span=0,3-5 level=MC
[  124.270366]   groups: 0:{ span=0 cap=446 }, 3:{ span=3 cap=446 }, 4:{ 
span=4 cap=446 }, 5:{ span=5 cap=446 }
[  124.280131] CPU3 attaching sched-domain(s):
[  124.284273]  domain-0: span=0,3-5 level=MC
[  124.288334]   groups: 3:{ span=3 cap=446 }, 4:{ span=4 cap=446 }, 5:{ 
span=5 cap=446 }, 0:{ span=0 cap=446 }
[  124.298096] CPU4 attaching sched-domain(s):
[  124.302239]  domain-0: span=0,3-5 level=MC
[  124.306298]   groups: 4:{ span=4 cap=446 }, 5:{ span=5 cap=446 }, 0:{ 
span=0 cap=446 }, 3:{ span=3 cap=446 }
[  124.316063] CPU5 attaching sched-domain(s):
[  124.320205]  domain-0: span=0,3-5 level=MC
[  124.324265]   groups: 5:{ span=5 cap=446 }, 0:{ span=0 cap=446 }, 3:{ 
span=3 cap=446 }, 4:{ span=4 cap=446 }
[  124.334031] root domain span: 0,3-5 (max cpu_capacity = 446)

Re: [PATCH] kernel/sched/topology: Clarify root domain(s) debug string

[Juri Lelli]

On 25/05/18 13:35, Dietmar Eggemann wrote:

[...]

> 
> Looks good to me. Probably especially helpful when setting up exclusive
> cpusets.
> 
> Juno with big and little exclusive cpuset:
> 
> ...
> [  124.231333] CPU1 attaching sched-domain(s):
> [  124.235482]  domain-0: span=1-2 level=MC
> [  124.239382]   groups: 1:{ span=1 }, 2:{ span=2 }
> [  124.243969] CPU2 attaching sched-domain(s):
> [  124.248112]  domain-0: span=1-2 level=MC
> [  124.251998]   groups: 2:{ span=2 }, 1:{ span=1 }
> [  124.256585] root domain span: 1-2 (max cpu_capacity = 1024)
> [  124.262150] CPU0 attaching sched-domain(s):
> [  124.266307]  domain-0: span=0,3-5 level=MC
> [  124.270366]   groups: 0:{ span=0 cap=446 }, 3:{ span=3 cap=446 }, 4:{
> span=4 cap=446 }, 5:{ span=5 cap=446 }
> [  124.280131] CPU3 attaching sched-domain(s):
> [  124.284273]  domain-0: span=0,3-5 level=MC
> [  124.288334]   groups: 3:{ span=3 cap=446 }, 4:{ span=4 cap=446 }, 5:{
> span=5 cap=446 }, 0:{ span=0 cap=446 }
> [  124.298096] CPU4 attaching sched-domain(s):
> [  124.302239]  domain-0: span=0,3-5 level=MC
> [  124.306298]   groups: 4:{ span=4 cap=446 }, 5:{ span=5 cap=446 }, 0:{
> span=0 cap=446 }, 3:{ span=3 cap=446 }
> [  124.316063] CPU5 attaching sched-domain(s):
> [  124.320205]  domain-0: span=0,3-5 level=MC
> [  124.324265]   groups: 5:{ span=5 cap=446 }, 0:{ span=0 cap=446 }, 3:{
> span=3 cap=446 }, 4:{ span=4 cap=446 }
> [  124.334031] root domain span: 0,3-5 (max cpu_capacity = 446)

Yep, that's similar to how I noticed.

Thanks for testing.

Best,

- Juri

Re: [PATCH] kernel/sched/topology: Clarify root domain(s) debug string

[Joel Fernandes]

On Thu, May 24, 2018 at 05:29:36PM +0200, Juri Lelli wrote:
> When scheduler debug is enabled, building scheduling domains outputs
> information about how the domains are laid out and to which root domain
> each CPU (or sets of CPUs) belongs, e.g.:
> 
>  CPU0 attaching sched-domain(s):
>   domain-0: span=0-5 level=MC
>    groups: 0:{ span=0 }, 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }
>  CPU1 attaching sched-domain(s):
>   domain-0: span=0-5 level=MC
>    groups: 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }, 0:{ span=0 }
> 
>  [...]
> 
>  span: 0-5 (max cpu_capacity = 1024)
> 
> The fact that latest line refers to CPUs 0-5 root domain doesn't however look

                last line?

> immediately obvious to me: one might wonder why span 0-5 is reported "again".
> 
> Make it more clear by adding "root domain" to it, as to end with the
> following.
> 
>  CPU0 attaching sched-domain(s):
>   domain-0: span=0-5 level=MC
>    groups: 0:{ span=0 }, 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }
>  CPU1 attaching sched-domain(s):
>   domain-0: span=0-5 level=MC
>    groups: 1:{ span=1 }, 2:{ span=2 }, 3:{ span=3 }, 4:{ span=4 }, 5:{ span=5 }, 0:{ span=0 }
> 
>  [...]
> 
>  root domain span: 0-5 (max cpu_capacity = 1024)
> 
> Signed-off-by: Juri Lelli <juri.lelli@redhat.com>

I played the sched_load_balance flag to trigger this and it makes sense to
improve the print with 'root domain'.

Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

One thing I believe is a bit weird is sched_load_balance also can affect the
wake-up path, because a NULL sd is attached to the rq if sched_load_balance
is set to 0.

This could turn off the "for_each_domain(cpu, tmp)" loop in
select_task_rq_fair and hence we would always end up in the
select_idle_sibling path for those CPUs.

It also means that "XXX always" can/should be removed because sd can very
well be NULL for other sd_flag types as well, not just sd_flag ==
SD_BALANCE_WAKE. I'll send a patch to remove that comment as I just tested
this is true.

thanks,

 - Joel