sched: Fix scheduling for multi-node processors

I've decided to add this as a subitem of MC scheduling.
I think the normal case will be that a multi-node CPU has more than 1 core on each of its nodes. Thus using MN scheduling without MC scheduling does not make much sense.

Signed-off-by Andreas Herrmann <[email protected]>

---
 arch/x86/Kconfig |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

Quoted 10 lines · Andreas Herrmann

I've decided to add this as a subitem of MC scheduling.
I think the normal case will be that a multi-node CPU has more than 1 core on each of its nodes. Thus using MN scheduling without MC scheduling does not make much sense.

Signed-off-by Andreas Herrmann <[email protected]>

---
 arch/x86/Kconfig |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

Is this patch series bisectable? I admit not having checked deeply, and at least at first glance the rest looks sane - but usually Kconfig changes are kept until the *last* patch so that there's no chance of inter-patch dependencies breaking...

Use new function sd_balance_for_mn_power() and adapt sd_balance_for_package_power() and sd_power_saving_flags() for correct setting of flags SD_POWERSAVINGS_BALANCE and SD_BALANCE_NEWIDLE in CPU and MN domains.

Furthermore add flag SD_SHARE_PKG_RESOURCES to MN domain.
Rational: a multi-node processor most likely shares package resources (on Magny-Cours the package constitues a "voltage domain").

Signed-off-by Andreas Herrmann <[email protected]>

---
 arch/x86/include/asm/topology.h |    3 ++-
 include/linux/sched.h           |   14 ++++++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

arch/x86/include/asm/topology.h +2 -1

index 6d7d133..4a520b8 100644
@@ -198,7 +198,8 @@ static inline void setup_node_to_cpumask_map(void) { }
 				| SD_BALANCE_EXEC	\
 				| SD_WAKE_AFFINE	\
 				| SD_WAKE_BALANCE	\
-				| sd_balance_for_package_power()\
+				| SD_SHARE_PKG_RESOURCES\
+				| sd_balance_for_mn_power()\
 				| sd_power_saving_flags(),\
 	.last_balance		= jiffies,		\
 	.balance_interval	= 1,			\

include/linux/sched.h +12 -2

index 5755643..c53bdd8 100644
@@ -844,9 +844,18 @@ static inline int sd_balance_for_mc_power(void)
 	return 0;
 }
 
+static inline int sd_balance_for_mn_power(void)
+{
+	if (sched_mc_power_savings || sched_smt_power_savings)
+		return SD_POWERSAVINGS_BALANCE;
+
+	return 0;
+}
+
 static inline int sd_balance_for_package_power(void)
 {
-	if (sched_mc_power_savings | sched_smt_power_savings)
+	if (sched_mn_power_savings || sched_mc_power_savings ||
+	    sched_smt_power_savings)
 		return SD_POWERSAVINGS_BALANCE;
 
 	return 0;

@@ -860,7 +869,8 @@ static inline int sd_balance_for_package_power(void)
 
 static inline int sd_power_saving_flags(void)
 {
-	if (sched_mc_power_savings | sched_smt_power_savings)
+	if (sched_mn_power_savings || sched_mc_power_savings ||
+	    sched_smt_power_savings)
 		return SD_BALANCE_NEWIDLE;
 
 	return 0;

The patch adds support for POWERSAVINGS_BALANCE_BASIC for MN domain level. Currently POWERSAVINGS_BALANCE_WAKEUP is not used for MN domain.

(I have to admit that so far I don't have the correct understanding what's the benefit of POWERSAVINGS_BALANCE_WAKEUP (when an deticated wakeup CPU is used) in contrast to POWERSAVINGS_BALANCE_BASIC. I also have not found an example that would demonstrate the difference between those two powersaving levels.)

Signed-off-by Andreas Herrmann <[email protected]>

---
 kernel/sched.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

kernel/sched.c +3 -2

index ebcda58..7a0d710 100644
@@ -4591,7 +4591,8 @@ static int find_new_ilb(int cpu)
 	 * Have idle load balancer selection from semi-idle packages only
 	 * when power-aware load balancing is enabled
 	 */
-	if (!(sched_smt_power_savings || sched_mc_power_savings))
+	if (!(sched_smt_power_savings || sched_mc_power_savings ||
+	      sched_mn_power_savings))
 		goto out_done;
 
 	/*

@@ -4681,7 +4682,7 @@ int select_nohz_load_balancer(int stop_tick)
 			int new_ilb;
 
 			if (!(sched_smt_power_savings ||
-						sched_mc_power_savings))
+			      sched_mc_power_savings || sched_mn_power_savings))
 				return 1;
 			/*
 			 * Check to see if there is a more power-efficient

For performance reasons __cpu_power in a sched_group might be limited such that the group can handle only one task. To correctly calculate the capacity in upper domain level groups the unlimited power information is required. This patch stores unlimited __cpu_power information in sched_groups.orig_power and uses this when calculating __cpu_power in upper domain level groups.

Signed-off-by Andreas Herrmann <[email protected]>

---
 include/linux/sched.h |    8 +++++++-
 kernel/sched.c        |   36 ++++++++++++++++++++++++------------
 2 files changed, 31 insertions(+), 13 deletions(-)

include/linux/sched.h +7 -1

index c53bdd8..d230717 100644
@@ -890,7 +890,13 @@ struct sched_group {
 	 * (see include/linux/reciprocal_div.h)
 	 */
 	u32 reciprocal_cpu_power;
-
+	/*
+	 * Backup of original power for this group.
+	 * It is used to pass correct power information to upper
+	 * domain level groups in case __cpu_power is limited for
+	 * performance reasons.
+	 */
+	unsigned int orig_power;
 	/*
 	 * The CPUs this group covers.
 	 *

kernel/sched.c +24 -12

index 7a0d710..464b6ba 100644
@@ -8376,6 +8376,7 @@ static void init_numa_sched_groups_power(struct sched_group *group_head)
 
 			sg_inc_cpu_power(sg, sd->groups->__cpu_power);
 		}
+		sg->orig_power = sg->__cpu_power;
 		sg = sg->next;
 	} while (sg != group_head);
 }

@@ -8514,18 +8515,9 @@ static void init_sched_groups_power(int cpu, struct sched_domain *sd)
 	child = sd->child;
 
 	sd->groups->__cpu_power = 0;
-
-	/*
-	 * For perf policy, if the groups in child domain share resources
-	 * (for example cores sharing some portions of the cache hierarchy
-	 * or SMT), then set this domain groups cpu_power such that each group
-	 * can handle only one task, when there are other idle groups in the
-	 * same sched domain.
-	 */
-	if (!child || (!(sd->flags & SD_POWERSAVINGS_BALANCE) &&
-		       (child->flags &
-			(SD_SHARE_CPUPOWER | SD_SHARE_PKG_RESOURCES)))) {
+	if (!child) {
 		sg_inc_cpu_power(sd->groups, SCHED_LOAD_SCALE);
+		sd->groups->orig_power = sd->groups->__cpu_power;
 		return;
 	}
 

@@ -8534,9 +8526,29 @@ static void init_sched_groups_power(int cpu, struct sched_domain *sd)
 	 */
 	group = child->groups;
 	do {
-		sg_inc_cpu_power(sd->groups, group->__cpu_power);
+		sg_inc_cpu_power(sd->groups, group->orig_power);
 		group = group->next;
 	} while (group != child->groups);
+	sd->groups->orig_power = sd->groups->__cpu_power;
+
+	/*
+	 * For perf policy, if the groups in child domain share resources
+	 * (for example cores sharing some portions of the cache hierarchy
+	 * or SMT), then set this domain groups cpu_power such that each group
+	 * can handle only one task, when there are other idle groups in the
+	 * same sched domain.
+	 * Note: Unmodified power information is kept in orig_power and
+	 *       can be used in higher domain levels to calculate
+	 *       and reflect the correct capacity of a sched_group.
+	 *       This is required for power_savings scheduling.
+	 */
+	if (!(sd->flags & SD_POWERSAVINGS_BALANCE) &&
+	    ((child->flags &
+	      (SD_SHARE_CPUPOWER | SD_SHARE_PKG_RESOURCES)))) {
+		sd->groups->__cpu_power = 0;
+		sg_inc_cpu_power(sd->groups, SCHED_LOAD_SCALE);
+	}
+
 }
 
 /*

kernel/sched.c int __init sched_create_sysfs_power_savings_entries(struct sysdev_class *cls) · line 9236

Quoted 8 lines · Andreas Herrmann

Signed-off-by Andreas Herrmann <[email protected]>

---

kernel/sched.c +5 -0

+#ifdef CONFIG_SCHED_MN
+	if (!err && mc_capable())
+		err = sysfs_create_file(&cls->kset.kobj,
+					&attr_sched_mn_power_savings.attr);
+#endif

*sigh* another crappy sysfs file

Guys, can't we come up with anything better than sched_*_power_saving=n?

This configuration space is _way_ too large, and now it gets even crazier.

The level of NODE domain's child domain is provided in s_data.numa_child_level. Then several adaptions are required when creating the domain hierarchy. In case NODE domain is parent of MC domain we have to:
- limit NODE domains' span in sched_domain_node_span() to not exceed corresponding topology_core_cpumask.
- fix CPU domain span to cover entire cpu_map
- fix CPU domain sched groups to cover entire physical groups instead of covering a node (a node sched_group might be a proper subset of a CPU sched_group).
- use correct child domain in init_numa_sched_groups_power() when calculating sched_group.__cpu_power in NODE domain
- calculate group_power of NODE domain after its child domain

Note: As I have no idea when the ALLNODES domain is required
I assumed that an ALLNODES domain exists only if NODE domain is parent of CPU domain.

On multi-node processors a NUMA node might not span a socket. Instead a socket might span several NUMA nodes.

This patch introduces a check whether NODE domain is parent of MC domain and sets s_data.numa_child_level accordingly. (See previous patch for further details.)

We need this in case of performance policy. All sched_groups in child's parent domain (MN in this case) should be limited such that tasks are balanced among these sched_groups.

Signed-off-by Andreas Herrmann <[email protected]>

---
 kernel/sched.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

kernel/sched.c +2 -2

index 0c950dc..ab88d88 100644
@@ -8555,11 +8555,11 @@ static void init_sched_groups_power(int cpu, struct sched_domain *sd)
 	 */
 	if (!(sd->flags & SD_POWERSAVINGS_BALANCE) &&
 	    ((child->flags &
-	      (SD_SHARE_CPUPOWER | SD_SHARE_PKG_RESOURCES)))) {
+	      (SD_SHARE_CPUPOWER | SD_SHARE_PKG_RESOURCES)) ||
+	     (child->level == SD_LV_NODE))) {
 		sd->groups->__cpu_power = 0;
 		sg_inc_cpu_power(sd->groups, SCHED_LOAD_SCALE);
 	}
