linux-kernel - Re: [RFT][PATCH v5 0/7] sched/cpuidle: Idle loop rework

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 19 Mar 2018 12:58:00 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Doug Smythies <dsmythies@...us.net>,
        Thomas Ilsche <thomas.ilsche@...dresden.de>,
        Linux PM <linux-pm@...r.kernel.org>,
        Frederic Weisbecker <fweisbec@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Paul McKenney <paulmck@...ux.vnet.ibm.com>,
        Rik van Riel <riel@...riel.com>,
        Aubrey Li <aubrey.li@...ux.intel.com>,
        Mike Galbraith <mgalbraith@...e.de>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFT][PATCH v5 0/7] sched/cpuidle: Idle loop rework

On Mon, Mar 19, 2018 at 12:36 PM, Rafael J. Wysocki <rafael@...nel.org> wrote:
> On Mon, Mar 19, 2018 at 11:49 AM, Peter Zijlstra <peterz@...radead.org> wrote:
>> On Sun, Mar 18, 2018 at 05:15:22PM +0100, Rafael J. Wysocki wrote:
>>> On Sun, Mar 18, 2018 at 12:00 PM, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
>>> > @@ -354,6 +360,7 @@ static int menu_select(struct cpuidle_dr
>>> >         if (latency_req > interactivity_req)
>>> >                 latency_req = interactivity_req;
>>> >
>>> > +       expected_interval = TICK_USEC_HZ;
>>> >         /*
>>> >          * Find the idle state with the lowest power while satisfying
>>> >          * our constraints.
>>> > @@ -367,17 +374,44 @@ static int menu_select(struct cpuidle_dr
>>> >                         continue;
>>> >                 if (idx == -1)
>>> >                         idx = i; /* first enabled state */
>>> > -               if (s->target_residency > data->predicted_us)
>>> > +               if (s->target_residency > data->predicted_us) {
>>> > +                       /*
>>> > +                        * Retain the tick if the selected state is shallower
>>> > +                        * than the deepest available one with target residency
>>> > +                        * within the tick period range.
>>> > +                        *
>>> > +                        * This allows the tick to be stopped even if the
>>> > +                        * predicted idle duration is within the tick period
>>> > +                        * range to counter the effect by which the prediction
>>> > +                        * may be skewed towards lower values due to the tick
>>> > +                        * bias.
>>> > +                        */
>>> > +                       expected_interval = s->target_residency;
>>> >                         break;
>>>
>>> BTW, I guess I need to explain the motivation here more thoroughly, so
>>> here it goes.
>>>
>>> The governor predicts idle duration under the assumption that the
>>> tick will be stopped, so if the result of the prediction is within the tick
>>> period range and it is not accurate, that needs to be taken into
>>> account in the governor's statistics.  However, if the tick is allowed
>>> to run every time the governor predicts idle duration within the tick
>>> period range, the governor will always see that it was "almost
>>> right" and the correction factor applied by it to improve the
>>> prediction next time will not be sufficient.  For this reason, it
>>> is better to stop the tick at least sometimes when the governor
>>> predicts idle duration within the tick period range and the idea
>>> here is to do that when the selected state is the deepest available
>>> one with the target residency within the tick period range.  This
>>> allows the opportunity to save more energy to be seized which
>>> balances the extra overhead of stopping the tick.
>>
>> My brain is just not willing to understand how that work this morning.
>> Also it sounds really dodgy.
>
> Well, I guess I can't really explain it better. :-)
>
> The reason why this works better than the original v5 is because of
> how menu_update() works AFAICS.

Actually, it looks like menu_update() doesn't do the right thing when
the tick isn't stopped at all, because data->next_timer_us is useless
then.

Let me try to fix that in a new respin of the series.