Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755496AbdGKLj4 (ORCPT ); Tue, 11 Jul 2017 07:39:56 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:45441 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752659AbdGKLjz (ORCPT ); Tue, 11 Jul 2017 07:39:55 -0400 Date: Tue, 11 Jul 2017 13:39:45 +0200 From: Peter Zijlstra To: Viresh Kumar Cc: Joel Fernandes , 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 Subject: Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy efficient Message-ID: <20170711113945.7wyw7un5mma4m3gq@hirez.programming.kicks-ass.net> References: <20170709170826.29396-1-joelaf@google.com> <20170711101432.GB17115@vireshk-i7> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170711101432.GB17115@vireshk-i7> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1965 Lines: 49 On Tue, Jul 11, 2017 at 03:44:32PM +0530, Viresh Kumar wrote: > 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. I suspect this actually works for Joel to get rid of the transient spikes he was seeing. Starting at the current freq, as you suggest, appears to make sense, but would add immediate transients back.