linux-kernel - Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy efficient

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJWu+or8XwnoA4SM93Cu2nhKoE8UwbdADYqPzxJ-KxOFL8uR-w@mail.gmail.com>
Date:   Wed, 12 Jul 2017 20:52:59 -0700
From:   Joel Fernandes <joelaf@...gle.com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Patrick Bellasi <patrick.bellasi@....com>,
        Juri Lelli <juri.lelli@....com>,
        Andres Oportus <andresoportus@...gle.com>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Len Brown <lenb@...nel.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy efficient

On Wed, Jul 12, 2017 at 8:35 PM, Viresh Kumar <viresh.kumar@...aro.org> wrote:
[..]
>
>> > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
>> > index 076a2e31951c..3459f327c94e 100644
>> > --- a/kernel/sched/cpufreq_schedutil.c
>> > +++ b/kernel/sched/cpufreq_schedutil.c
>> > @@ -53,6 +53,7 @@ struct sugov_cpu {
>> >         struct update_util_data update_util;
>> >         struct sugov_policy *sg_policy;
>> >
>> > +       bool iowait_boost_pending;
>> >         unsigned long iowait_boost;
>> >         unsigned long iowait_boost_max;
>> >         u64 last_update;
>> > @@ -169,7 +170,12 @@ 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;
>> > +               sg_cpu->iowait_boost_pending = true;
>> > +
>> > +               if (!sg_cpu->iowait_boost) {
>> > +                       sg_cpu->iowait_boost = sg_cpu->sg_policy->policy->cur;
>> > +                       sg_cpu->iowait_boost >>= 1;
>> > +               }
>>
>> Hmm, this doesn't look right to me.. why are we decaying in this path?
>
> I am convinced that we need a comment here as what I did wasn't
> straight forward :)
>
> The idea: We wouldn't increase the frequency for the first event with
> IOWAIT flag set, but on every subsequent event (captured over
> rate-limit-us window). You may have noticed that I am updating the
> boost values in sugov_iowait_boost() a bit earlier now and they will
> affect the current frequency update as well.
>
> Because I wanted to do a 2X there unconditionally if
> iowait_boost_pending is set, I had to make it half for the very first
> event with IOWAIT flag set.
>
> End result:
> - First event, we stay at current freq.
> - Second and all consecutive events every rate_limit_us time, 2X
> - If there is no IOWAIT event in last rate_limit_us, X/2
>
> Makes sense ?
>

Yes, that makes sense, its a bit subtle but I get what you're doing
now and I agree with it. Its also cleaner than my original patch :-)
and yeah definitely needs a comment ;-)

thanks,

-Joel