Skip to contentSkip to conversation

sched: unified sched_powersavings tunables

RFC PATCH In review Vaidyanathan Srinivasan 4 participants 23 messages Jan 16, 2012 – Jan 28, 2012

Subsystems (4)
x86 Block layer Scheduler Driver core
lore
Share & tools
lore

Vaidyanathan Srinivasan

author
#0 lore raw

cover sched: unified sched_powersavings tunables

Hi,

I have created the following RFC patch based on the recent discussions and consensus on simplifying the power aware scheduler in the kernel.

Ref: LWN: Rethinking power-aware scheduling

     http://lwn.net/Articles/474915/

The goal of the unified tunable are as follows:

* Simple and single tunable for different (all) topology
* Good default powersavings for kernel
* Potential to map this setting to other subsystems like cpufreq and cpuidle

What this patch does: (first step)

* Create a single sched_powersavings tunable in sysfs
* Enable current sched_mc and sched_smt features based on value of this single tunable

What this patch is yet to do:

* Tune the default power savings to pack packages only till a threshold of say 50%
* Add notifiers to change setting on battery/AC transitions
* Feed the thresholds from arch specific code so that different archs can do 'optimal' packing based on topology
* May be move this to /sys/device/system/powersavings and add additional platform tunables like x86_energy_perf_policy?

This RFC has only x86 changes and has been tested on dual-socket, quad-core,HT configuration.

Please let me know your comments and feedback.

Thanks,
Vaidy

---

Vaidyanathan Srinivasan (2):
sched: unified sched_powersavings sysfs tunable
sched: fix group_capacity for thread level consolidation

 arch/x86/Kconfig          |   20 ++++--------
 arch/x86/kernel/smpboot.c |    2 +
 block/blk.h               |   11 ++++---
 drivers/base/cpu.c        |    2 +
 include/linux/sched.h     |   29 +++++++++--------
 include/linux/topology.h  |    9 +----
 kernel/sched/core.c       |   75 +++++++++++----------------------------------
 kernel/sched/fair.c       |   38 ++++++++++++++++-------
 8 files changed, 77 insertions(+), 109 deletions(-)
author
#1 lore raw

RFC PATCH 1/2 sched: unified sched_powersavings sysfs tunable

Combine the sched_mc_powersavings and sched_smt_powersavings sysfs tunables into a single sysfs tunable:

/sys/devices/system/cpu/sched_powersavings={0,1,2}

0 - Power savings disabled (performance mode) 1 - Default kernel settings. Automatic powersave
vs performance tradeoff by the kernel
2 - Maximum power savings

The kernel will default to '1' which is equivalent to sched_mc_powersavings=1 or consolidate at package level.

Max power saving setting '2' would consolidate to sibling threads and also do aggressive active balancing.

Signed-off-by Vaidyanathan Srinivasan <[email protected]>
---
 arch/x86/Kconfig          |   20 ++++--------
 arch/x86/kernel/smpboot.c |    2 +
 block/blk.h               |   11 ++++---
 drivers/base/cpu.c        |    2 +
 include/linux/sched.h     |   29 +++++++++--------
 include/linux/topology.h  |    9 +----
 kernel/sched/core.c       |   75 +++++++++++----------------------------------
 kernel/sched/fair.c       |   23 +++++++-------
 8 files changed, 62 insertions(+), 109 deletions(-)
arch/x86/Kconfig +6 -14
index 6c14ecd..ee615af 100644
@@ -788,23 +788,15 @@ config NR_CPUS
 	  This is purely to save memory - each supported CPU adds
 	  approximately eight kilobytes to the kernel image.
 
-config SCHED_SMT
-	bool "SMT (Hyperthreading) scheduler support"
-	depends on X86_HT
-	---help---
-	  SMT scheduler support improves the CPU scheduler's decision making
-	  when dealing with Intel Pentium 4 chips with HyperThreading at a
-	  cost of slightly increased overhead in some places. If unsure say
-	  N here.
-
-config SCHED_MC
+config SCHED_POWERSAVE
 	def_bool y
-	prompt "Multi-core scheduler support"
+	prompt "Power save support in scheduler"
 	depends on X86_HT
 	---help---
-	  Multi-core scheduler support improves the CPU scheduler's decision
-	  making when dealing with multi-core CPU chips at a cost of slightly
-	  increased overhead in some places. If unsure say N here.
+	  Power saving feature in scheduler optimizes task placement
+	  in a multi-core or mulit-threaded system whenever possible.
+	  Default kernel settings would suit most applications, while
+	  sysfs tunables can be used to control this feature at runtime.
 
 config IRQ_TIME_ACCOUNTING
 	bool "Fine granularity task level IRQ time accounting"
arch/x86/kernel/smpboot.c +1 -1
index 66d250c..1d60cdd 100644
@@ -414,7 +414,7 @@ const struct cpumask *cpu_coregroup_mask(int cpu)
 	 * For perf, we return last level cache shared map.
 	 * And for power savings, we return cpu_core_map
 	 */