-
 }
 
 /*

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Signed-off-by Andreas Herrmann <[email protected]>

---
 arch/x86/kernel/smpboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kernel/smpboot.c +1 -1

index f797214..f39bb2c 100644
@@ -446,7 +446,7 @@ const struct cpumask *cpu_coregroup_mask(int cpu)
 	 * And for power savings, we return cpu_core_map
 	 */
 	if (sched_mc_power_savings || sched_smt_power_savings)
-		return cpu_core_mask(cpu);
+		return topology_core_cpumask(cpu);
 	else
 		return c->llc_shared_map;
 }

Quoted 8 lines · Andreas Herrmann

The patch adds support for POWERSAVINGS_BALANCE_BASIC for MN domain level. Currently POWERSAVINGS_BALANCE_WAKEUP is not used for MN domain.

(I have to admit that so far I don't have the correct understanding what's the benefit of POWERSAVINGS_BALANCE_WAKEUP (when an deticated wakeup CPU is used) in contrast to POWERSAVINGS_BALANCE_BASIC. I also have not found an example that would demonstrate the difference between those two powersaving levels.)

blame svaidy for not writing enough comments ;-)

iirc it moves tasks to sched_mv_preferred_wakeup_cpu instead of waking an idle cpu, this leaves idle cpus idle longer at the cost of creating overload on other cpus.

Quoted 22 lines

Signed-off-by Andreas Herrmann <[email protected]>

---
 kernel/sched.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

kernel/sched.c +2 -1

index ebcda58..7a0d710 100644
@@ -4591,7 +4591,8 @@ static int find_new_ilb(int cpu)
 	 * Have idle load balancer selection from semi-idle packages only
 	 * when power-aware load balancing is enabled
 	 */
