All of lore.kernel.org
 help / color / mirror / Atom feed
From: dietmar.eggemann@arm.com (Dietmar Eggemann)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 6/6] sched: ARM: create a dedicated scheduler topology table
Date: Wed, 05 Mar 2014 22:38:52 +0000	[thread overview]
Message-ID: <5317A77C.8020501@arm.com> (raw)
In-Reply-To: <1394003906-11630-7-git-send-email-vincent.guittot@linaro.org>

On 05/03/14 07:18, Vincent Guittot wrote:
> Create a dedicated topology table for ARM which will create new level to
> differentiate CPUs that can or not powergate independantly from others.
> 
> The patch gives an example of how to add domain that will take advantage of
> SD_SHARE_POWERDOMAIN.
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
>   arch/arm/kernel/topology.c |   26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index 0bc94b1..ae8ffbc 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -185,6 +185,15 @@ const struct cpumask *cpu_coregroup_mask(int cpu)
>   	return &cpu_topology[cpu].core_sibling;
>   }
>   
> +/*
> + * The current assumption is that we can power gate each core independently.
> + * This will be superseded by DT binding once available.
> + */
> +const struct cpumask *cpu_corepower_mask(int cpu)
> +{
> +	return &cpu_topology[cpu].thread_sibling;
> +}

Although you already explained this to me in a private conversation,
it's important to notice that running this set-up on a dual cluster TC2
(2 Cortex A15 - 3 Cortex A7) (no independent core power gating) we
don't see the SD_SHARE_POWERDOMAIN topology flag set in the sd's on MC
level because this patch-set doesn't contain the appropriate DT
parsing. Like you said back then, the comment above mentions this.

> +
>   static void update_siblings_masks(unsigned int cpuid)
>   {
>   	struct cputopo_arm *cpu_topo, *cpuid_topo = &cpu_topology[cpuid];
> @@ -266,6 +275,20 @@ void store_cpu_topology(unsigned int cpuid)
>   		cpu_topology[cpuid].socket_id, mpidr);
>   }
>   
> +static struct sched_domain_topology_level arm_topology[] = {
> +#ifdef CONFIG_SCHED_MC
> +	{ cpu_corepower_mask, SD_SHARE_PKG_RESOURCES | SD_SHARE_POWERDOMAIN, SD_INIT_NAME(GMC) },
> +	{ cpu_coregroup_mask, SD_SHARE_PKG_RESOURCES, SD_INIT_NAME(MC) },
> +#endif
> +	{ cpu_cpu_mask, SD_INIT_NAME(DIE) },
> +	{ NULL, },
> +};
> +
> +static void __init set_sched_topology(void)
> +{
> +	sched_domain_topology = arm_topology;
> +}
> +
>   /*
>    * init_cpu_topology is called at boot when only one cpu is running
>    * which prevent simultaneous write access to cpu_topology array
> @@ -289,4 +312,7 @@ void __init init_cpu_topology(void)
>   	smp_wmb();
>   
>   	parse_dt_topology();
> +
> +	/* Set scheduler topology descriptor */
> +	set_sched_topology();
>   }
> 

How about the core scheduler provides an interface set_sched_topology()
instead of each arch has its own __init function? Sketched out below
for ARM.

-- >8 --
Subject: [PATCH] sched: set_sched_topology() as an interface of core scheduler

---
 arch/arm/kernel/topology.c | 7 +------
 include/linux/sched.h      | 2 +-
 kernel/sched/core.c        | 5 +++++
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index ae8ffbc..89d5592 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -284,11 +284,6 @@ static struct sched_domain_topology_level arm_topology[] = {
 	{ NULL, },
 };
 
-static void __init set_sched_topology(void)
-{
-	sched_domain_topology = arm_topology;
-}
-
 /*
  * init_cpu_topology is called at boot when only one cpu is running
  * which prevent simultaneous write access to cpu_topology array
@@ -314,5 +309,5 @@ void __init init_cpu_topology(void)
 	parse_dt_topology();
 
 	/* Set scheduler topology descriptor */
-	set_sched_topology();
+	set_sched_topology(arm_topology);
 }
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 8831413..fefd4e7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -998,7 +998,7 @@ struct sched_domain_topology_level {
 #endif
 };
 