-	if ((sched_mc_power_savings || sched_smt_power_savings) &&
+	if ((sched_power_savings) &&
 	    !(cpu_has(c, X86_FEATURE_AMD_DCM)))
 		return cpu_core_mask(cpu);
 	else
block/blk.h +6 -5
index 7efd772..1457107 100644
@@ -167,14 +167,15 @@ static inline int queue_congestion_off_threshold(struct request_queue *q)
 static inline int blk_cpu_to_group(int cpu)
 {
 	int group = NR_CPUS;
-#ifdef CONFIG_SCHED_MC
-	const struct cpumask *mask = cpu_coregroup_mask(cpu);
-	group = cpumask_first(mask);
-#elif defined(CONFIG_SCHED_SMT)
-	group = cpumask_first(topology_thread_cpumask(cpu));
+#ifdef CONFIG_SCHED_POWERSAVE
+	if (smt_capable())
+		group = cpumask_first(topology_thread_cpumask(cpu));
+	else	
+		group = cpumask_first(cpu_coregroup_mask(cpu));
 #else
 	return cpu;
 #endif
+	/* Possible dead code?? */
 	if (likely(group < NR_CPUS))
 		return group;
 	return cpu;
drivers/base/cpu.c +1 -1
index db87e78..dbaa35f 100644
@@ -299,7 +299,7 @@ void __init cpu_dev_init(void)
 
 	cpu_dev_register_generic();
 
-#if defined(CONFIG_SCHED_MC) || defined(CONFIG_SCHED_SMT)
+#ifdef CONFIG_SCHED_POWERSAVE
 	sched_create_sysfs_power_savings_entries(cpu_subsys.dev_root);
 #endif
 }
include/linux/sched.h +15 -14
index 4032ec1..5c33bbc 100644
@@ -850,33 +850,34 @@ enum cpu_idle_type {
 #define SD_PREFER_SIBLING	0x1000	/* Prefer to place tasks in a sibling domain */
 #define SD_OVERLAP		0x2000	/* sched_domains of this level overlap */
 
-enum powersavings_balance_level {
-	POWERSAVINGS_BALANCE_NONE = 0,  /* No power saving load balance */
-	POWERSAVINGS_BALANCE_BASIC,	/* Fill one thread/core/package
-					 * first for long running threads
-					 */
-	POWERSAVINGS_BALANCE_WAKEUP,	/* Also bias task wakeups to semi-idle
-					 * cpu package for power savings
-					 */
-	MAX_POWERSAVINGS_BALANCE_LEVELS
+enum powersavings_level {
+	POWERSAVINGS_DISABLED = 0,	/* Max performance */
+	POWERSAVINGS_DEFAULT,		/* Kernel default policy, automatic powersave */
+					/* vs performance tradeoff */
+	POWERSAVINGS_MAX		/* Favour power savings over peformance */
 };
 
-extern int sched_mc_power_savings, sched_smt_power_savings;
+extern int sched_power_savings;
 
 static inline int sd_balance_for_mc_power(void)
 {
-	if (sched_smt_power_savings)
+	switch (sched_power_savings) {
+	case POWERSAVINGS_MAX:
 		return SD_POWERSAVINGS_BALANCE;
 
-	if (!sched_mc_power_savings)
+	case POWERSAVINGS_DISABLED:
 		return SD_PREFER_SIBLING;
 
+	default:
+		break;
+	}
+
 	return 0;
 }
 
 static inline int sd_balance_for_package_power(void)
 {
-	if (sched_mc_power_savings | sched_smt_power_savings)
+	if (sched_power_savings != POWERSAVINGS_DISABLED)
 		return SD_POWERSAVINGS_BALANCE;
 
 	return SD_PREFER_SIBLING;

@@ -892,7 +893,7 @@ extern int __weak arch_sd_sibiling_asym_packing(void);
 
 static inline int sd_power_saving_flags(void)
 {
-	if (sched_mc_power_savings | sched_smt_power_savings)
+	if (sched_power_savings != POWERSAVINGS_DISABLED)
 		return SD_BALANCE_NEWIDLE;
 
 	return 0;
include/linux/topology.h +2 -7
index e26db03..61f3659 100644
@@ -79,10 +79,7 @@ int arch_update_cpu_topology(void);
  * (Only non-zero and non-null fields need be specified.)
  */
 
-#ifdef CONFIG_SCHED_SMT
-/* MCD - Do we really need this?  It is always on if CONFIG_SCHED_SMT is,
- * so can't we drop this in favor of CONFIG_SCHED_SMT?
- */
+#ifdef CONFIG_SCHED_POWERSAVE
 #define ARCH_HAS_SCHED_WAKE_IDLE
 /* Common values for SMT siblings */
 #ifndef SD_SIBLING_INIT

@@ -110,9 +107,7 @@ int arch_update_cpu_topology(void);
 	.smt_gain		= 1178,	/* 15% */			\
 }
 #endif
-#endif /* CONFIG_SCHED_SMT */
 
-#ifdef CONFIG_SCHED_MC
 /* Common values for MC siblings. for now mostly derived from SD_CPU_INIT */
 #ifndef SD_MC_INIT
 #define SD_MC_INIT (struct sched_domain) {				\

@@ -142,7 +137,7 @@ int arch_update_cpu_topology(void);
 	.balance_interval	= 1,					\
 }
 #endif
-#endif /* CONFIG_SCHED_MC */
+#endif /* CONFIG_SCHED_POWERSAVE */
 
 /* Common values for CPUs */
 #ifndef SD_CPU_INIT
kernel/sched/core.c +19 -56

142 lines not on this page · raw message

kernel/sched/fair.c +12 -11
index 84adb2d..bae6ec8 100644
@@ -3497,7 +3497,7 @@ struct sd_lb_stats {
 	unsigned int  busiest_group_weight;
 
 	int group_imb; /* Is there imbalance in this sd */
-#if defined(CONFIG_SCHED_MC) || defined(CONFIG_SCHED_SMT)
+#ifdef CONFIG_SCHED_POWERSAVE
 	int power_savings_balance; /* Is powersave balance needed for this sd */
 	struct sched_group *group_min; /* Least loaded group in sd */
 	struct sched_group *group_leader; /* Group which relieves group_min */

@@ -3549,7 +3549,7 @@ static inline int get_sd_load_idx(struct sched_domain *sd,
 }
 
 
-#if defined(CONFIG_SCHED_MC) || defined(CONFIG_SCHED_SMT)
+#ifdef CONFIG_SCHED_POWERSAVE
 /**
  * init_sd_power_savings_stats - Initialize power savings statistics for
  * the given sched_domain, during load balancing.

@@ -3669,7 +3669,7 @@ static inline int check_power_save_busiest_group(struct sd_lb_stats *sds,
 	return 1;
 
 }
-#else /* CONFIG_SCHED_MC || CONFIG_SCHED_SMT */
+#else /* CONFIG_SCHED_POWERSAVE */
 static inline void init_sd_power_savings_stats(struct sched_domain *sd,
 	struct sd_lb_stats *sds, enum cpu_idle_type idle)
 {

@@ -3687,7 +3687,7 @@ static inline int check_power_save_busiest_group(struct sd_lb_stats *sds,
 {
 	return 0;
 }
-#endif /* CONFIG_SCHED_MC || CONFIG_SCHED_SMT */
+#endif /* CONFIG_SCHED_POWERSAVE */
 
 
 unsigned long default_scale_freq_power(struct sched_domain *sd, int cpu)

@@ -4422,9 +4422,10 @@ static int need_active_balance(struct sched_domain *sd, int idle,
 		 *
 		 * The package power saving logic comes from
 		 * find_busiest_group(). If there are no imbalance, then
-		 * f_b_g() will return NULL. However when sched_mc={1,2} then
-		 * f_b_g() will select a group from which a running task may be
-		 * pulled to this cpu in order to make the other package idle.
+		 * f_b_g() will return NULL. However when
+		 * sched_powersavings={1,2} then f_b_g() will select a group
+		 * from which a running task may be pulled to this cpu
+		 * in order to make the other package idle.
 		 * If there is no opportunity to make a package idle and if
 		 * there are no imbalance, then f_b_g() will return NULL and no
 		 * action will be taken in load_balance_newidle().

@@ -4434,7 +4435,7 @@ static int need_active_balance(struct sched_domain *sd, int idle,
 		 * move_tasks() will succeed.  ld_moved will be true and this
 		 * active balance code will not be triggered.
 		 */
-		if (sched_mc_power_savings < POWERSAVINGS_BALANCE_WAKEUP)
+		if (sched_power_savings < POWERSAVINGS_MAX)
 			return 0;
 	}
 

@@ -4739,7 +4740,7 @@ static struct {
 	unsigned long next_balance;     /* in jiffy units */
 } nohz ____cacheline_aligned;
 
-#if defined(CONFIG_SCHED_MC) || defined(CONFIG_SCHED_SMT)
+#ifdef CONFIG_SCHED_POWERSAVE
 /**
  * lowest_flag_domain - Return lowest sched_domain containing flag.
  * @cpu:	The cpu whose lowest level of sched domain is to

@@ -4796,7 +4797,7 @@ 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_power_savings))
 		goto out_done;
 
 	/*

@@ -4831,7 +4832,7 @@ out_done:
 
 	return nr_cpu_ids;
 }
-#else /*  (CONFIG_SCHED_MC || CONFIG_SCHED_SMT) */
+#else /*  (CONFIG_SCHED_POWERSAVE) */
 static inline int find_new_ilb(int call_cpu)
 {
 	return nr_cpu_ids;
#2 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 12 lines · Vaidyanathan Srinivasan

+++ b/block/blk.h

block/blk.h +2 -5
@@ -167,14 +167,15 @@ static inline int queue_congestion_off_threshold(struct request_queue *q)
 static inline int blk_cpu_to_group(int cpu)
 {
        int group = NR_CPUS;
-#ifdef CONFIG_SCHED_MC
-       const struct cpumask *mask = cpu_coregroup_mask(cpu);
-       group = cpumask_first(mask);
-#elif defined(CONFIG_SCHED_SMT)
-       group = cpumask_first(topology_thread_cpumask(cpu));
+#ifdef CONFIG_SCHED_POWERSAVE
+       if (smt_capable())

block/blk.h static inline int queue_congestion_off_threshold(struct request_queue *q) · line 181

Quoted 10 lines
block/blk.h +4 -0
+               group = cpumask_first(topology_thread_cpumask(cpu));
+       else    
+               group = cpumask_first(cpu_coregroup_mask(cpu));
 #else
        return cpu;
 #endif
+       /* Possible dead code?? */
        if (likely(group < NR_CPUS))
                return group;
        return cpu;

After going, WTF is block doing! I had a closer look and this doesn't seem right at all. The old code would use coregroup_mask when SCHED_MC && SCHED_SMT, the new code does something else.

Jens, what is this thing trying to do?

maintainer
#3 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 28 lines · Peter Zijlstra
Quoted 12 lines · Vaidyanathan Srinivasan

+++ b/block/blk.h

block/blk.h +2 -5
@@ -167,14 +167,15 @@ static inline int queue_congestion_off_threshold(struct request_queue *q)
 static inline int blk_cpu_to_group(int cpu)
 {
        int group = NR_CPUS;
-#ifdef CONFIG_SCHED_MC
-       const struct cpumask *mask = cpu_coregroup_mask(cpu);
-       group = cpumask_first(mask);
-#elif defined(CONFIG_SCHED_SMT)
-       group = cpumask_first(topology_thread_cpumask(cpu));
+#ifdef CONFIG_SCHED_POWERSAVE
+       if (smt_capable())

block/blk.h static inline int queue_congestion_off_threshold(struct request_queue *q) · line 181

Quoted 10 lines
block/blk.h +4 -0
+               group = cpumask_first(topology_thread_cpumask(cpu));
+       else    
+               group = cpumask_first(cpu_coregroup_mask(cpu));
 #else
        return cpu;
 #endif
+       /* Possible dead code?? */
        if (likely(group < NR_CPUS))
                return group;
        return cpu;

After going, WTF is block doing! I had a closer look and this doesn't seem right at all. The old code would use coregroup_mask when SCHED_MC && SCHED_SMT, the new code does something else.

Quoted 1 line

Jens, what is this thing trying to do?

Not surprised that it's broken for some configs. The intent of the code is to return the first CPU in the "group" that the passed in core/thread belongs to. This is used to decide whether to perform a completion locally, or to send it off to a different "group".

Signature · 1 line
Jens Axboe
#4 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 8 lines · Jens Axboe
Quoted 1 line · Peter Zijlstra

Jens, what is this thing trying to do?

The intent of the code
is to return the first CPU in the "group" that the passed in core/thread belongs to. This is used to decide whether to perform a completion locally, or to send it off to a different "group".

Would you perhaps have meant to identify some shared cache domain?

In the scheduler core code we have (for CONFIG_SMP):

  static int ttwu_share_cache(int this_cpu, int that_cpu);

which returns true if this and that share a cache and false otherwise. Would that suffice or do you need a slightly different form? That is, we should provide you with some API and avoid you having to poke around with CONFIG_SCHED* and topology bits methinks.

maintainer
#5 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 12 lines · Peter Zijlstra
Quoted 8 lines · Jens Axboe
Quoted 1 line · Peter Zijlstra

Jens, what is this thing trying to do?

The intent of the code
is to return the first CPU in the "group" that the passed in core/thread belongs to. This is used to decide whether to perform a completion locally, or to send it off to a different "group".

Would you perhaps have meant to identify some shared cache domain?

Quoted 8 lines

In the scheduler core code we have (for CONFIG_SMP):

  static int ttwu_share_cache(int this_cpu, int that_cpu);

which returns true if this and that share a cache and false otherwise. Would that suffice or do you need a slightly different form? That is, we should provide you with some API and avoid you having to poke around with CONFIG_SCHED* and topology bits methinks.

Yeah, I think that would suit my purpose nicely, in fact. What level of cache sharing is being used here? The block code wanted a per-socket type operation, but since it's a heuristic, perhaps the above is even better (or equivelant, perhaps).

Signature · 1 line
Jens Axboe
#6 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 5 lines · Jens Axboe

O
Yeah, I think that would suit my purpose nicely, in fact. What level of cache sharing is being used here? The block code wanted a per-socket type operation, but since it's a heuristic, perhaps the above is even better (or equivelant, perhaps).

It uses the biggest shared cache exposed in the topology information the scheduler has (which is currently somewhat funny but is on the todo list for improvements).

Effectively it ends up being the socket wide LLC for modern Intel chips though.

Would something like the below work for you (compile tested only).

---
Subject: sched, block: Unify cache detection
From: Peter Zijlstra <[email protected]>
Date: Thu Jan 26 12:44:34 CET 2012

The block layer has some code trying to determine if two CPUs share a cache, the scheduler has a similar function. Expose the function used by the scheduler and make the block layer use it, thereby removing the block layers usage of CONFIG_SCHED* and topology bits.

Signed-off-by Peter Zijlstra <[email protected]>
---
 block/blk-softirq.c   |   16 ++++++++--------
 block/blk.h           |   16 ----------------
 include/linux/sched.h |    8 ++++++++
 kernel/sched/core.c   |    6 +++---
 4 files changed, 19 insertions(+), 27 deletions(-)
block/blk-softirq.c +8 -8
@@ -8,6 +8,7 @@
 #include <linux/blkdev.h>
 #include <linux/interrupt.h>
 #include <linux/cpu.h>
+#include <linux/sched.h>
 
 #include "blk.h"
 

@@ -103,9 +104,10 @@ static struct notifier_block __cpuinitda
 
 void __blk_complete_request(struct request *req)
 {
-	int ccpu, cpu, group_cpu = NR_CPUS;
+	int ccpu, cpu;
 	struct request_queue *q = req->q;
 	unsigned long flags;
+	bool shared = false;
 
 	BUG_ON(!q->softirq_done_fn);
 

@@ -117,22 +119,20 @@ void __blk_complete_request(struct reque
 	 */
 	if (req->cpu != -1) {
 		ccpu = req->cpu;
-		if (!test_bit(QUEUE_FLAG_SAME_FORCE, &q->queue_flags)) {
-			ccpu = blk_cpu_to_group(ccpu);
-			group_cpu = blk_cpu_to_group(cpu);
-		}
+		if (!test_bit(QUEUE_FLAG_SAME_FORCE, &q->queue_flags))
+			shared = cpus_share_cache(cpu, ccpu);
 	} else
 		ccpu = cpu;
 
 	/*
-	 * If current CPU and requested CPU are in the same group, running
-	 * softirq in current CPU. One might concern this is just like
+	 * If current CPU and requested CPU share a cache, run the softirq on
+	 * the current CPU. One might concern this is just like
 	 * QUEUE_FLAG_SAME_FORCE, but actually not. blk_complete_request() is
 	 * running in interrupt handler, and currently I/O controller doesn't
 	 * support multiple interrupts, so current CPU is unique actually. This
 	 * avoids IPI sending from current CPU to the first CPU of a group.
 	 */
-	if (ccpu == cpu || ccpu == group_cpu) {
+	if (ccpu == cpu || shared) {
 		struct list_head *list;
 do_local:
 		list = &__get_cpu_var(blk_cpu_done);
block/blk.h +0 -16
@@ -164,22 +164,6 @@ static inline int queue_congestion_off_t
 	return q->nr_congestion_off;
 }
 
-static inline int blk_cpu_to_group(int cpu)
-{
-	int group = NR_CPUS;
-#ifdef CONFIG_SCHED_MC
-	const struct cpumask *mask = cpu_coregroup_mask(cpu);
-	group = cpumask_first(mask);
-#elif defined(CONFIG_SCHED_SMT)
-	group = cpumask_first(topology_thread_cpumask(cpu));
-#else
-	return cpu;
-#endif
-	if (likely(group < NR_CPUS))
-		return group;
-	return cpu;
-}
-
 /*
  * Contribute to IO statistics IFF:
  *
include/linux/sched.h +8 -0
@@ -1052,6 +1052,8 @@ static inline int test_sd_parent(struct
 unsigned long default_scale_freq_power(struct sched_domain *sd, int cpu);
 unsigned long default_scale_smt_power(struct sched_domain *sd, int cpu);
 
+bool cpus_share_cache(int this_cpu, int that_cpu);
+
 #else /* CONFIG_SMP */
 
 struct sched_domain_attr;

@@ -1061,6 +1063,12 @@ partition_sched_domains(int ndoms_new, c
 			struct sched_domain_attr *dattr_new)
 {
 }
+
+static inline bool cpus_share_cache(int this_cpu, int that_cpu)
+{
+	return true;
+}
+
 #endif	/* !CONFIG_SMP */
kernel/sched/core.c +3 -3
@@ -1512,7 +1512,7 @@ static int ttwu_activate_remote(struct t
 }
 #endif /* __ARCH_WANT_INTERRUPTS_ON_CTXSW */
 
-static inline int ttwu_share_cache(int this_cpu, int that_cpu)
+bool cpus_share_cache(int this_cpu, int that_cpu)
 {
 	return per_cpu(sd_llc_id, this_cpu) == per_cpu(sd_llc_id, that_cpu);
 }

@@ -1523,7 +1523,7 @@ static void ttwu_queue(struct task_struc
 	struct rq *rq = cpu_rq(cpu);
 
 #if defined(CONFIG_SMP)
-	if (sched_feat(TTWU_QUEUE) && !ttwu_share_cache(smp_processor_id(), cpu)) {
+	if (sched_feat(TTWU_QUEUE) && !cpus_share_cache(smp_processor_id(), cpu)) {
 		sched_clock_cpu(cpu); /* sync clocks x-cpu */
 		ttwu_queue_remote(p, cpu);
 		return;

@@ -5759,7 +5759,7 @@ static void destroy_sched_domains(struct
  *
  * Also keep a unique ID per domain (we use the first cpu number in
  * the cpumask of the domain), this allows us to quickly tell if
- * two cpus are in the same cache domain, see ttwu_share_cache().
+ * two cpus are in the same cache domain, see cpus_share_cache().
  */
 DEFINE_PER_CPU(struct sched_domain *, sd_llc);
 DEFINE_PER_CPU(int, sd_llc_id);
maintainer
#7 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 7 lines · Peter Zijlstra
Quoted 5 lines · Jens Axboe

O
Yeah, I think that would suit my purpose nicely, in fact. What level of cache sharing is being used here? The block code wanted a per-socket type operation, but since it's a heuristic, perhaps the above is even better (or equivelant, perhaps).

Quoted 8 lines

It uses the biggest shared cache exposed in the topology information the scheduler has (which is currently somewhat funny but is on the todo list for improvements).

Effectively it ends up being the socket wide LLC for modern Intel chips though.

Would something like the below work for you (compile tested only).

Yep, looks good to me, and an improvement for me not to carry this code that doesn't really belong there.

Signature · 1 line
Jens Axboe
#8 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 3 lines · Jens Axboe

Yep, looks good to me, and an improvement for me not to carry this code
that doesn't really belong there.

I'll take this as an ACK from you and slam the patch in my 3.4 queue. Yell if I misconstrued your intent.

Thanks!

maintainer
#9 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 7 lines · Peter Zijlstra
Quoted 3 lines · Jens Axboe

Yep, looks good to me, and an improvement for me not to carry this code
that doesn't really belong there.

I'll take this as an ACK from you and slam the patch in my 3.4 queue. Yell if I misconstrued your intent.

Sorry, should have been clearer. Yes, you can add my acked-by. Thanks Peter!

Signature · 1 line
Jens Axboe
#10 lore raw

[tip:sched/core] sched, block: Unify cache detection

In reply to Peter Zijlstra
Commit-ID:  39be350127ec60a078edffe5b4915dafba4ba514
Gitweb:     http://git.kernel.org/tip/39be350127ec60a078edffe5b4915dafba4ba514
Author:     Peter Zijlstra <[email protected]>
AuthorDate: Thu, 26 Jan 2012 12:44:34 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 27 Jan 2012 13:28:48 +0100

sched, block: Unify cache detection

The block layer has some code trying to determine if two CPUs share a cache, the scheduler has a similar function. Expose the function used by the scheduler and make the block layer use it, thereby removing the block layers usage of CONFIG_SCHED* and topology bits.

Signed-off-by Peter Zijlstra <[email protected]>
Acked-by Jens Axboe <[email protected]>
Link http://lkml.kernel.org/r/1327579450.2446.95.camel@twins open here
---
 block/blk-softirq.c   |   16 ++++++++--------
 block/blk.h           |   16 ----------------
 include/linux/sched.h |    8 ++++++++
 kernel/sched/core.c   |    6 +++---
 4 files changed, 19 insertions(+), 27 deletions(-)
block/blk-softirq.c +8 -8
index 1366a89..467c8de 100644
@@ -8,6 +8,7 @@
 #include <linux/blkdev.h>
 #include <linux/interrupt.h>
 #include <linux/cpu.h>
+#include <linux/sched.h>
 
 #include "blk.h"
 

@@ -103,9 +104,10 @@ static struct notifier_block __cpuinitdata blk_cpu_notifier = {
 
 void __blk_complete_request(struct request *req)
 {
-	int ccpu, cpu, group_cpu = NR_CPUS;
+	int ccpu, cpu;
 	struct request_queue *q = req->q;
 	unsigned long flags;
+	bool shared = false;
 
 	BUG_ON(!q->softirq_done_fn);
 

@@ -117,22 +119,20 @@ void __blk_complete_request(struct request *req)
 	 */
 	if (req->cpu != -1) {
 		ccpu = req->cpu;
-		if (!test_bit(QUEUE_FLAG_SAME_FORCE, &q->queue_flags)) {
-			ccpu = blk_cpu_to_group(ccpu);
-			group_cpu = blk_cpu_to_group(cpu);
-		}
+		if (!test_bit(QUEUE_FLAG_SAME_FORCE, &q->queue_flags))
+			shared = cpus_share_cache(cpu, ccpu);
 	} else
 		ccpu = cpu;
 
 	/*
-	 * If current CPU and requested CPU are in the same group, running
-	 * softirq in current CPU. One might concern this is just like
+	 * If current CPU and requested CPU share a cache, run the softirq on
+	 * the current CPU. One might concern this is just like
 	 * QUEUE_FLAG_SAME_FORCE, but actually not. blk_complete_request() is
 	 * running in interrupt handler, and currently I/O controller doesn't
 	 * support multiple interrupts, so current CPU is unique actually. This
 	 * avoids IPI sending from current CPU to the first CPU of a group.
 	 */
-	if (ccpu == cpu || ccpu == group_cpu) {
+	if (ccpu == cpu || shared) {
 		struct list_head *list;
 do_local:
 		list = &__get_cpu_var(blk_cpu_done);
block/blk.h +0 -16
index 7efd772..df5b59a 100644
@@ -164,22 +164,6 @@ static inline int queue_congestion_off_threshold(struct request_queue *q)
 	return q->nr_congestion_off;
 }
 
-static inline int blk_cpu_to_group(int cpu)
-{
-	int group = NR_CPUS;
-#ifdef CONFIG_SCHED_MC
-	const struct cpumask *mask = cpu_coregroup_mask(cpu);
-	group = cpumask_first(mask);
-#elif defined(CONFIG_SCHED_SMT)
-	group = cpumask_first(topology_thread_cpumask(cpu));
-#else
-	return cpu;
-#endif
-	if (likely(group < NR_CPUS))
-		return group;
-	return cpu;
-}
-
 /*
  * Contribute to IO statistics IFF:
  *
include/linux/sched.h +8 -0
index 513f524..0e19595 100644
@@ -1052,6 +1052,8 @@ static inline int test_sd_parent(struct sched_domain *sd, int flag)
 unsigned long default_scale_freq_power(struct sched_domain *sd, int cpu);
 unsigned long default_scale_smt_power(struct sched_domain *sd, int cpu);
 
+bool cpus_share_cache(int this_cpu, int that_cpu);
+
 #else /* CONFIG_SMP */
 
 struct sched_domain_attr;

@@ -1061,6 +1063,12 @@ partition_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
 			struct sched_domain_attr *dattr_new)
 {
 }
+
+static inline bool cpus_share_cache(int this_cpu, int that_cpu)
+{
+	return true;
+}
+
 #endif	/* !CONFIG_SMP */
kernel/sched/core.c +3 -3
index 5255c9d..d7c4322 100644
@@ -1507,7 +1507,7 @@ static int ttwu_activate_remote(struct task_struct *p, int wake_flags)
 }
 #endif /* __ARCH_WANT_INTERRUPTS_ON_CTXSW */
 
-static inline int ttwu_share_cache(int this_cpu, int that_cpu)
+bool cpus_share_cache(int this_cpu, int that_cpu)
 {
 	return per_cpu(sd_llc_id, this_cpu) == per_cpu(sd_llc_id, that_cpu);
 }

@@ -1518,7 +1518,7 @@ static void ttwu_queue(struct task_struct *p, int cpu)
 	struct rq *rq = cpu_rq(cpu);
 
 #if defined(CONFIG_SMP)
-	if (sched_feat(TTWU_QUEUE) && !ttwu_share_cache(smp_processor_id(), cpu)) {
+	if (sched_feat(TTWU_QUEUE) && !cpus_share_cache(smp_processor_id(), cpu)) {
 		sched_clock_cpu(cpu); /* sync clocks x-cpu */
 		ttwu_queue_remote(p, cpu);
 		return;

@@ -5754,7 +5754,7 @@ static void destroy_sched_domains(struct sched_domain *sd, int cpu)
  *
  * Also keep a unique ID per domain (we use the first cpu number in
  * the cpumask of the domain), this allows us to quickly tell if
- * two cpus are in the same cache domain, see ttwu_share_cache().
+ * two cpus are in the same cache domain, see cpus_share_cache().
  */
 DEFINE_PER_CPU(struct sched_domain *, sd_llc);
 DEFINE_PER_CPU(int, sd_llc_id);
author
#11 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

In reply to Peter Zijlstra
Quoted 28 lines · Peter Zijlstra
Quoted 12 lines · Vaidyanathan Srinivasan

+++ b/block/blk.h

block/blk.h +2 -5
@@ -167,14 +167,15 @@ static inline int queue_congestion_off_threshold(struct request_queue *q)
 static inline int blk_cpu_to_group(int cpu)
 {
        int group = NR_CPUS;
-#ifdef CONFIG_SCHED_MC
-       const struct cpumask *mask = cpu_coregroup_mask(cpu);
-       group = cpumask_first(mask);
-#elif defined(CONFIG_SCHED_SMT)
-       group = cpumask_first(topology_thread_cpumask(cpu));
+#ifdef CONFIG_SCHED_POWERSAVE
+       if (smt_capable())

block/blk.h static inline int queue_congestion_off_threshold(struct request_queue *q) · line 181

Quoted 10 lines
block/blk.h +4 -0
+               group = cpumask_first(topology_thread_cpumask(cpu));
+       else    
+               group = cpumask_first(cpu_coregroup_mask(cpu));
 #else
        return cpu;
 #endif
+       /* Possible dead code?? */
        if (likely(group < NR_CPUS))
                return group;
        return cpu;

After going, WTF is block doing! I had a closer look and this doesn't seem right at all. The old code would use coregroup_mask when SCHED_MC && SCHED_SMT, the new code does something else.

Quoted 1 line

Jens, what is this thing trying to do?

I understood the requirement as get first cpu in the 'core' on a hyper-threaded system and first cpu in the 'socket' on a non-threaded system for best cache affinity. Based on Jens explanation and Peter's patch, identifying last-level shared cache works best for this case.

--Vaidy

#12 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

In reply to Vaidyanathan Srinivasan

include/linux/sched.h enum cpu_idle_type { · line 858

Quoted 6 lines · Vaidyanathan Srinivasan
include/linux/sched.h +5 -0
+enum powersavings_level {
+       POWERSAVINGS_DISABLED = 0,      /* Max performance */
+       POWERSAVINGS_DEFAULT,           /* Kernel default policy, automatic powersave */
+                                       /* vs performance tradeoff */
+       POWERSAVINGS_MAX                /* Favour power savings over peformance */
 };

I don't like that, I can get, OFF, AUTO, ON, but to overload that with different policies for AUTO and ON just reeks.

author
#13 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 10 lines · Peter Zijlstra

include/linux/sched.h enum cpu_idle_type { · line 858

Quoted 6 lines · Vaidyanathan Srinivasan
include/linux/sched.h +5 -0
+enum powersavings_level {
+       POWERSAVINGS_DISABLED = 0,      /* Max performance */
+       POWERSAVINGS_DEFAULT,           /* Kernel default policy, automatic powersave */
+                                       /* vs performance tradeoff */
+       POWERSAVINGS_MAX                /* Favour power savings over peformance */
 };

I don't like that, I can get, OFF, AUTO, ON, but to overload that with different policies for AUTO and ON just reeks.

How about this:

enum powersavings_level {
POWERSAVINGS_OFF = 0, /* Max performance */
POWERSAVINGS_AUTO, /* Kernel default policy, automatic powersave */
/* vs performance tradeoff */
POWERSAVINGS_ON /* Max power savings */
};

Basically AUTO is where we have a 'policy' or heuristics, but simple straight forward decisions for OFF and ON case.

--Vaidy

#14 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

In reply to Vaidyanathan Srinivasan
Quoted 23 lines · Vaidyanathan Srinivasan
kernel/sched/core.c +2 -4
@@ -6150,10 +6150,8 @@ SD_INIT_FUNC(CPU)
  SD_INIT_FUNC(ALLNODES)
  SD_INIT_FUNC(NODE)
 #endif
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_SCHED_POWERSAVE
  SD_INIT_FUNC(SIBLING)
-#endif
-#ifdef CONFIG_SCHED_MC
  SD_INIT_FUNC(MC)
 #endif
 #ifdef CONFIG_SCHED_BOOK

@@ -6250,7 +6248,7 @@ static void claim_allocations(int cpu, struct sched_domain *sd)
                *per_cpu_ptr(sdd->sgp, cpu) = NULL;
 }
 
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_SCHED_POWERSAVE
 static const struct cpumask *cpu_smt_mask(int cpu)
 {
        return topology_thread_cpumask(cpu);

@@ -6261,10 +6259,8 @@ static const struct cpumask *cpu_smt_mask(int cpu)
  * Topology list, bottom-up.

kernel/sched/core.c static const struct cpumask *cpu_smt_mask(int cpu) · line 6266

Quoted 10 lines
kernel/sched/core.c +1 -3
  */
 static struct sched_domain_topology_level default_topology[] = {
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_SCHED_POWERSAVE
        { sd_init_SIBLING, cpu_smt_mask, },
-#endif
-#ifdef CONFIG_SCHED_MC
        { sd_init_MC, cpu_coregroup_mask, },
 #endif
 #ifdef CONFIG_SCHED_BOOK

I don't like this either, SCHED_{MC,SMT} here have nothing to do with powersavings, its topology support.

#15 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

On 1/25/2012 7:10 AM, Peter Zijlstra wrote:

btw... any reason why this feature is a config option with tons of ifdefs... why not just have this available all the time? it shouldn't be all that much code in the first place.

#16 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 6 lines · Arjan van de Ven

On 1/25/2012 7:10 AM, Peter Zijlstra wrote:

btw... any reason why this feature is a config option with tons of ifdefs... why not just have this available all the time? it shouldn't be all that much code in the first place.

I guess its because SCHED_SMT and SCHED_MC also indicate the availability of the topology functions like cpu_coregroup_mask() etc.

The whole powersave stuff got intermixed with all that.

But yes, I agree, we should kill all that and sort the topology stuff differently.

author
#17 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

In reply to Peter Zijlstra
Quoted 29 lines · Peter Zijlstra
Quoted 28 lines · Vaidyanathan Srinivasan
kernel/sched/core.c +3 -5
@@ -6150,10 +6150,8 @@ SD_INIT_FUNC(CPU)
  SD_INIT_FUNC(ALLNODES)
  SD_INIT_FUNC(NODE)
 #endif
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_SCHED_POWERSAVE
  SD_INIT_FUNC(SIBLING)
-#endif
-#ifdef CONFIG_SCHED_MC
  SD_INIT_FUNC(MC)
 #endif
 #ifdef CONFIG_SCHED_BOOK

@@ -6250,7 +6248,7 @@ static void claim_allocations(int cpu, struct sched_domain *sd)
                *per_cpu_ptr(sdd->sgp, cpu) = NULL;
 }
 
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_SCHED_POWERSAVE
 static const struct cpumask *cpu_smt_mask(int cpu)
 {
        return topology_thread_cpumask(cpu);

@@ -6261,10 +6259,8 @@ static const struct cpumask *cpu_smt_mask(int cpu)
  * Topology list, bottom-up.
  */
 static struct sched_domain_topology_level default_topology[] = {
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_SCHED_POWERSAVE
        { sd_init_SIBLING, cpu_smt_mask, },
Quoted 8 lines

kernel/sched/core.c static const struct cpumask *cpu_smt_mask(int cpu) · line 6266

Quoted 5 lines
kernel/sched/core.c +0 -2
-#endif
-#ifdef CONFIG_SCHED_MC
        { sd_init_MC, cpu_coregroup_mask, },
 #endif
 #ifdef CONFIG_SCHED_BOOK

I don't like this either, SCHED_{MC,SMT} here have nothing to do with powersavings, its topology support.

Yes, but we don't need these domains for any other purpose other than powersave balance. The code overheads are not high, I will remove the config option and check.

--Vaidy

#18 lore raw

Re: [RFC PATCH v1 1/2] sched: unified sched_powersavings sysfs tunable

Quoted 40 lines · Vaidyanathan Srinivasan
Quoted 29 lines · Peter Zijlstra
Quoted 28 lines · Vaidyanathan Srinivasan
kernel/sched/core.c +3 -5
@@ -6150,10 +6150,8 @@ SD_INIT_FUNC(CPU)
  SD_INIT_FUNC(ALLNODES)
  SD_INIT_FUNC(NODE)
 #endif
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_SCHED_POWERSAVE
  SD_INIT_FUNC(SIBLING)
-#endif
-#ifdef CONFIG_SCHED_MC
  SD_INIT_FUNC(MC)
 #endif
 #ifdef CONFIG_SCHED_BOOK

@@ -6250,7 +6248,7 @@ static void claim_allocations(int cpu, struct sched_domain *sd)
                *per_cpu_ptr(sdd->sgp, cpu) = NULL;
 }
 
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_SCHED_POWERSAVE
 static const struct cpumask *cpu_smt_mask(int cpu)
 {
        return topology_thread_cpumask(cpu);

@@ -6261,10 +6259,8 @@ static const struct cpumask *cpu_smt_mask(int cpu)
  * Topology list, bottom-up.
  */
 static struct sched_domain_topology_level default_topology[] = {
-#ifdef CONFIG_SCHED_SMT
+#ifdef CONFIG_SCHED_POWERSAVE
        { sd_init_SIBLING, cpu_smt_mask, },
Quoted 8 lines

kernel/sched/core.c static const struct cpumask *cpu_smt_mask(int cpu) · line 6266

Quoted 5 lines
kernel/sched/core.c +0 -2
-#endif
-#ifdef CONFIG_SCHED_MC
        { sd_init_MC, cpu_coregroup_mask, },
 #endif
 #ifdef CONFIG_SCHED_BOOK

I don't like this either, SCHED_{MC,SMT} here have nothing to do with powersavings, its topology support.

Quoted 3 lines

Yes, but we don't need these domains for any other purpose other than powersave balance. The code overheads are not high, I will remove the config option and check.

Yes we do, SMT perf skips siblings and only loads them when there's only siblings left idle. Similarly for MC, we prefer to run tasks on separate sockets when possible.

author
#19 lore raw

RFC PATCH 2/2 sched: fix group_capacity for thread level consolidation

In reply to Vaidyanathan Srinivasan

sched_powersavings for threaded systems need this fix for consolidation to sibling threads to work. Since threads have fractional capacity, group_capacity will turn out to be one always and not accommodate another task in the sibling thread.

This fix makes group_capacity a function of cpumask_weight that will enable the power saving load balancer to pack tasks among sibling threads and keep more cores idle.

Signed-off-by Vaidyanathan Srinivasan <[email protected]>
---
 kernel/sched/fair.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)
kernel/sched/fair.c +15 -0
index bae6ec8..c94e768 100644
@@ -4012,6 +4012,21 @@ static inline void update_sd_lb_stats(struct sched_domain *sd, int this_cpu,
 		 */
 		if (prefer_sibling && !local_group && sds->this_has_capacity)
 			sgs.group_capacity = min(sgs.group_capacity, 1UL);
+		/*
+		 * If power savings balance is set at this domain, then
+		 * make capacity equal to number of hardware threads to
+		 * accommodate more tasks until capacity is reached.
+		 */
+		else if (sd->flags & SD_POWERSAVINGS_BALANCE)
+			sgs.group_capacity =
+				cpumask_weight(sched_group_cpus(sg));
+
+			/*
+			 * The default group_capacity is rounded from sum of
+			 * fractional cpu_powers of sibling hardware threads
+			 * in order to enable fair use of available hardware
+			 * resources.
+			 */
 
 		if (local_group) {
 			sds->this_load = sgs.avg_load;
#20 lore raw

Re: [RFC PATCH v1 2/2] sched: fix group_capacity for thread level consolidation

kernel/sched/fair.c static inline void update_sd_lb_stats(struct sched_domain *sd, int this_cpu, · line 4022

Quoted 8 lines · Vaidyanathan Srinivasan
kernel/sched/fair.c +8 -0
+               /*
+                * If power savings balance is set at this domain, then
+                * make capacity equal to number of hardware threads to
+                * accommodate more tasks until capacity is reached.
+                */
+               else if (sd->flags & SD_POWERSAVINGS_BALANCE)
+                       sgs.group_capacity =
+                               cpumask_weight(sched_group_cpus(sg));

sg->group_weight perhaps?

author
#21 lore raw

Re: [RFC PATCH v1 2/2] sched: fix group_capacity for thread level consolidation

Quoted 11 lines · Peter Zijlstra

kernel/sched/fair.c static inline void update_sd_lb_stats(struct sched_domain *sd, int this_cpu, · line 4022

Quoted 8 lines · Vaidyanathan Srinivasan
kernel/sched/fair.c +8 -0
+               /*
+                * If power savings balance is set at this domain, then
+                * make capacity equal to number of hardware threads to
+                * accommodate more tasks until capacity is reached.
+                */
+               else if (sd->flags & SD_POWERSAVINGS_BALANCE)
+                       sgs.group_capacity =
+                               cpumask_weight(sched_group_cpus(sg));

sg->group_weight perhaps?

Yes, I will correct this. sg->group_weight is updated to the correct value on sched_domain rebuild in init_sched_groups_power().

--Vaidy