-	if (!(sched_smt_power_savings || sched_mc_power_savings))
+	if (!(sched_smt_power_savings || sched_mc_power_savings ||
+	      sched_mn_power_savings))
 		goto out_done;
 
 	/*

@@ -4681,7 +4682,7 @@ int select_nohz_load_balancer(int stop_tick)
 			int new_ilb;
 

kernel/sched.c int select_nohz_load_balancer(int stop_tick) · line 4688

Quoted 6 lines

kernel/sched.c +1 -1

 			if (!(sched_smt_power_savings ||
-						sched_mc_power_savings))
+			      sched_mc_power_savings || sched_mn_power_savings))
 				return 1;
 			/*
 			 * Check to see if there is a more power-efficient

Quoted 14 lines · Peter Zijlstra

kernel/sched.c int __init sched_create_sysfs_power_savings_entries(struct sysdev_class *cls) · line 9236

Quoted 8 lines · Andreas Herrmann

Signed-off-by Andreas Herrmann <[email protected]>

---

kernel/sched.c +5 -0

+#ifdef CONFIG_SCHED_MN
+	if (!err && mc_capable())
+		err = sysfs_create_file(&cls->kset.kobj,
+					&attr_sched_mn_power_savings.attr);
+#endif

*sigh* another crappy sysfs file

Guys, can't we come up with anything better than sched_*_power_saving=n?

Quoted 2 lines

This configuration space is _way_ too large, and now it gets even crazier.

Hi Peter and Andreas,

Actually we had sched_power_savings and related simplifications, but that did not really simplify the interface.

As for this mulit-node MN stuff, Gautham had posted a better solution to propagate the sched_mc flags without need for new sysfs file and related changes.

Please take a look at: http://lkml.org/lkml/2009/3/31/137 and
http://lkml.org/lkml/2009/3/31/142 which actually degenerates the domain.

However Andreas's requirement seem to indicate multiple nodes within a single socket. I did not yet completely understand that topology. Some for of smart degeneration may save an additional tunable here.

Quoted 6 lines · Andreas Herrmann

For performance reasons __cpu_power in a sched_group might be limited such that the group can handle only one task. To correctly calculate the capacity in upper domain level groups the unlimited power information is required. This patch stores unlimited __cpu_power information in sched_groups.orig_power and uses this when calculating __cpu_power in upper domain level groups.

OK, so this tries to fix the cpu_power wreckage?

ok, so let me try this with an example:

Suppose we have a dual-core with shared cache and SMT

0-3 MC
0-1 2-3 SMT

Then both levels fancy setting SHARED_RESOURCES and both levels end up normalizing the cpu_power to 1, so when we unplug cpu 2, load-balancing gets all screwy because the whole system doesn't get normalized properly.

What you propose here is every time we muck with cpu_power we keep the real stuff in orig_power and use that to compute the level above.

Except you don't use it in the load-balancer proper, so normalization is still hosed.

Its a creative solution, but I'd rather see cpu_power returned to a straight sum of actual power to normalize the inter-cpu runqueue weights and do the placement decision using something else.

Quoted 29 lines · Peter Zijlstra

Quoted 6 lines · Andreas Herrmann

For performance reasons __cpu_power in a sched_group might be limited such that the group can handle only one task. To correctly calculate the capacity in upper domain level groups the unlimited power information is required. This patch stores unlimited __cpu_power information in sched_groups.orig_power and uses this when calculating __cpu_power in upper domain level groups.

OK, so this tries to fix the cpu_power wreckage?

ok, so let me try this with an example:

Suppose we have a dual-core with shared cache and SMT

0-3 MC
0-1 2-3 SMT

Then both levels fancy setting SHARED_RESOURCES and both levels end up normalizing the cpu_power to 1, so when we unplug cpu 2, load-balancing gets all screwy because the whole system doesn't get normalized properly.

What you propose here is every time we muck with cpu_power we keep the real stuff in orig_power and use that to compute the level above.

Except you don't use it in the load-balancer proper, so normalization is still hosed.

Quoted 3 lines

Its a creative solution, but I'd rather see cpu_power returned to a straight sum of actual power to normalize the inter-cpu runqueue weights and do the placement decision using something else.

The real solution is to find a way to solve asymmetric load balancing, I suppose. The asymmetry might be due to cores being hot-plugged for example

Quoted 31 lines · Balbir Singh

Quoted 29 lines · Peter Zijlstra

Quoted 6 lines · Andreas Herrmann

For performance reasons __cpu_power in a sched_group might be limited such that the group can handle only one task. To correctly calculate the capacity in upper domain level groups the unlimited power information is required. This patch stores unlimited __cpu_power information in sched_groups.orig_power and uses this when calculating __cpu_power in upper domain level groups.

OK, so this tries to fix the cpu_power wreckage?

ok, so let me try this with an example:

Suppose we have a dual-core with shared cache and SMT

0-3 MC
0-1 2-3 SMT

Then both levels fancy setting SHARED_RESOURCES and both levels end up normalizing the cpu_power to 1, so when we unplug cpu 2, load-balancing gets all screwy because the whole system doesn't get normalized properly.

What you propose here is every time we muck with cpu_power we keep the real stuff in orig_power and use that to compute the level above.

Except you don't use it in the load-balancer proper, so normalization is still hosed.

Quoted 7 lines

Quoted 3 lines

Its a creative solution, but I'd rather see cpu_power returned to a straight sum of actual power to normalize the inter-cpu runqueue weights and do the placement decision using something else.

The real solution is to find a way to solve asymmetric load balancing, I suppose. The asymmetry might be due to cores being hot-plugged for example

No, the solution is to not use cpu_power for placement and use it for normalization of the weight only. That would make the asym work by definition.

The real fun comes when we then introduce dynamic cpu_power based on feedback from things like aperf/mperf ratios for SMT and feedback from the RT scheduler.

The trouble is that cpu_power is now abused for placement decisions too, and that needs to be taken out.

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

kernel/sched.c int __init sched_create_sysfs_power_savings_entries(struct sysdev_class *cls) · line 9236

Quoted 8 lines · Andreas Herrmann

Signed-off-by Andreas Herrmann <[email protected]>

---

kernel/sched.c +5 -0

+#ifdef CONFIG_SCHED_MN
+	if (!err && mc_capable())
+		err = sysfs_create_file(&cls->kset.kobj,
+					&attr_sched_mn_power_savings.attr);
+#endif

*sigh* another crappy sysfs file

Guys, can't we come up with anything better than sched_*_power_saving=n?

This configuration space is _way_ too large, and now it gets even crazier.

Quoted 12 lines · Peter Zijlstra

kernel/sched.c int __init sched_create_sysfs_power_savings_entries(struct sysdev_class *cls) · line 9236

Quoted 8 lines · Andreas Herrmann

Signed-off-by Andreas Herrmann <[email protected]>

---

kernel/sched.c +5 -0

+#ifdef CONFIG_SCHED_MN
+	if (!err && mc_capable())
+		err = sysfs_create_file(&cls->kset.kobj,
+					&attr_sched_mn_power_savings.attr);
+#endif

*sigh* another crappy sysfs file

Quoted 1 line

Guys, can't we come up with anything better than sched_*_power_saving=n?

Thought this is a settled thing. At least there are already two such parameters. So using the existing convention is an obvious thing, no?

Quoted 2 lines

This configuration space is _way_ too large, and now it gets even crazier.

I don't fully agree.

Having one control interface for each domain level is just one approach. It gives the user full control of scheduling policies. It just might have to be properly documented.

In another mail Vaidy mentioned that

"at some point we wanted to change the interface to
sched_power_savings=N and and set the flags according to system topology".

But how you'll decide at which domain level you have to do power savings scheduling?

Using sched_mn_power_savings=1 is quite different from sched_smt_power_savings=1. Probably, the most power you save if you switch on power saving scheduling on each domain level. I.e. first filling threads of one core, then filling all cores on one internal node, then filling all internal nodes of one socket.

But for performance reasons a user might just want to use power savings in the MN domain. How you'd allow the user to configure that with just one interface? Passing the domain level to sched_power_savings, e.g. sched_power_savings=MC instead of the power saving level?

Besides that, don't we have to keep the user-interface stable, i.e. stick to sched_smt_power_savings and sched_mc_power_savings?

That reminds me, accounting is currently broken and should be based on APER/MPERF (Power gets it right - based on SPURR).

Quoted 2 lines

The trouble is that cpu_power is now abused for placement decisions too, and that needs to be taken out.

OK.. so you propose extending the static cpu_power to dynamic cpu_power but based on current topology?

Quoted 12 lines · Peter Zijlstra

kernel/sched.c int __init sched_create_sysfs_power_savings_entries(struct sysdev_class *cls) · line 9236

Quoted 8 lines · Andreas Herrmann

Signed-off-by Andreas Herrmann <[email protected]>

---

kernel/sched.c +5 -0

+#ifdef CONFIG_SCHED_MN
+	if (!err && mc_capable())
+		err = sysfs_create_file(&cls->kset.kobj,
+					&attr_sched_mn_power_savings.attr);
+#endif

*sigh* another crappy sysfs file

Quoted 4 lines

Guys, can't we come up with anything better than sched_*_power_saving=n?

This configuration space is _way_ too large, and now it gets even crazier.

Hi Peter and Andreas,

Actually we had sched_power_savings and related simplifications, but that did not really simplify the interface.

As for this mulit-node MN stuff, Gautham had posted a better solution to propagate the sched_mc flags without need for new sysfs file and related changes.

Please take a look at: http://lkml.org/lkml/2009/3/31/137 and
http://lkml.org/lkml/2009/3/31/142 which actually degenerates the domain.

However Andreas's requirement seem to indicate multiple nodes within a single socket. I did not yet completely understand that topology. Some for of smart degeneration may save an additional tunable here.

Thanks for pointing me to this patch.

Quoted 19 lines · Vaidyanathan Srinivasan

Quoted 16 lines · Peter Zijlstra

kernel/sched.c int __init sched_create_sysfs_power_savings_entries(struct sysdev_class *cls) · line 9236

Quoted 8 lines · Andreas Herrmann

Signed-off-by Andreas Herrmann <[email protected]>

---

kernel/sched.c +5 -0

+#ifdef CONFIG_SCHED_MN
+	if (!err && mc_capable())
+		err = sysfs_create_file(&cls->kset.kobj,
+					&attr_sched_mn_power_savings.attr);
+#endif

*sigh* another crappy sysfs file

Guys, can't we come up with anything better than sched_*_power_saving=n?

This configuration space is _way_ too large, and now it gets even crazier.

Quoted 4 lines

Hi Peter and Andreas,

Actually we had sched_power_savings and related simplifications, but that did not really simplify the interface.

Well, I prefer a single sched_power knob that either goes on or off.

A user really isn't interested in exploring a 3^3 configuration space {PERF, POWER, POWER-WAKE-BALANCE} x {SMT, MC, MN} in order to find what works best.

Quoted 7 lines

As for this mulit-node MN stuff, Gautham had posted a better solution to propagate the sched_mc flags without need for new sysfs file and related changes.

Please take a look at: http://lkml.org/lkml/2009/3/31/137 and
http://lkml.org/lkml/2009/3/31/142 which actually degenerates the domain.

Ah, right, that got lost in my inbox :/ Let me go read those too.

Quoted 3 lines

However Andreas's requirement seem to indicate multiple nodes within a single socket. I did not yet completely understand that topology. Some for of smart degeneration may save an additional tunable here.

Yes, apparently AMD is going to put multiple nodes in a single socket, not sure how they do that, Andreas do these chips have multiple memory busses?

I was thinking chips were pin constrained and wouldn't add a whole second memory interface to the package, but what do I know...

Quoted 11 lines · Peter Zijlstra

Quoted 8 lines · Andreas Herrmann

The patch adds support for POWERSAVINGS_BALANCE_BASIC for MN domain level. Currently POWERSAVINGS_BALANCE_WAKEUP is not used for MN domain.

(I have to admit that so far I don't have the correct understanding what's the benefit of POWERSAVINGS_BALANCE_WAKEUP (when an deticated wakeup CPU is used) in contrast to POWERSAVINGS_BALANCE_BASIC. I also have not found an example that would demonstrate the difference between those two powersaving levels.)

blame svaidy for not writing enough comments ;-)

I am here to explain ;)

Quoted 3 lines

iirc it moves tasks to sched_mv_preferred_wakeup_cpu instead of waking an idle cpu, this leaves idle cpus idle longer at the cost of creating overload on other cpus.

Yes, as Peter said, the POWERSAVINGS_BALANCE_WAKEUP biases task wakeups to sched_mc_preferred_wakeup_cpu which has been nominated from previous load balance loops.

Task wakeup biasing of sched_mc=2 works for most workloads like kernbench and other sleeping tasks that come in and out of runqueue. The default sched_mc=1 will work only for jobs running much longer than the loadbalance interval or almost 100% CPU intensive job where

the load balancer can take time to identify the load pattern and initiate a task migrate.

The wakeup biasing (sched_mc=2) will help move bursty jobs faster and statistically pack them in single package and save power.

Quoted 2 lines · Balbir Singh

That reminds me, accounting is currently broken and should be based on APER/MPERF (Power gets it right - based on SPURR).

What accounting?

Quoted 5 lines

Quoted 2 lines

The trouble is that cpu_power is now abused for placement decisions too, and that needs to be taken out.

OK.. so you propose extending the static cpu_power to dynamic cpu_power but based on current topology?

Right, so cpu_power is primarily used to normalize domain weight in the load-balancer.

Suppose a 4 core machine with 1 unplugged core:

0,1,3

0,1 3

The sd-0,1 will have cpu_power 2048, while the sd-3 will have 1024, this allowed find_busiest_group() for sd-0,1,3 to pick the one which is relatively most overloaded.

Supposing 3, 2, 2 (nice0) tasks on these cores, the domain weight of sd-0,1 is 5*1024 and sd-3 is 2*1024, normalized that becomes 5/2 and 2 resp. which clearly shows sd-0,1 to be the busiest of the pair.

Now back in the days Nick wrote all this, he did the cpu_power hack for SMT which sets the combined cpu_power of 2 threads (that's all we had back then) to 1024, because two threads share 1 core, and are roughly as fast.

He then also used this to influence task placement, preferring to move tasks to another sibling domain before getting the second thread active, this worked.

Then multi-core with shared caches came along and people did the same trick for mc power save in order to get that placement stuff, but that horribly broke the load-balancer normalization.

Now comes multi-node, and people asking for more elaborate placement strategies and all this starts creaking like a ghost house about to collapse.

Therefore I want cpu_power back to load normalization only, and do the placement stuff with something else.

Once cpu_power is pure again, we can start making it dynamic, for SMT we can utilize APERF/MPERF to guesstimate the actual work capacity of threads, and scaling cpu_power back based on RT time used on the cpu.

Then when we walk the domain tree for load-balancing we re-do the cpu_power sum, etc..

Quoted 5 lines · Peter Zijlstra

Quoted 2 lines · Balbir Singh

That reminds me, accounting is currently broken and should be based on APER/MPERF (Power gets it right - based on SPURR).

Quoted 2 lines

What accounting?

We need scaled time accounting for x86 (see *timescaled). By scaled accounting I mean ratio of APERF/MPERF

Quoted 40 lines

Quoted 5 lines

Quoted 2 lines

The trouble is that cpu_power is now abused for placement decisions too, and that needs to be taken out.

OK.. so you propose extending the static cpu_power to dynamic cpu_power but based on current topology?

Right, so cpu_power is primarily used to normalize domain weight in the load-balancer.

Suppose a 4 core machine with 1 unplugged core:

0,1,3

0,1 3

The sd-0,1 will have cpu_power 2048, while the sd-3 will have 1024, this allowed find_busiest_group() for sd-0,1,3 to pick the one which is relatively most overloaded.

Supposing 3, 2, 2 (nice0) tasks on these cores, the domain weight of sd-0,1 is 5*1024 and sd-3 is 2*1024, normalized that becomes 5/2 and 2 resp. which clearly shows sd-0,1 to be the busiest of the pair.

Now back in the days Nick wrote all this, he did the cpu_power hack for SMT which sets the combined cpu_power of 2 threads (that's all we had back then) to 1024, because two threads share 1 core, and are roughly as fast.

He then also used this to influence task placement, preferring to move tasks to another sibling domain before getting the second thread active, this worked.

Then multi-core with shared caches came along and people did the same trick for mc power save in order to get that placement stuff, but that horribly broke the load-balancer normalization.

Now comes multi-node, and people asking for more elaborate placement strategies and all this starts creaking like a ghost house about to collapse.

Quoted 3 lines

Therefore I want cpu_power back to load normalization only, and do the placement stuff with something else.

What do you have in mind for the something else? Aren't normalization and placement two sides of the same coin? My concern is that load normalization might give different recommendations from the placement stuff, then what do we do?

Quoted 13 lines · Andreas Herrmann

Quoted 12 lines · Peter Zijlstra

kernel/sched.c int __init sched_create_sysfs_power_savings_entries(struct sysdev_class *cls) · line 9236

Quoted 8 lines · Andreas Herrmann

Signed-off-by Andreas Herrmann <[email protected]>

---

kernel/sched.c +5 -0

+#ifdef CONFIG_SCHED_MN
+	if (!err && mc_capable())
+		err = sysfs_create_file(&cls->kset.kobj,
+					&attr_sched_mn_power_savings.attr);
+#endif

*sigh* another crappy sysfs file

Quoted 5 lines

Quoted 1 line

Guys, can't we come up with anything better than sched_*_power_saving=n?

Thought this is a settled thing. At least there are already two such parameters. So using the existing convention is an obvious thing, no?

Well, yes its the obvious thing, but I'm questioning whether its the best thing ;-)

Quoted 9 lines

Quoted 2 lines

This configuration space is _way_ too large, and now it gets even crazier.

I don't fully agree.

Having one control interface for each domain level is just one approach. It gives the user full control of scheduling policies. It just might have to be properly documented.

Quoted 8 lines

In another mail Vaidy mentioned that

"at some point we wanted to change the interface to
sched_power_savings=N and and set the flags according to system topology".

But how you'll decide at which domain level you have to do power savings scheduling?

The user isn't interested in knowing about domains and cpu topology in 99% of the cases, all they want is the machine not burning power like there's no tomorrow.

Users (me including) have no interest exploring a 27-state power configuration space in order to find out what works best for them, I'd throw up my hands and not bother, really.

Quoted 11 lines

Using sched_mn_power_savings=1 is quite different from sched_smt_power_savings=1. Probably, the most power you save if you switch on power saving scheduling on each domain level. I.e. first filling threads of one core, then filling all cores on one internal node, then filling all internal nodes of one socket.

But for performance reasons a user might just want to use power savings in the MN domain. How you'd allow the user to configure that with just one interface? Passing the domain level to sched_power_savings, e.g. sched_power_savings=MC instead of the power saving level?

Sure its different, it reduces the configuration space, that gives less choice, but does make it accessible.

Ask joe-admin what he prefers.

If you're really really worried people might miss the joy of fine tuning their power scheduling, then we can provide a dual interface, one for dumb people like me, and one for crazy people like you ;-)

Quoted 2 lines

Besides that, don't we have to keep the user-interface stable, i.e. stick to sched_smt_power_savings and sched_mc_power_savings?

Don't ever defend crappy stuff with interface stability, that's just lame ;-)

Quoted 6 lines · Andreas Herrmann

For performance reasons __cpu_power in a sched_group might be limited such that the group can handle only one task. To correctly calculate the capacity in upper domain level groups the unlimited power information is required. This patch stores unlimited __cpu_power information in sched_groups.orig_power and uses this when calculating __cpu_power in upper domain level groups.

OK, so this tries to fix the cpu_power wreckage?

ok, so let me try this with an example:

Suppose we have a dual-core with shared cache and SMT

0-3 MC
0-1 2-3 SMT

Then both levels fancy setting SHARED_RESOURCES and both levels end up normalizing the cpu_power to 1, so when we unplug cpu 2, load-balancing gets all screwy because the whole system doesn't get normalized properly.

What you propose here is every time we muck with cpu_power we keep the real stuff in orig_power and use that to compute the level above.

Except you don't use it in the load-balancer proper, so normalization is still hosed.

Its a creative solution, but I'd rather see cpu_power returned to a straight sum of actual power to normalize the inter-cpu runqueue weights and do the placement decision using something else.

Quoted 13 lines · Andreas Herrmann

The level of NODE domain's child domain is provided in s_data.numa_child_level. Then several adaptions are required when creating the domain hierarchy. In case NODE domain is parent of MC domain we have to:
- limit NODE domains' span in sched_domain_node_span() to not exceed corresponding topology_core_cpumask.
- fix CPU domain span to cover entire cpu_map
- fix CPU domain sched groups to cover entire physical groups instead of covering a node (a node sched_group might be a proper subset of a CPU sched_group).
- use correct child domain in init_numa_sched_groups_power() when calculating sched_group.__cpu_power in NODE domain
- calculate group_power of NODE domain after its child domain

Quoted 3 lines

Note: As I have no idea when the ALLNODES domain is required
I assumed that an ALLNODES domain exists only if NODE domain is parent of CPU domain.

I think its only used when the regular node level is too large, then we split it into smaller bits. SGI folks who run crazy large machines use this.

/me mumbels about renaming the domain level, CPU is the physical socket level, right? stupid names.

Patch sounds funky though, numa_child_level should be effident from the tree build.

Quoted 6 lines · Andreas Herrmann

On multi-node processors a NUMA node might not span a socket. Instead a socket might span several NUMA nodes.

This patch introduces a check whether NODE domain is parent of MC domain and sets s_data.numa_child_level accordingly. (See previous patch for further details.)

right, except that the previous patch
was rather cryptic :/

So you're proposing to have the NODE level depend on multi-node and then flip NODE and CPU around?

Quoted 3 lines · Andreas Herrmann

We need this in case of performance policy. All sched_groups in child's parent domain (MN in this case) should be limited such that tasks are balanced among these sched_groups.

/me fails at correlating the above changelog and the below patch.

So here we go mess up cpu_power again in order to invluence the placement policy?

Quoted 22 lines

Signed-off-by Andreas Herrmann <[email protected]>

---
 kernel/sched.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

kernel/sched.c +2 -2

index 0c950dc..ab88d88 100644
@@ -8555,11 +8555,11 @@ static void init_sched_groups_power(int cpu, struct sched_domain *sd)
 	 */
 	if (!(sd->flags & SD_POWERSAVINGS_BALANCE) &&
 	    ((child->flags &
-	      (SD_SHARE_CPUPOWER | SD_SHARE_PKG_RESOURCES)))) {
+	      (SD_SHARE_CPUPOWER | SD_SHARE_PKG_RESOURCES)) ||
+	     (child->level == SD_LV_NODE))) {
 		sd->groups->__cpu_power = 0;
 		sg_inc_cpu_power(sd->groups, SCHED_LOAD_SCALE);
 	}
