Date: Tue, 11 Jul 2017 15:38:31 +0100
From: Juri Lelli <juri.lelli@arm.com>
To: Joel Fernandes <joelaf@google.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
        Patrick Bellasi <patrick.bellasi@arm.com>,
        Andres Oportus <andresoportus@google.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        Len Brown <lenb@kernel.org>, "Rafael J . Wysocki" <rjw@rjwysocki.net>,
        Viresh Kumar <viresh.kumar@linaro.org>, Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy
 efficient
Message-ID: <20170711143831.bn3th6wudo5qyflu@e106622-lin>
References: <20170709170826.29396-1-joelaf@google.com>
 <20170710105500.ef26lvurzg6ptbhq@e106622-lin>
 <CAJWu+oqwKyVBt7R_9Rt0gcScKoubMHhXN11SByuQ3x2vVG1zdg@mail.gmail.com>
 <20170711071401.475zpaqx5eyiotjy@e106622-lin>
 <CAJWu+opKdDJygN00_CNh17p+ZU971zd+FZKiMhTrUtHOXkYS1g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAJWu+opKdDJygN00_CNh17p+ZU971zd+FZKiMhTrUtHOXkYS1g@mail.gmail.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2079
Lines: 54

On 11/07/17 07:33, Joel Fernandes wrote:
> Hi Juri,
> 
> On Tue, Jul 11, 2017 at 12:14 AM, Juri Lelli <juri.lelli@arm.com> wrote:
> [..]
> >> > Considering it a per-cpu thing, isn't enough that it gets bumped up or
> >> > decayed only when a CPU does an update (by using the above from
> >> > sugov_update_shared)?
> >> >
> >> > If we go this way I think we will only need to reset prev_iowait_boost
> >> > if delta_ns > TICK_NSEC during the for_each_cpu() loop of sugov_next_
> >> > freq_shared().
> >> >
> >>
> >> Actually the "decay" was already being done before (without this
> >> patch), I am just preserving the same old behavior where we do decay.
> >> Perhaps your proposal can be a separate match? Or did I miss something
> >> else subtle here?
> >>
> >
> > True, we are currently decaying anyway.
> >
> > Looking again at this path made me however think if we really need to. I
> > guess we need currently, as we bump frenquency to max and then decay it.
> > But, with your changes, I was wondering if we can simplify the thing and
> > decay only on the per-CPU update path.
> 
> Yes that makes sense to me, but even if we're at max, we should still
> benefit from your idea right? (I didn't follow why being at min makes
> the idea better, I think its a good idea in both cases (whether we are
> boosting to max and ramping down or starting from the min) but let me
> know if I missed something)
> 

Not better, but I thought that if we start from max and decay we
probably need to have more chances to decay faster (so decaying also
when aggregating request for a domain is probably needed)?

> If I understand correctly what you're proposing:
> 1. Remove the decay from sugov_iowait_boost
> 2. Add the iowait_boost decay unconditionally to
> sugov_set_iowait_boost for the !SCHED_CPUFREQ_IOWAIT case.
> 
> That would also get rid of the prev_iowait_boost flag and simplify
> things, and also address Peter's concern of adding 'bool' in composite
> types :)
> 

OK, just a suggestion however, didn't play with the idea myself, it
might not work. :)

Thanks,

- Juri