linux-kernel - Re: [RFC][PATCH 0/3] cpuidle,teo: Improve TEO tick decisions

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]

Message-ID: <11e17c44-7c4c-88de-546e-263c5a6aad2@linutronix.de>
Date:   Wed, 2 Aug 2023 10:12:20 +0200 (CEST)
From:   Anna-Maria Behnsen <anna-maria@...utronix.de>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
cc:     Peter Zijlstra <peterz@...radead.org>, tglx@...utronix.de,
        frederic@...nel.org, gautham.shenoy@....com,
        linux-kernel@...r.kernel.org, daniel.lezcano@...aro.org,
        linux-pm@...r.kernel.org, mingo@...hat.com, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com, vschneid@...hat.com
Subject: Re: [RFC][PATCH 0/3] cpuidle,teo: Improve TEO tick decisions

On Mon, 31 Jul 2023, Rafael J. Wysocki wrote:

> On Mon, Jul 31, 2023 at 1:47 PM Anna-Maria Behnsen
> <anna-maria@...utronix.de> wrote:
> >
> > Hi,
> >
> > On Fri, 28 Jul 2023, Peter Zijlstra wrote:
> >
> > > Hi,
> > >
> > > Wanted to send this yesterday, but my home server died and took everything
> > > down :/
> > >
> > > These patches are lightly tested but appear to behave as expected.
> > >
> > >
> >
> > As I was asked to see if the patches of Raphael improve the behavior, I
> > rerun the tests with Raphaels v2 as well as with Peters RFC patchset. Here
> > are the results (compared to upstream):
> 
> Thanks for the numbers!
> 
> >                         upstream                raphael v2              peter RFC
> >
> > Idle Total              2533    100.00%         1183    100.00%         5563    100.00%
> > x >= 4ms                1458    57.56%          1151    97.30%          3385    60.85%
> > 4ms> x >= 2ms           91      3.59%           12      1.01%           133     2.39%
> > 2ms > x >= 1ms          56      2.21%           3       0.25%           80      1.44%
> > 1ms > x >= 500us        64      2.53%           1       0.08%           98      1.76%
> > 500us > x >= 250us      73      2.88%           0       0.00%           113     2.03%
> > 250us > x >=100us       76      3.00%           2       0.17%           106     1.91%
> > 100us > x >= 50us       33      1.30%           4       0.34%           75      1.35%
> > 50us > x >= 25us        39      1.54%           4       0.34%           152     2.73%
> > 25us > x >= 10us        199     7.86%           4       0.34%           404     7.26%
> > 10us > x > 5us          156     6.16%           0       0.00%           477     8.57%
> > 5us > x                 288     11.37%          2       0.17%           540     9.71%
> >
> > tick_nohz_next_event()
> > count                   8839790                 6142079                 36623
> >
> > Raphaels Approach still does the tick_nohz_get_sleep_length() execution
> > unconditional. It executes ~5000 times more tick_nohz_next_event() as the
> > tick is stopped. This relation improved massively in Peters approach
> > (factor is ~7).
> 
> So I'm drawing slightly different conclusions from the above.
> 
> First, because there are different numbers of idle cycles in each
> case, I think it only makes sense to look at the percentages.
> 
> So in both the "upstream" and "Peter RFC" cases there is a significant
> percentage of times when the tick was stopped and the measure idle
> duration was below 25 us (25.39% and 25.54%, respectively), whereas in
> the "Rafael v2" case that is only 0.51% of times (not even 1%).  This
> means a huge improvement to me, because all of these cases mean that
> the governor incorrectly told the idle loop to stop the tick (it must
> have selected a shallow state and so it should have not stopped the
> tick in those cases).  To me, this clearly means fixing a real bug in
> the governor.
> 
> Now, I said in the changelog of my v1 that the goal was not to reduce
> the number of tick_nohz_get_sleep_length() invocations, so I'm not
> sure why it is regarded as a comparison criteria.
> 

I just mentioned this relation as the percentage values are obvious. I
know, your approach does not adress the tick_nohz_get_sleep_length
invocations, but it might be worth a try - I saw your new patchset
adressing it and will have a deeper look at it today/tomorrow. Thanks!

I also try to play around with the timer logics (moving marking timer base
idle into tick_nohz_stop_tick), but the results does not look pretty good
at the moment.