-
 }
 

kernel/sched.c static void init_sched_groups_power(int cpu, struct sched_domain *sd) · line 8565

Quoted 1 line

kernel/sched.c +0 -0

 /*

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

Quoted 5 lines

Signed-off-by Andreas Herrmann <[email protected]>

---
 arch/x86/kernel/smpboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kernel/smpboot.c const struct cpumask *cpu_coregroup_mask(int cpu) · line 452

Quoted 13 lines

arch/x86/kernel/smpboot.c +1 -1

index f797214..f39bb2c 100644
@@ -446,7 +446,7 @@ const struct cpumask *cpu_coregroup_mask(int cpu)
 	 * And for power savings, we return cpu_core_map
 	 */
 	if (sched_mc_power_savings || sched_smt_power_savings)
-		return cpu_core_mask(cpu);
+		return topology_core_cpumask(cpu);
 	else
 		return c->llc_shared_map;
 }

Quoted 8 lines · Andreas Herrmann

Use new function sd_balance_for_mn_power() and adapt sd_balance_for_package_power() and sd_power_saving_flags() for correct setting of flags SD_POWERSAVINGS_BALANCE and SD_BALANCE_NEWIDLE in CPU and MN domains.

Furthermore add flag SD_SHARE_PKG_RESOURCES to MN domain.
Rational: a multi-node processor most likely shares package resources (on Magny-Cours the package constitues a "voltage domain").

