Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy efficient
From: Juri Lelli
Date: Tue Jul 11 2017 - 10:38:43 EST
On 11/07/17 07:33, Joel Fernandes wrote:
> Hi Juri,
> On Tue, Jul 11, 2017 at 12:14 AM, Juri Lelli <juri.lelli@xxxxxxx> 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. :)