Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932335AbdGKKOi (ORCPT ); Tue, 11 Jul 2017 06:14:38 -0400 Received: from mail-pf0-f176.google.com ([209.85.192.176]:35314 "EHLO mail-pf0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753749AbdGKKOh (ORCPT ); Tue, 11 Jul 2017 06:14:37 -0400 Date: Tue, 11 Jul 2017 15:44:32 +0530 From: Viresh Kumar To: Joel Fernandes Cc: linux-kernel@vger.kernel.org, patrick.bellasi@arm.com, juri.lelli@arm.com, andresoportus@google.com, dietmar.eggemann@arm.com, Srinivas Pandruvada , Len Brown , "Rafael J . Wysocki" , Ingo Molnar , Peter Zijlstra Subject: Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy efficient Message-ID: <20170711101432.GB17115@vireshk-i7> References: <20170709170826.29396-1-joelaf@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170709170826.29396-1-joelaf@google.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2708 Lines: 74 On 09-07-17, 10:08, Joel Fernandes wrote: > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c > index 622eed1b7658..4d9e8b96bed1 100644 > --- a/kernel/sched/cpufreq_schedutil.c > +++ b/kernel/sched/cpufreq_schedutil.c > @@ -53,7 +53,9 @@ struct sugov_cpu { > struct update_util_data update_util; > struct sugov_policy *sg_policy; > > + bool prev_iowait_boost; > unsigned long iowait_boost; > + unsigned long iowait_boost_min; > unsigned long iowait_boost_max; > u64 last_update; > > @@ -168,22 +170,47 @@ static void sugov_get_util(unsigned long *util, unsigned long *max) > *max = cfs_max; > } > > +static void sugov_decay_iowait_boost(struct sugov_cpu *sg_cpu) > +{ > + sg_cpu->iowait_boost >>= 1; > + > + if (sg_cpu->iowait_boost < sg_cpu->iowait_boost_min) > + sg_cpu->iowait_boost = 0; > +} > + > static void sugov_set_iowait_boost(struct sugov_cpu *sg_cpu, u64 time, > unsigned int flags) > { > if (flags & SCHED_CPUFREQ_IOWAIT) { > - sg_cpu->iowait_boost = sg_cpu->iowait_boost_max; > + /* Remember for next time that we did an iowait boost */ > + sg_cpu->prev_iowait_boost = true; > + if (sg_cpu->iowait_boost) { > + sg_cpu->iowait_boost <<= 1; > + sg_cpu->iowait_boost = min(sg_cpu->iowait_boost, > + sg_cpu->iowait_boost_max); > + } else { > + sg_cpu->iowait_boost = sg_cpu->iowait_boost_min; I am not sure if boost should start from the min frequency, as the current frequency will at least be equal to that. Which means that with no boost initially, we will never increase the frequency for the first IOWAIT flag event. > + } > } 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; > + > + /* > + * Since we don't decay iowait_boost when its consumed during > + * the previous SCHED_CPUFREQ_IOWAIT update, decay it now. > + */ > + if (sg_cpu->prev_iowait_boost) { SCHED_CPUFREQ_IOWAIT flag is set only by CFS from the enqueue_task() and in many cases we call the util hook twice from the same enqueue_task() instance before returning (2nd one after updating util). And in such cases we will set iowait_boost as 0 on the second call. Have you ever seen two consecutive calls to sugov_set_iowait_boost() with IOWAIT flag set ? Can we get the ratio of that against the other case where we have IOWAIT flag set in first call, followed by one or more non-IOWAIT calls and then IOWAIT again ? I am asking because if the calls with IOWAIT flag aren't consecutive then we will make iowait_boost as 0 in the next non-IOWAIT call. -- viresh