-extern struct sched_domain_topology_level *sched_domain_topology;
+extern void set_sched_topology(struct sched_domain_topology_level *tl);
 
 #ifdef CONFIG_SCHED_DEBUG
 # define SD_INIT_NAME(type)            .name = #type
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b4fb0df..a748c92 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6105,6 +6105,11 @@ static struct sched_domain_topology_level default_topology[] = {
 
 struct sched_domain_topology_level *sched_domain_topology = default_topology;
 
+void set_sched_topology(struct sched_domain_topology_level *tl)
+{
+	sched_domain_topology = tl;
+}
+
 #define for_each_sd_topology(tl)                       \
        for (tl = sched_domain_topology; tl->mask; tl++)
 
-- 
1.8.3.2

WARNING: multiple messages have this Message-ID (diff)
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Vincent Guittot <vincent.guittot@linaro.org>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"mingo@kernel.org" <mingo@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"tony.luck@intel.com" <tony.luck@intel.com>,
	"fenghua.yu@intel.com" <fenghua.yu@intel.com>,
	"schwidefsky@de.ibm.com" <schwidefsky@de.ibm.com>,
	"james.hogan@imgtec.com" <james.hogan@imgtec.com>,
	"cmetcalf@tilera.com" <cmetcalf@tilera.com>,
	"benh@kernel.crashing.org" <benh@kernel.crashing.org>,
	"linux@arm.linux.org.uk" <linux@arm.linux.org.uk>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Cc: "preeti@linux.vnet.ibm.com" <preeti@linux.vnet.ibm.com>,
	"linaro-kernel@lists.linaro.org" <linaro-kernel@lists.linaro.org>
Subject: Re: [RFC 6/6] sched: ARM: create a dedicated scheduler topology table
Date: Wed, 05 Mar 2014 22:38:52 +0000	[thread overview]
Message-ID: <5317A77C.8020501@arm.com> (raw)
In-Reply-To: <1394003906-11630-7-git-send-email-vincent.guittot@linaro.org>

On 05/03/14 07:18, Vincent Guittot wrote:
> Create a dedicated topology table for ARM which will create new level to
> differentiate CPUs that can or not powergate independantly from others.
> 
> The patch gives an example of how to add domain that will take advantage of
> SD_SHARE_POWERDOMAIN.
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
>   arch/arm/kernel/topology.c |   26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index 0bc94b1..ae8ffbc 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -185,6 +185,15 @@ const struct cpumask *cpu_coregroup_mask(int cpu)
>   	return &cpu_topology[cpu].core_sibling;
>   }
>   
> +/*
> + * The current assumption is that we can power gate each core independently.
> + * This will be superseded by DT binding once available.
> + */
> +const struct cpumask *cpu_corepower_mask(int cpu)
> +{
> +	return &cpu_topology[cpu].thread_sibling;
> +}

Although you already explained this to me in a private conversation,
it's important to notice that running this set-up on a dual cluster TC2
(2 Cortex A15 - 3 Cortex A7) (no independent core power gating) we
don't see the SD_SHARE_POWERDOMAIN topology flag set in the sd's on MC
level because this patch-set doesn't contain the appropriate DT
parsing. Like you said back then, the comment above mentions this.

> +
>   static void update_siblings_masks(unsigned int cpuid)
>   {
>   	struct cputopo_arm *cpu_topo, *cpuid_topo = &cpu_topology[cpuid];
> @@ -266,6 +275,20 @@ void store_cpu_topology(unsigned int cpuid)
>   		cpu_topology[cpuid].socket_id, mpidr);
>   }
>   
> +static struct sched_domain_topology_level arm_topology[] = {
> +#ifdef CONFIG_SCHED_MC
> +	{ cpu_corepower_mask, SD_SHARE_PKG_RESOURCES | SD_SHARE_POWERDOMAIN, SD_INIT_NAME(GMC) },
> +	{ cpu_coregroup_mask, SD_SHARE_PKG_RESOURCES, SD_INIT_NAME(MC) },
> +#endif
> +	{ cpu_cpu_mask, SD_INIT_NAME(DIE) },
> +	{ NULL, },
> +};
> +
> +static void __init set_sched_topology(void)
> +{
> +	sched_domain_topology = arm_topology;
> +}
> +
>   /*
>    * init_cpu_topology is called at boot when only one cpu is running
>    * which prevent simultaneous write access to cpu_topology array
> @@ -289,4 +312,7 @@ void __init init_cpu_topology(void)
>   	smp_wmb();
>   
>   	parse_dt_topology();
> +
> +	/* Set scheduler topology descriptor */
> +	set_sched_topology();
>   }
> 

How about the core scheduler provides an interface set_sched_topology()
instead of each arch has its own __init function? Sketched out below
for ARM.

-- >8 --
Subject: [PATCH] sched: set_sched_topology() as an interface of core scheduler

---
 arch/arm/kernel/topology.c | 7 +------
 include/linux/sched.h      | 2 +-
 kernel/sched/core.c        | 5 +++++
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index ae8ffbc..89d5592 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -284,11 +284,6 @@ static struct sched_domain_topology_level arm_topology[] = {
 	{ NULL, },
 };
 
-static void __init set_sched_topology(void)
-{
-	sched_domain_topology = arm_topology;
-}
-
 /*
  * init_cpu_topology is called at boot when only one cpu is running
  * which prevent simultaneous write access to cpu_topology array
@@ -314,5 +309,5 @@ void __init init_cpu_topology(void)
 	parse_dt_topology();
 
 	/* Set scheduler topology descriptor */
-	set_sched_topology();
+	set_sched_topology(arm_topology);
 }
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 8831413..fefd4e7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -998,7 +998,7 @@ struct sched_domain_topology_level {
 #endif
 };
 
-extern struct sched_domain_topology_level *sched_domain_topology;
+extern void set_sched_topology(struct sched_domain_topology_level *tl);
 
 #ifdef CONFIG_SCHED_DEBUG
 # define SD_INIT_NAME(type)            .name = #type
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b4fb0df..a748c92 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6105,6 +6105,11 @@ static struct sched_domain_topology_level default_topology[] = {
 
 struct sched_domain_topology_level *sched_domain_topology = default_topology;
 
+void set_sched_topology(struct sched_domain_topology_level *tl)
+{
+	sched_domain_topology = tl;
+}
+
 #define for_each_sd_topology(tl)                       \
        for (tl = sched_domain_topology; tl->mask; tl++)
 
-- 
1.8.3.2






  reply	other threads:[~2014-03-05 22:38 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-05  7:18 [RFC 0/6] rework sched_domain topology description Vincent Guittot
2014-03-05  7:18 ` Vincent Guittot
2014-03-05  7:18 ` [RFC 1/6] sched: remove unused SCHED_INIT_NODE Vincent Guittot
2014-03-05  7:18 ` [PATCH 2/6] sched: rework of sched_domain topology definition Vincent Guittot
2014-03-05  7:18   ` Vincent Guittot
2014-03-05 17:09   ` Dietmar Eggemann
2014-03-05 17:09     ` Dietmar Eggemann
2014-03-06  8:32     ` Vincent Guittot
2014-03-06  8:32       ` Vincent Guittot
2014-03-11 10:31       ` Peter Zijlstra
2014-03-11 10:31         ` Peter Zijlstra
2014-03-11 13:27         ` Vincent Guittot
2014-03-11 13:27           ` Vincent Guittot
2014-03-11 13:48           ` Dietmar Eggemann
2014-03-11 13:48             ` Dietmar Eggemann
2014-03-05  7:18 ` [RFC 3/6] sched: s390: create a dedicated topology table Vincent Guittot
2014-03-05  7:18 ` [RFC 4/6] sched: powerpc: " Vincent Guittot
2014-03-11 10:08   ` Preeti U Murthy
2014-03-11 10:08     ` Preeti U Murthy
2014-03-11 13:18     ` Vincent Guittot
2014-03-11 13:18       ` Vincent Guittot
2014-03-12  4:42       ` Preeti U Murthy
2014-03-12  4:42         ` Preeti U Murthy
2014-03-12  7:44         ` Vincent Guittot
2014-03-12  7:44           ` Vincent Guittot
2014-03-12 11:04           ` Dietmar Eggemann
2014-03-12 11:04             ` Dietmar Eggemann
2014-03-14  2:30             ` Preeti U Murthy
2014-03-14  2:30               ` Preeti U Murthy
2014-03-14  2:14           ` Preeti U Murthy
2014-03-14  2:14             ` Preeti U Murthy
2014-03-05  7:18 ` [RFC 5/6] sched: add a new SD_SHARE_POWERDOMAIN for sched_domain Vincent Guittot
2014-03-05  7:18 ` [RFC 6/6] sched: ARM: create a dedicated scheduler topology table Vincent Guittot
2014-03-05 22:38   ` Dietmar Eggemann [this message]
2014-03-05 22:38     ` Dietmar Eggemann
2014-03-06  8:42     ` Vincent Guittot
2014-03-06  8:42       ` Vincent Guittot
2014-03-05 23:17 ` [RFC 0/6] rework sched_domain topology description Dietmar Eggemann
2014-03-05 23:17   ` Dietmar Eggemann
2014-03-06  9:04   ` Vincent Guittot
2014-03-06  9:04     ` Vincent Guittot
2014-03-06 12:31     ` Dietmar Eggemann
2014-03-06 12:31       ` Dietmar Eggemann
2014-03-07  2:47       ` Vincent Guittot
2014-03-07  2:47         ` Vincent Guittot
2014-03-08 12:40         ` Dietmar Eggemann
2014-03-08 12:40           ` Dietmar Eggemann
2014-03-10 13:21           ` Vincent Guittot
2014-03-10 13:21             ` Vincent Guittot
2014-03-11 13:17           ` Peter Zijlstra
2014-03-11 13:17             ` Peter Zijlstra
2014-03-12 13:28             ` Dietmar Eggemann
2014-03-12 13:28               ` Dietmar Eggemann
2014-03-12 13:47               ` Vincent Guittot
2014-03-12 13:47                 ` Vincent Guittot
2014-03-13 14:07                 ` Dietmar Eggemann
2014-03-13 14:07                   ` Dietmar Eggemann
2014-03-17 11:52               ` Peter Zijlstra
2014-03-17 11:52                 ` Peter Zijlstra
2014-03-19 19:15                 ` Dietmar Eggemann
2014-03-19 19:15                   ` Dietmar Eggemann
2014-03-20  8:28                   ` Vincent Guittot
2014-03-20  8:28                     ` Vincent Guittot
2014-03-11 13:08         ` Peter Zijlstra
2014-03-11 13:08           ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5317A77C.8020501@arm.com \
    --to=dietmar.eggemann@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.