MIME-Version: 1.0
In-Reply-To: <20170711071401.475zpaqx5eyiotjy@e106622-lin>
References: <20170709170826.29396-1-joelaf@google.com> <20170710105500.ef26lvurzg6ptbhq@e106622-lin>
 <CAJWu+oqwKyVBt7R_9Rt0gcScKoubMHhXN11SByuQ3x2vVG1zdg@mail.gmail.com> <20170711071401.475zpaqx5eyiotjy@e106622-lin>
From: Joel Fernandes <joelaf@google.com>
Date: Tue, 11 Jul 2017 07:33:29 -0700
Message-ID: <CAJWu+opKdDJygN00_CNh17p+ZU971zd+FZKiMhTrUtHOXkYS1g@mail.gmail.com>
Subject: Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy efficient
To: Juri Lelli <juri.lelli@arm.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>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1671
Lines: 44

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)

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 :)

thanks,

-Joel