Messages in this thread Patch in this message | | | Date | Wed, 19 Mar 2014 19:15:01 +0000 | From | Dietmar Eggemann <> | Subject | Re: [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
| |