IIRC SD_SHARE_PKG_RESOURCES plays games with the cpu_pwer of a sched_domain, which breaks in all kinds of curious ways, this adds more breakage afaict.

ego?

Quoted 55 lines

Signed-off-by Andreas Herrmann <[email protected]>

---
 arch/x86/include/asm/topology.h |    3 ++-
 include/linux/sched.h           |   14 ++++++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

arch/x86/include/asm/topology.h +2 -1

index 6d7d133..4a520b8 100644
@@ -198,7 +198,8 @@ static inline void setup_node_to_cpumask_map(void) { }
 				| SD_BALANCE_EXEC	\
 				| SD_WAKE_AFFINE	\
 				| SD_WAKE_BALANCE	\
-				| sd_balance_for_package_power()\
+				| SD_SHARE_PKG_RESOURCES\
+				| sd_balance_for_mn_power()\
 				| sd_power_saving_flags(),\
 	.last_balance		= jiffies,		\
 	.balance_interval	= 1,			\

include/linux/sched.h +12 -2

index 5755643..c53bdd8 100644
@@ -844,9 +844,18 @@ static inline int sd_balance_for_mc_power(void)
 	return 0;
 }
 
+static inline int sd_balance_for_mn_power(void)
+{
+	if (sched_mc_power_savings || sched_smt_power_savings)
+		return SD_POWERSAVINGS_BALANCE;
+
+	return 0;
+}
+
 static inline int sd_balance_for_package_power(void)
 {
-	if (sched_mc_power_savings | sched_smt_power_savings)
+	if (sched_mn_power_savings || sched_mc_power_savings ||
+	    sched_smt_power_savings)
 		return SD_POWERSAVINGS_BALANCE;
 
 	return 0;

@@ -860,7 +869,8 @@ static inline int sd_balance_for_package_power(void)
 
 static inline int sd_power_saving_flags(void)
 {
-	if (sched_mc_power_savings | sched_smt_power_savings)
+	if (sched_mn_power_savings || sched_mc_power_savings ||
+	    sched_smt_power_savings)
 		return SD_BALANCE_NEWIDLE;
 
 	return 0;

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Quoted 2 lines

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

So this is the third code area
(besides sched_*_power_savings sysfs interface, and the __cpu_power fiddling) that is crap, mess, a hack.

Didn't know that I'd enter such a minefield when touching this code. ;-(

What would be your perferred solution for the core_cpumask/llc_shared_map stuff? Another domain level to get rid of this function?

Quoted 7 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

I think most of the trouble here comes from having inconsistent names, a rather static structure for sched-domains setup and then we are confusing things back and forth.

Right now we have thread/sibling, core, CPU/socket and node, with many data structures around these hardcoded. Certain scheduler features only operate on the hardcoded fields.

Now Magny-Cours adds a socket internal node construct to the whole thing, names it randomly and basically breaks the semi-static representation.

We cannot just flip around our static names and hope it goes well and everything just drops into place. Everything just falls apart really instead.

Instead we should have an arch-defined tree and a CPU architecture dependent ASCII name associated with each level - but not hardcoded into the scheduler.

Plus we should have independent scheduler domains feature flags that can be turned on/off in various levels of that tree, depending on the cache and interconnect properties of the hardware - without having to worry about what the ASCII name says. Those features should be capable to work not just on the lowest level of the tree, but on higher levels too, regardless whether that level is called a 'core', a 'socket' or an 'internal node' on the ASCII level really.

This is why i insisted on handling the Magny-Cours topology discovery and enumeration patches together with the scheduler patches. It can easily become a mess if extended.

Ingo

Quoted 7 lines · Andreas Herrmann

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Quoted 8 lines

Quoted 2 lines

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

So this is the third code area
(besides sched_*_power_savings sysfs interface, and the __cpu_power fiddling) that is crap, mess, a hack.

Didn't know that I'd enter such a minefield when touching this code. ;-(

Yeah, you're lucky that way ;-) Its been creaking for a while, and I've been making noises to the IBM people (who so far have been the main source of power saving patches) to clean this up, but now you trod onto all of it at once..

Quoted 3 lines

What would be your perferred solution for the core_cpumask/llc_shared_map stuff? Another domain level to get rid of this function?

Right, I'd like to see everything exposed as domain levels.

numa-cluster
numa
socket
in-socket-numa
multi-core
shared-cache
core
threads

We currently have a fixed order of these things, but I think we should simply provide helpers for building the sd tree and let the arch code do that instead of exporting all these masks in a fixed order.

Once we get the arch domain tree, we do degenerate stuff to cull all the trivial domains and fold SD flags.

Quoted 7 lines · Andreas Herrmann

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Quoted 8 lines

Quoted 2 lines

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

So this is the third code area
(besides sched_*_power_savings sysfs interface, and the __cpu_power fiddling) that is crap, mess, a hack.

Didn't know that I'd enter such a minefield when touching this code. ;-(

Yeah, you're lucky that way ;-) Its been creaking for a while, and I've been making noises to the IBM people (who so far have been the main source of power saving patches) to clean this up, but now you trod onto all of it at once..

Quoted 3 lines

What would be your perferred solution for the core_cpumask/llc_shared_map stuff? Another domain level to get rid of this function?

Right, I'd like to see everything exposed as domain levels.

numa-cluster
numa
socket
in-socket-numa
multi-core
shared-cache
core
threads

We currently have a fixed order of these things, but I think we should simply provide helpers for building the sd tree and let the arch code do that instead of exporting all these masks in a fixed order.

Once we get the arch domain tree, we do degenerate stuff to cull all the trivial domains and fold SD flags.

Quoted 38 lines · Peter Zijlstra

Quoted 7 lines · Andreas Herrmann

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Quoted 8 lines

Quoted 2 lines

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

So this is the third code area
(besides sched_*_power_savings sysfs interface, and the __cpu_power fiddling) that is crap, mess, a hack.

Didn't know that I'd enter such a minefield when touching this code. ;-(

Yeah, you're lucky that way ;-) Its been creaking for a while, and I've been making noises to the IBM people (who so far have been the main source of power saving patches) to clean this up, but now you trod onto all of it at once..

Quoted 3 lines

What would be your perferred solution for the core_cpumask/llc_shared_map stuff? Another domain level to get rid of this function?

Right, I'd like to see everything exposed as domain levels.

numa-cluster
numa
socket
in-socket-numa
multi-core
shared-cache
core
threads

Quoted 6 lines

We currently have a fixed order of these things, but I think we should simply provide helpers for building the sd tree and let the arch code do that instead of exporting all these masks in a fixed order.

Once we get the arch domain tree, we do degenerate stuff to cull all the trivial domains and fold SD flags.

So any in-socket-numa is only going to haeppen with the arch-defined domain tree.

Now that this is settled you should throw away the __build_sched_domains cleanup patches that are in tip. They won't be of use when domain creation code is basically changed.

Quoted 42 lines · Peter Zijlstra

Quoted 7 lines · Andreas Herrmann

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Quoted 8 lines

Quoted 2 lines

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

So this is the third code area
(besides sched_*_power_savings sysfs interface, and the __cpu_power fiddling) that is crap, mess, a hack.

Didn't know that I'd enter such a minefield when touching this code. ;-(

Yeah, you're lucky that way ;-) Its been creaking for a while, and I've been making noises to the IBM people (who so far have been the main source of power saving patches) to clean this up, but now you trod onto all of it at once..

Quoted 3 lines

What would be your perferred solution for the core_cpumask/llc_shared_map stuff? Another domain level to get rid of this function?

Right, I'd like to see everything exposed as domain levels.

numa-cluster
numa
socket
in-socket-numa
multi-core
shared-cache
core
threads

We currently have a fixed order of these things, but I think we should simply provide helpers for building the sd tree and let the arch code do that instead of exporting all these masks in a fixed order.

Quoted 2 lines

Once we get the arch domain tree, we do degenerate stuff to cull all the trivial domains and fold SD flags.

So any in-socket-numa is only going to haeppen with the arch-defined domain tree.

Now that this is settled you should throw away the __build_sched_domains cleanup patches that are in tip. They won't be of use when domain creation code is basically changed.

Quoted 23 lines · Ingo Molnar

Quoted 7 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

I think most of the trouble here comes from having inconsistent names, a rather static structure for sched-domains setup and then we are confusing things back and forth.

Right now we have thread/sibling, core, CPU/socket and node, with many data structures around these hardcoded. Certain scheduler features only operate on the hardcoded fields.

Now Magny-Cours adds a socket internal node construct to the whole thing, names it randomly and basically breaks the semi-static representation.

Quoted 7 lines

We cannot just flip around our static names and hope it goes well and everything just drops into place. Everything just falls apart really instead.

Instead we should have an arch-defined tree and a CPU architecture dependent ASCII name associated with each level - but not hardcoded into the scheduler.

I admit that it's strange to have the x86 specific SCHED_SMT/MC snippets in common code.

And the NUMA/SD_NODE stuff is not used by all architectures either.

Having an arch-defined tree seems the right thing to do.

Quoted 9 lines

Plus we should have independent scheduler domains feature flags that can be turned on/off in various levels of that tree, depending on the cache and interconnect properties of the hardware - without having to worry about what the ASCII name says. Those features should be capable to work not just on the lowest level of the tree, but on higher levels too, regardless whether that level is called a 'core', a 'socket' or an 'internal node' on the ASCII level really.

Quoted 3 lines

This is why i insisted on handling the Magny-Cours topology discovery and enumeration patches together with the scheduler patches. It can easily become a mess if extended.

I don't buy this argument.

The main source of information when building sched-domains will be the CPU topology. That must be provided somehow independent of how scheduling domains are created. When the domains are built you just need to know which cpumask to use when the sched_groups and domain's span are determined.

Thus I think the topology detection is rather self-contained and can/should be provided independent of how the scheduler side is going to be implemented.

kernel/sched.c +0 -0

 	return err;
 }
 #endif /* CONFIG_SCHED_MC || CONFIG_SCHED_SMT */

Signature · 1 line

1.6.0.4

Use new function sd_balance_for_mn_power() and adapt sd_balance_for_package_power() and sd_power_saving_flags() for correct setting of flags SD_POWERSAVINGS_BALANCE and SD_BALANCE_NEWIDLE in CPU and MN domains.

Furthermore add flag SD_SHARE_PKG_RESOURCES to MN domain.
Rational: a multi-node processor most likely shares package resources (on Magny-Cours the package constitues a "voltage domain").

Signed-off-by Andreas Herrmann <[email protected]>

---
 arch/x86/include/asm/topology.h |    3 ++-
 include/linux/sched.h           |   14 ++++++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

arch/x86/include/asm/topology.h +2 -1

index 6d7d133..4a520b8 100644
@@ -198,7 +198,8 @@ static inline void setup_node_to_cpumask_map(void) { }
 				| SD_BALANCE_EXEC	\
 				| SD_WAKE_AFFINE	\
 				| SD_WAKE_BALANCE	\
-				| sd_balance_for_package_power()\
+				| SD_SHARE_PKG_RESOURCES\
+				| sd_balance_for_mn_power()\
 				| sd_power_saving_flags(),\
 	.last_balance		= jiffies,		\
 	.balance_interval	= 1,			\

include/linux/sched.h +0 -0

index 5755643..c53bdd8 100644

include/linux/sched.h +5 -2

+
 static inline int sd_balance_for_package_power(void)
 {
-	if (sched_mc_power_savings | sched_smt_power_savings)
+	if (sched_mn_power_savings || sched_mc_power_savings ||
+	    sched_smt_power_savings)
 		return SD_POWERSAVINGS_BALANCE;
 
 	return 0;

@@ -860,7 +869,8 @@ static inline int sd_balance_for_package_power(void)
 
 static inline int sd_power_saving_flags(void)
 {
-	if (sched_mc_power_savings | sched_smt_power_savings)
+	if (sched_mn_power_savings || sched_mc_power_savings ||
+	    sched_smt_power_savings)
 		return SD_BALANCE_NEWIDLE;
 
 	return 0;

Signature · 9 lines

1.6.0.4



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Hi Andreas,

kernel/sched.c int __init sched_create_sysfs_power_savings_entries(struct sysdev_class *cls) · line 9236

Quoted 9 lines · Andreas Herrmann

kernel/sched.c +5 -0

@@ -9208,6 +9229,11 @@ int __init sched_create_sysfs_power_savings_entries(struct sysdev_class *cls)
 		err = sysfs_create_file(&cls->kset.kobj,
 					&attr_sched_mc_power_savings.attr);
 #endif
+#ifdef CONFIG_SCHED_MN
+	if (!err && mc_capable())
+		err = sysfs_create_file(&cls->kset.kobj,
+					&attr_sched_mn_power_savings.attr);
+#endif

This would create the sysfs tunable even on systems which are mc_capable() but don't have multi-nodes on a package, no ?

Quoted 32 lines · Andreas Herrmann

Quoted 23 lines · Ingo Molnar

Quoted 7 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

I think most of the trouble here comes from having inconsistent names, a rather static structure for sched-domains setup and then we are confusing things back and forth.

Right now we have thread/sibling, core, CPU/socket and node, with many data structures around these hardcoded. Certain scheduler features only operate on the hardcoded fields.

Now Magny-Cours adds a socket internal node construct to the whole thing, names it randomly and basically breaks the semi-static representation.

Quoted 7 lines

We cannot just flip around our static names and hope it goes well and everything just drops into place. Everything just falls apart really instead.

Instead we should have an arch-defined tree and a CPU architecture dependent ASCII name associated with each level - but not hardcoded into the scheduler.

Quoted 7 lines

I admit that it's strange to have the x86 specific SCHED_SMT/MC snippets in common code.

And the NUMA/SD_NODE stuff is not used by all architectures either.

Having an arch-defined tree seems the right thing to do.

yep, with generic helpers to reduce per arch bloat. (named/structured in a neutral way)

Quoted 21 lines

Quoted 9 lines

Plus we should have independent scheduler domains feature flags that can be turned on/off in various levels of that tree, depending on the cache and interconnect properties of the hardware - without having to worry about what the ASCII name says. Those features should be capable to work not just on the lowest level of the tree, but on higher levels too, regardless whether that level is called a 'core', a 'socket' or an 'internal node' on the ASCII level really.

Quoted 3 lines

This is why i insisted on handling the Magny-Cours topology discovery and enumeration patches together with the scheduler patches. It can easily become a mess if extended.

I don't buy this argument.

The main source of information when building sched-domains will be the CPU topology. That must be provided somehow independent of how scheduling domains are created. When the domains are built you just need to know which cpumask to use when the sched_groups and domain's span are determined.

Quoted 3 lines

Thus I think the topology detection is rather self-contained and can/should be provided independent of how the scheduler side is going to be implemented.

This is the sysfs bits?

What is this needed for exactly?

The scheduler is pretty much the most important thing to tune in a topology aware manner, besides memory allocations.

Quoted 7 lines · Andreas Herrmann

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Quoted 8 lines

Quoted 2 lines

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

So this is the third code area
(besides sched_*_power_savings sysfs interface, and the __cpu_power fiddling) that is crap, mess, a hack.

Didn't know that I'd enter such a minefield when touching this code. ;-(

Yeah, you're lucky that way ;-) Its been creaking for a while, and I've been making noises to the IBM people (who so far have been the main source of power saving patches) to clean this up, but now you trod onto all of it at once..

Quoted 3 lines

What would be your perferred solution for the core_cpumask/llc_shared_map stuff? Another domain level to get rid of this function?

Right, I'd like to see everything exposed as domain levels.

numa-cluster
numa
socket
in-socket-numa
multi-core
shared-cache
core
threads

Quoted 46 lines · Andreas Herrmann

Quoted 44 lines · Peter Zijlstra

Quoted 7 lines · Andreas Herrmann

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Quoted 8 lines

Quoted 2 lines

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

So this is the third code area
(besides sched_*_power_savings sysfs interface, and the __cpu_power fiddling) that is crap, mess, a hack.

Didn't know that I'd enter such a minefield when touching this code. ;-(

Yeah, you're lucky that way ;-) Its been creaking for a while, and I've been making noises to the IBM people (who so far have been the main source of power saving patches) to clean this up, but now you trod onto all of it at once..

Quoted 3 lines

What would be your perferred solution for the core_cpumask/llc_shared_map stuff? Another domain level to get rid of this function?

Right, I'd like to see everything exposed as domain levels.

numa-cluster
numa
socket
in-socket-numa
multi-core
shared-cache
core
threads

We currently have a fixed order of these things, but I think we should simply provide helpers for building the sd tree and let the arch code do that instead of exporting all these masks in a fixed order.

Once we get the arch domain tree, we do degenerate stuff to cull all the trivial domains and fold SD flags.

Quoted 2 lines

So any in-socket-numa is only going to haeppen with the arch-defined domain tree.

Well, we could see what it takes to make this work without that. I mean, this is just how I'd like to see it end up, doesn't mean we cannot work on it from multiple angles at the same time.

Quoted 3 lines

Now that this is settled you should throw away the __build_sched_domains cleanup patches that are in tip. They won't be of use when domain creation code is basically changed.

I'm not sure that's needed, we can continue work on refactoring that. Small steps towards something better seems a better plan than a single large step.

Quoted 29 lines · Peter Zijlstra

Quoted 7 lines · Andreas Herrmann

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Quoted 8 lines

Quoted 2 lines

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

So this is the third code area
(besides sched_*_power_savings sysfs interface, and the __cpu_power fiddling) that is crap, mess, a hack.

Didn't know that I'd enter such a minefield when touching this code. ;-(

Yeah, you're lucky that way ;-) Its been creaking for a while, and I've been making noises to the IBM people (who so far have been the main source of power saving patches) to clean this up, but now you trod onto all of it at once..

Quoted 3 lines

What would be your perferred solution for the core_cpumask/llc_shared_map stuff? Another domain level to get rid of this function?

Right, I'd like to see everything exposed as domain levels.

Quoted 8 lines

numa-cluster
numa
socket
in-socket-numa
multi-core
shared-cache
core
threads

Out of curiosity, when does cpu_core_mask differ from llc_shared_map on Intel? Only in case of MCM (e.g. Core2 Quad)?

If yes, the hackery of cpu_coregroup_mask() could be replace by the domain that I'd like to introduce for Magny-Cours:

MC domain span would represent one die.
The new domain would span all dies in an MCM.

Bad idea?

Quoted 39 lines · Andreas Herrmann

Quoted 29 lines · Peter Zijlstra

Quoted 7 lines · Andreas Herrmann

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Andreas Herrmann

The correct mask that describes core-siblings of an processor is topology_core_cpumask. See topology adapation patches, especially
http://marc.info/?l=linux-kernel&m=124964999608179

Quoted 8 lines

Quoted 2 lines

argh, violence, murder kill.. this is the worst possible hack and you're extending it :/

So this is the third code area
(besides sched_*_power_savings sysfs interface, and the __cpu_power fiddling) that is crap, mess, a hack.

Didn't know that I'd enter such a minefield when touching this code. ;-(

Yeah, you're lucky that way ;-) Its been creaking for a while, and I've been making noises to the IBM people (who so far have been the main source of power saving patches) to clean this up, but now you trod onto all of it at once..

Quoted 3 lines

What would be your perferred solution for the core_cpumask/llc_shared_map stuff? Another domain level to get rid of this function?

Right, I'd like to see everything exposed as domain levels.

Quoted 8 lines

numa-cluster
numa
socket
in-socket-numa
multi-core
shared-cache
core
threads

Quoted 2 lines

Out of curiosity, when does cpu_core_mask differ from llc_shared_map on Intel? Only in case of MCM (e.g. Core2 Quad)?

Yes, I think both c2q and some dual-core opteron have multiple cache domains per socket.

Quoted 6 lines

If yes, the hackery of cpu_coregroup_mask() could be replace by the domain that I'd like to introduce for Magny-Cours:

MC domain span would represent one die.
The new domain would span all dies in an MCM.

Quoted 1 line

Bad idea?

No, I think all the mentioned chips have the multi-die thing in common, the intel c2q has 2 dual-core dies,

the opteron I have seems to be two
single cores

and this magny thing has 2 many cores -- teh pun, sides aching :-)

So the generalization to dies per socket seems sensible.

Quoted 3 lines

the opteron I have seems to be two
single cores

Really? I am not aware of such a thing.

Can you check how many sets of northbridge functions do you have? If you would have two dies in one package then you should see one set of PCI functions at bus 0 device 24 and a second set of PCI functions at bus 0 device 25, e.g.

  # lspci -d 1022:
  ...
  00:18.0 Host bridge [0600]: Advanced Micro Devices [AMD] Family 10h
          [Opteron, Athlon64, Sempron] HyperTransport Configuration [1022:1200]
  ...
  00:19.0 Host bridge [0600]: Advanced Micro Devices [AMD] Family 10h
          [Opteron, Athlon64, Sempron] HyperTransport Configuration [1022:1200]
  ...