| Message ID | 1572483.euTsoFDNE9@vostro.rjw.lan (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On Mon, Aug 01, 2016 at 01:37:59AM +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Modify the schedutil cpufreq governor to boost the CPU frequency > if the UUF_IO flag is passed to it via cpufreq_update_util(). > > If that happens, the frequency is set to the maximum during > the first update after receiving the UUF_IO flag and then the > boost is reduced by half during each following update. Were these changes to schedutil part of the positive test results mentioned in patch 5? Or are those just from intel pstate? I was nervous about the effect of this on power and tested a couple low power usecases. The platform is the Hikey 96board (8 core ARM A53, single CPUfreq domain) running AOSP Android and schedutil backported to kernel 4.4. These tests run mp3 and mpeg4 playback for a little while, recording total energy consumption during the test along with frequency residency. As the results below show I did not measure an appreciable effect - if anything things may be slightly better with the patches. The hardcoding of a non-tunable boosting scheme makes me nervous but perhaps it could be revisited if some platform or configuration shows a noticeable regression? Testcase Energy /----- CPU frequency residency -----\ (J) 208000 432000 729000 960000 1200000 mp3-before-1 26.822 47.27% 24.79% 16.23% 5.20% 6.52% mp3-before-2 26.817 41.70% 28.75% 17.62% 5.17% 6.75% mp3-before-3 26.65 42.48% 28.65% 17.25% 5.07% 6.55% mp3-after-1 26.667 42.51% 27.38% 18.00% 5.40% 6.71% mp3-after-2 26.777 48.37% 24.15% 15.68% 4.55% 7.25% mp3-after-3 26.806 41.93% 27.71% 18.35% 4.78% 7.35% mpeg4-before-1 26.024 18.41% 60.09% 13.16% 0.49% 7.85% mpeg4-before-2 25.147 20.47% 64.80% 8.44% 1.37% 4.91% mpeg4-before-3 25.007 19.18% 66.08% 10.01% 0.59% 4.22% mpeg4-after-1 25.598 19.77% 61.33% 11.63% 0.79% 6.48% mpeg4-after-2 25.18 22.31% 62.78% 8.83% 1.18% 4.90% mpeg4-after-3 25.162 21.59% 64.88% 8.29% 0.49% 4.71% thanks, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, August 01, 2016 06:35:31 PM Steve Muckle wrote: > On Mon, Aug 01, 2016 at 01:37:59AM +0200, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Modify the schedutil cpufreq governor to boost the CPU frequency > > if the UUF_IO flag is passed to it via cpufreq_update_util(). > > > > If that happens, the frequency is set to the maximum during > > the first update after receiving the UUF_IO flag and then the > > boost is reduced by half during each following update. > > Were these changes to schedutil part of the positive test results > mentioned in patch 5? Or are those just from intel pstate? > > I was nervous about the effect of this on power and tested a couple low > power usecases. The platform is the Hikey 96board (8 core ARM A53, > single CPUfreq domain) running AOSP Android and schedutil backported to > kernel 4.4. These tests run mp3 and mpeg4 playback for a little while, > recording total energy consumption during the test along with frequency > residency. > > As the results below show I did not measure an appreciable effect - if > anything things may be slightly better with the patches. > > The hardcoding of a non-tunable boosting scheme makes me nervous but > perhaps it could be revisited if some platform or configuration shows > a noticeable regression? That would be my approach. :-) I'm not a big fan of tunables in general, as there are only a few people who actually set them to anything different from the default and then they get a lot of focus (even though they are after super-corner cases sometimes). Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux-pm/kernel/sched/cpufreq_schedutil.c =================================================================== --- linux-pm.orig/kernel/sched/cpufreq_schedutil.c +++ linux-pm/kernel/sched/cpufreq_schedutil.c @@ -48,11 +48,13 @@ struct sugov_cpu { struct sugov_policy *sg_policy; unsigned int cached_raw_freq; + unsigned long iowait_boost; + unsigned long iowait_boost_max; + u64 last_update; /* The fields below are only needed when sharing a policy. */ unsigned long util; unsigned long max; - u64 last_update; unsigned int flags; }; @@ -172,22 +174,58 @@ static void sugov_get_util(unsigned long } } +static void sugov_set_iowait_boost(struct sugov_cpu *sg_cpu, u64 time, + unsigned int flags) +{ + if (flags & UUF_IO) { + sg_cpu->iowait_boost = sg_cpu->iowait_boost_max; + } else if (sg_cpu->iowait_boost) { + s64 delta_ns = time - sg_cpu->last_update; + + /* Clear iowait_boost if the CPU apprears to have been idle. */ + if (delta_ns > TICK_NSEC) + sg_cpu->iowait_boost = 0; + } +} + +static void sugov_iowait_boost(struct sugov_cpu *sg_cpu, unsigned long *util, + unsigned long *max) +{ + unsigned long boost_util = sg_cpu->iowait_boost; + unsigned long boost_max = sg_cpu->iowait_boost_max; + + if (!boost_util) + return; + + if (*util * boost_max < *max * boost_util) { + *util = boost_util; + *max = boost_max; + } + sg_cpu->iowait_boost >>= 1; +} + static void sugov_update_single(struct update_util_data *hook, u64 time, unsigned int flags) { struct sugov_cpu *sg_cpu = container_of(hook, struct sugov_cpu, update_util); struct sugov_policy *sg_policy = sg_cpu->sg_policy; - struct cpufreq_policy *policy = sg_policy->policy; unsigned long util, max; unsigned int next_f; + sugov_set_iowait_boost(sg_cpu, time, flags); + sg_cpu->last_update = time; + if (!sugov_should_update_freq(sg_policy, time)) return; sugov_get_util(&util, &max, flags); - next_f = flags & UUF_RT ? policy->cpuinfo.max_freq : - get_next_freq(sg_cpu, util, max); + if (flags & UUF_RT) { + next_f = sg_policy->policy->cpuinfo.max_freq; + } else { + sugov_iowait_boost(sg_cpu, &util, &max); + next_f = get_next_freq(sg_cpu, util, max); + } sugov_update_commit(sg_policy, time, next_f); } @@ -204,6 +242,8 @@ static unsigned int sugov_next_freq_shar if (flags & UUF_RT) return max_f; + sugov_iowait_boost(sg_cpu, &util, &max); + for_each_cpu(j, policy->cpus) { struct sugov_cpu *j_sg_cpu; unsigned long j_util, j_max; @@ -218,12 +258,13 @@ static unsigned int sugov_next_freq_shar * frequency update and the time elapsed between the last update * of the CPU utilization and the last frequency update is long * enough, don't take the CPU into account as it probably is - * idle now. + * idle now (and clear iowait_boost for it). */ delta_ns = last_freq_update_time - j_sg_cpu->last_update; - if (delta_ns > TICK_NSEC) + if (delta_ns > TICK_NSEC) { + j_sg_cpu->iowait_boost = 0; continue; - + } if (j_sg_cpu->flags & UUF_RT) return max_f; @@ -233,6 +274,8 @@ static unsigned int sugov_next_freq_shar util = j_util; max = j_max; } + + sugov_iowait_boost(j_sg_cpu, &util, &max); } return get_next_freq(sg_cpu, util, max); @@ -253,6 +296,8 @@ static void sugov_update_shared(struct u sg_cpu->util = util; sg_cpu->max = max; sg_cpu->flags = flags; + + sugov_set_iowait_boost(sg_cpu, time, flags); sg_cpu->last_update = time; if (sugov_should_update_freq(sg_policy, time)) { @@ -485,6 +530,8 @@ static int sugov_start(struct cpufreq_po sg_cpu->flags = UUF_RT; sg_cpu->last_update = 0; sg_cpu->cached_raw_freq = 0; + sg_cpu->iowait_boost = 0; + sg_cpu->iowait_boost_max = policy->cpuinfo.max_freq; cpufreq_add_update_util_hook(cpu, &sg_cpu->update_util, sugov_update_shared); } else {