From 8e7fbcbc22c12414bcc9dfdd683637f58fb32759 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 9 Jan 2012 11:28:35 +0100 Subject: sched: Remove stale power aware scheduling remnants and dysfunctional knobs It's been broken forever (i.e. it's not scheduling in a power aware fashion), as reported by Suresh and others sending patches, and nobody cares enough to fix it properly ... so remove it to make space free for something better. There's various problems with the code as it stands today, first and foremost the user interface which is bound to topology levels and has multiple values per level. This results in a state explosion which the administrator or distro needs to master and almost nobody does. Furthermore large configuration state spaces aren't good, it means the thing doesn't just work right because it's either under so many impossibe to meet constraints, or even if there's an achievable state workloads have to be aware of it precisely and can never meet it for dynamic workloads. So pushing this kind of decision to user-space was a bad idea even with a single knob - it's exponentially worse with knobs on every node of the topology. There is a proposal to replace the user interface with a single 3 state knob: sched_balance_policy := { performance, power, auto } where 'auto' would be the preferred default which looks at things like Battery/AC mode and possible cpufreq state or whatever the hw exposes to show us power use expectations - but there's been no progress on it in the past many months. Aside from that, the actual implementation of the various knobs is known to be broken. There have been sporadic attempts at fixing things but these always stop short of reaching a mergable state. Therefore this wholesale removal with the hopes of spurring people who care to come forward once again and work on a coherent replacement. Signed-off-by: Peter Zijlstra Cc: Suresh Siddha Cc: Arjan van de Ven Cc: Vincent Guittot Cc: Vaidyanathan Srinivasan Cc: Linus Torvalds Cc: Andrew Morton Link: http://lkml.kernel.org/r/1326104915.2442.53.camel@twins Signed-off-by: Ingo Molnar --- tools/power/cpupower/man/cpupower-set.1 | 9 -------- tools/power/cpupower/utils/helpers/sysfs.c | 35 ++---------------------------- 2 files changed, 2 insertions(+), 42 deletions(-) (limited to 'tools/power') diff --git a/tools/power/cpupower/man/cpupower-set.1 b/tools/power/cpupower/man/cpupower-set.1 index c4954a9fe4e..9dbd536518a 100644 --- a/tools/power/cpupower/man/cpupower-set.1 +++ b/tools/power/cpupower/man/cpupower-set.1 @@ -85,15 +85,6 @@ Possible values are: savings .RE -sched_mc_power_savings is dependent upon SCHED_MC, which is -itself architecture dependent. - -sched_smt_power_savings is dependent upon SCHED_SMT, which -is itself architecture dependent. - -The two files are independent of each other. It is possible -that one file may be present without the other. - .SH "SEE ALSO" cpupower-info(1), cpupower-monitor(1), powertop(1) .PP diff --git a/tools/power/cpupower/utils/helpers/sysfs.c b/tools/power/cpupower/utils/helpers/sysfs.c index c6343024a61..96e28c124b5 100644 --- a/tools/power/cpupower/utils/helpers/sysfs.c +++ b/tools/power/cpupower/utils/helpers/sysfs.c @@ -362,22 +362,7 @@ char *sysfs_get_cpuidle_driver(void) */ int sysfs_get_sched(const char *smt_mc) { - unsigned long value; - char linebuf[MAX_LINE_LEN]; - char *endp; - char path[SYSFS_PATH_MAX]; - - if (strcmp("mc", smt_mc) && strcmp("smt", smt_mc)) - return -EINVAL; - - snprintf(path, sizeof(path), - PATH_TO_CPU "sched_%s_power_savings", smt_mc); - if (sysfs_read_file(path, linebuf, MAX_LINE_LEN) == 0) - return -1; - value = strtoul(linebuf, &endp, 0); - if (endp == linebuf || errno == ERANGE) - return -1; - return value; + return -ENODEV; } /* @@ -388,21 +373,5 @@ int sysfs_get_sched(const char *smt_mc) */ int sysfs_set_sched(const char *smt_mc, int val) { - char linebuf[MAX_LINE_LEN]; - char path[SYSFS_PATH_MAX]; - struct stat statbuf; - - if (strcmp("mc", smt_mc) && strcmp("smt", smt_mc)) - return -EINVAL; - - snprintf(path, sizeof(path), - PATH_TO_CPU "sched_%s_power_savings", smt_mc); - sprintf(linebuf, "%d", val); - - if (stat(path, &statbuf) != 0) - return -ENODEV; - - if (sysfs_write_file(path, linebuf, MAX_LINE_LEN) == 0) - return -1; - return 0; + return -ENODEV; } -- cgit v1.2.1 From d15cf7c129fa4ec4b44c52521e49ffafb9749029 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Sun, 3 Jun 2012 23:24:00 -0400 Subject: tools/power turbostat: fix un-intended affinity of forked program Linux 3.4 included a modification to turbostat to lower cross-call overhead by using scheduler affinity: 15aaa34654831e98dd76f7738b6c7f5d05a66430 (tools turbostat: reduce measurement overhead due to IPIs) In the use-case where turbostat forks a child program, that change had the un-intended side-effect of binding the child to the last cpu in the system. This change removed the binding before forking the child. This is a back-port of a fix already included in turbostat v2. Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) (limited to 'tools/power') diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index ab2f682fd44..d5d6a3dc9ba 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -73,8 +73,8 @@ int backwards_count; char *progname; int num_cpus; -cpu_set_t *cpu_mask; -size_t cpu_mask_size; +cpu_set_t *cpu_present_set, *cpu_mask; +size_t cpu_present_setsize, cpu_mask_size; struct counters { unsigned long long tsc; /* per thread */ @@ -103,6 +103,12 @@ struct timeval tv_even; struct timeval tv_odd; struct timeval tv_delta; +int mark_cpu_present(int pkg, int core, int cpu) +{ + CPU_SET_S(cpu, cpu_present_setsize, cpu_present_set); + return 0; +} + /* * cpu_mask_init(ncpus) * @@ -118,6 +124,18 @@ void cpu_mask_init(int ncpus) } cpu_mask_size = CPU_ALLOC_SIZE(ncpus); CPU_ZERO_S(cpu_mask_size, cpu_mask); + + /* + * Allocate and initialize cpu_present_set + */ + cpu_present_set = CPU_ALLOC(ncpus); + if (cpu_present_set == NULL) { + perror("CPU_ALLOC"); + exit(3); + } + cpu_present_setsize = CPU_ALLOC_SIZE(ncpus); + CPU_ZERO_S(cpu_present_setsize, cpu_present_set); + for_all_cpus(mark_cpu_present); } void cpu_mask_uninit() @@ -125,6 +143,9 @@ void cpu_mask_uninit() CPU_FREE(cpu_mask); cpu_mask = NULL; cpu_mask_size = 0; + CPU_FREE(cpu_present_set); + cpu_present_set = NULL; + cpu_present_setsize = 0; } int cpu_migrate(int cpu) @@ -1047,6 +1068,9 @@ int fork_it(char **argv) int retval; pid_t child_pid; get_counters(cnt_even); + + /* clear affinity side-effect of get_counters() */ + sched_setaffinity(0, cpu_present_setsize, cpu_present_set); gettimeofday(&tv_even, (struct timezone *)NULL); child_pid = fork(); -- cgit v1.2.1 From 650a37f32d2bc16fa802075be579802bc4ec4132 Mon Sep 17 00:00:00 2001 From: Len Brown Date: Sun, 3 Jun 2012 23:34:44 -0400 Subject: tools/power turbostat: fix IVB support Initial IVB support went into turbostat in Linux-3.1: 553575f1ae048aa44682b46b3c51929a0b3ad337 (tools turbostat: recognize and run properly on IVB) However, when running on IVB, turbostat would fail to report the new couters added with SNB, c7, pc2 and pc7. So in scenarios where these counters are non-zero on IVB, turbostat would report erroneous residencey results. In particular c7 time would be added to c1 time, since c1 time is calculated as "that which is left over". Also, turbostat reports MHz capabilities when passed the "-v" option, and it would incorrectly report 133MHz bclk instead of 100MHz bclk for IVB, which would inflate GHz reported with that option. This patch is a backport of a fix already included in turbostat v2. Signed-off-by: Len Brown --- tools/power/x86/turbostat/turbostat.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'tools/power') diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index d5d6a3dc9ba..16de7ad4850 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -933,6 +933,8 @@ int is_snb(unsigned int family, unsigned int model) switch (model) { case 0x2A: case 0x2D: + case 0x3A: /* IVB */ + case 0x3D: /* IVB Xeon */ return 1; } return 0; -- cgit v1.2.1