public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Doug Smythies" <dsmythies@telus.net>
To: "'Rafael J. Wysocki'" <rafael@kernel.org>,
	"'Rafael J. Wysocki'" <rjw@rjwysocki.net>
Cc: "'Linux PM'" <linux-pm@vger.kernel.org>,
	"'LKML'" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 2/3] cpuidle: teo: Avoid expecting unrealistic idle times
Date: Thu, 14 Nov 2019 17:24:02 -0800	[thread overview]
Message-ID: <001a01d59b53$5eb8ae10$1c2a0a30$@net> (raw)
In-Reply-To: <CAJZ5v0jiBrx2xxf2EdvGrqke0ByK5YWXEw0R2rpHZYAJjgwJmg@mail.gmail.com>

On 2019.11.14 15:51 Rafael J. Wysocki wrote:
> On Wed, Nov 13, 2019 at 1:11 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> If an idle state shallower than the one "matching" the time till the
>> next timer event is considered for selection, expect the idle duration
>> to fall in the middle of the "bin" corresponding to that state rather
>> than at the beginning of it which is unrealistic.
>>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> ---
>>  drivers/cpuidle/governors/teo.c |    9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> Index: linux-pm/drivers/cpuidle/governors/teo.c
>> ===================================================================
>> --- linux-pm.orig/drivers/cpuidle/governors/teo.c
>> +++ linux-pm/drivers/cpuidle/governors/teo.c
>> @@ -360,7 +360,14 @@ static int teo_select(struct cpuidle_dri
>>
>>                 if (max_early_idx >= 0) {
>>                         idx = max_early_idx;
>> -                       duration_ns = drv->states[idx].target_residency_ns;
>> +                       /*
>> +                        * Expect the idle duration to fall in the middle of the
>> +                        * "bin" corresponding to idx (note that the maximum
>> +                        * state index is guaranteed to be greater than idx at
>> +                        * this point).
>> +                        */
>> +                       duration_ns = (drv->states[idx].target_residency_ns +
>> +                               drv->states[idx+1].target_residency_ns) / 2;
>>                 }
>>         }
>
> This change turns out to cause the governor to choose idle states that
> are too deep or too shallow too often, so I'm withdrawing it.

O.K. thanks for letting us know.
I did see some differences in the testing I did so far, but hadn't drilled down
into it yet.
I am somewhat wondering about the above and below stats in general.

By the way, I had a daft mistake in my post processing program, such that the
"below" graph for idle state 0 was always plotting 0.

Reference for that sweep test that I do (which is as far I got so far):
http://www.smythies.com/~doug/linux/idle/teo-2019-11/sweep/index.html

Legend:

teo-v2: re-run of previous teo-v2 so that I could get non-zero idle state "below" data
linux-next 2019.11.07 + cpuidle: Consolidate disabled state checks +
[PATCH v2] cpuidle: Use nanoseconds as the unit of time

teo-v3: teo-v2 + cpuidle: teo: Exclude cpuidle overhead from computations

teo-v4: linux-pm + linux-next 2019.11.12 +
cpuidle: teo: Avoid code duplication in conditionals
cpuidle: teo: Avoid expecting unrealistic idle times
cpuidle: teo: Avoid using "early hits" incorrectly

teo-v5: teo-v4 + cpuidle: teo: Exclude cpuidle overhead from computations

... Doug



  reply	other threads:[~2019-11-15  1:24 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-12 23:57 [PATCH 0/3] cpuidle: teo: Bug fix, formal correction and cleanup Rafael J. Wysocki
2019-11-13  0:03 ` [PATCH 1/3] cpuidle: teo: Avoid using "early hits" incorrectly Rafael J. Wysocki
2019-11-13  0:07 ` [PATCH 2/3] cpuidle: teo: Avoid expecting unrealistic idle times Rafael J. Wysocki
2019-11-14 23:50   ` Rafael J. Wysocki
2019-11-15  1:24     ` Doug Smythies [this message]
2019-11-15  9:07       ` Rafael J. Wysocki
2019-11-13  0:10 ` [PATCH 3/3] cpuidle: teo: Avoid code duplication in conditionals Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='001a01d59b53$5eb8ae10$1c2a0a30$@net' \
    --to=dsmythies@telus.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox