lkml.org 
[lkml]   [2014]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC 0/6] rework sched_domain topology description
On 17/03/14 11:52, Peter Zijlstra wrote:
> On Wed, Mar 12, 2014 at 01:28:07PM +0000, Dietmar Eggemann wrote:
[...]
>> By making it robust, I guess you mean that the core scheduler has to
>> check that the provided set-ups are sane, something like the following
>> code snippet in sd_init()
>>
>> if (WARN_ONCE(tl->sd_flags & ~TOPOLOGY_SD_FLAGS,
>> "wrong sd_flags in topology description\n"))
>> tl->sd_flags &= ~TOPOLOGY_SD_FLAGS;
>>
>> but for per cpu set-up's.
>
> So a domain is principally a group of CPUs with the same properties.
> However per-cpu domain attributes allows you to specify different domain
> properties within the one domain mask.
>
> That's completely broken.
>
> So the way to validate something like that would be:
>
> cpu = cpumask_first(tl->mask());
> flags = tl->flags(cpu);
>
> for (;cpu = cpumask_next(cpu, tl->mask()), cpu < nr_cpu_ids;)
> BUG_ON(tl->flags(cpu) != flags);
>
> Or something along those lines.

I tried this idea inside sd_init() on top of Vincent's V3 and it's doing
its job.

>
> But for me its far easier to think in the simple one domain one flags
> scenario. The whole degenerate folding is a very simple optimization
> simply removing redundant levels.
>

For me, the approach with the 'int cpu' parameter in the flag function is
easier to understand. One of the things I had to grasp though was the fact that
we can only specify SD_SHARE_FOO flags and not SD_NOT_SHARE_FOO per domain.

-- >8 --

Subject: [PATCH] sched: check that the sd_flags are consistent in one domain

---
arch/arm/kernel/topology.c | 13 +++++++++----
include/linux/sched.h | 6 +++---
kernel/sched/core.c | 11 +++++++++--
3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 71e1fec6d31a..425f133c690d 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -275,15 +275,20 @@ void store_cpu_topology(unsigned int cpuid)
cpu_topology[cpuid].socket_id, mpidr);
}

-static inline const int cpu_corepower_flags(void)
+//static inline const int cpu_corepower_flags(void)
+//{
+// return SD_SHARE_PKG_RESOURCES | SD_SHARE_POWERDOMAIN;
+//}
+
+static inline const int arm_cpu_core_flags(int cpu)
{
- return SD_SHARE_PKG_RESOURCES | SD_SHARE_POWERDOMAIN;
+ return (cpu < 2) ? SD_SHARE_PKG_RESOURCES : SD_SHARE_PKG_RESOURCES | SD_SHARE_POWERDOMAIN;
}

static struct sched_domain_topology_level arm_topology[] = {
#ifdef CONFIG_SCHED_MC
- { cpu_corepower_mask, cpu_corepower_flags, SD_INIT_NAME(GMC) },
- { cpu_coregroup_mask, cpu_core_flags, SD_INIT_NAME(MC) },
+// { cpu_corepower_mask, cpu_corepower_flags, SD_INIT_NAME(GMC) },
+ { cpu_coregroup_mask, arm_cpu_core_flags, SD_INIT_NAME(MC) },
#endif
{ cpu_cpu_mask, SD_INIT_NAME(DIE) },
{ NULL, },
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 05ce264e5144..45e5aa3d3e80 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -870,14 +870,14 @@ enum cpu_idle_type {
#define SD_NUMA 0x4000 /* cross-node balancing */

#ifdef CONFIG_SCHED_SMT
-static inline const int cpu_smt_flags(void)
+static inline const int cpu_smt_flags(int cpu)
{
return SD_SHARE_CPUPOWER | SD_SHARE_PKG_RESOURCES;
}
#endif

#ifdef CONFIG_SCHED_MC
-static inline const int cpu_core_flags(void)
+static inline const int cpu_core_flags(int cpu)
{
return SD_SHARE_PKG_RESOURCES;
}
@@ -990,7 +990,7 @@ void free_sched_domains(cpumask_var_t doms[], unsigned int ndoms);
bool cpus_share_cache(int this_cpu, int that_cpu);

typedef const struct cpumask *(*sched_domain_mask_f)(int cpu);
-typedef const int (*sched_domain_flags_f)(void);
+typedef const int (*sched_domain_flags_f)(int cpu);

#define SDTL_OVERLAP 0x01

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index f2ee6c72b13a..6b8ba837977c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5968,7 +5968,7 @@ sd_init(struct sched_domain_topology_level *tl, int cpu)
sd_weight = cpumask_weight(tl->mask(cpu));

if (tl->sd_flags)
- sd_flags = (*tl->sd_flags)();
+ sd_flags = (*tl->sd_flags)(cpu);
if (WARN_ONCE(sd_flags & ~TOPOLOGY_SD_FLAGS,
"wrong sd_flags in topology description\n"))
sd_flags &= ~TOPOLOGY_SD_FLAGS;
@@ -6044,9 +6044,16 @@ sd_init(struct sched_domain_topology_level *tl, int cpu)
sd->idle_idx = 1;
}

+ if (tl->sd_flags) {
+ int flags = (*tl->sd_flags)(cpumask_first(tl->mask(cpu)));
+
+ for (;cpu = cpumask_next(cpu, tl->mask(cpu)), cpu < nr_cpu_ids;)
+ BUG_ON((*tl->sd_flags)(cpu) != flags);
+ }
+
sd->private = &tl->data;

- return sd;
+ return sd;
}

/*
--
1.7.9.5






\
 
 \ /
  Last update: 2014-03-20 02:41    [W:0.079 / U:32.000 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site