From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: ulf.hansson@linaro.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, khilman@kernel.org
Subject: Re: [PATCH RFC] cpuidle: consolidate calls to time capture
Date: Wed, 18 Mar 2020 15:32:29 +0100 [thread overview]
Message-ID: <93da445f-ebfd-4d2e-c8b4-cd81e54892c0@linaro.org> (raw)
In-Reply-To: <669fe03f-0d65-8ca9-53dc-1323e0397c53@linaro.org>
On 18/03/2020 12:04, Daniel Lezcano wrote:
>
> Hi Rafael,
>
> On 18/03/2020 11:17, Rafael J. Wysocki wrote:
>> On Monday, March 16, 2020 10:08:43 PM CET Daniel Lezcano wrote:
>>> A few years ago, we changed the code in cpuidle to replace ktime_get()
>>> by a local_clock() to get rid of potential seq lock in the path and an
>>> extra latency.
>>>
>>> Meanwhile, the code evolved and we are getting the time in some other
>>> places like the power domain governor and in the future break even
>>> deadline proposal.
>>
>> Hmm?
>>
>> Have any patches been posted for that?
>
> https://lkml.org/lkml/2020/3/11/1113
>
> https://lkml.org/lkml/2020/3/13/466
>
> but there is no consensus yet if that has a benefit or not.
>
>>> Unfortunately, as the time must be compared across the CPU, we have no
>>> other option than using the ktime_get() again. Hopefully, we can
>>> factor out all the calls to local_clock() and ktime_get() into a
>>> single one when the CPU is entering idle as the value will be reuse in
>>> different places.
>>
>> So there are cases in which it is not necessary to synchronize the time
>> between CPUs and those would take the overhead unnecessarily.
>>
>> This change looks premature to me at least.
>
> The idea is to call one time ktime_get() when entering idle and store
> the result in the struct cpuidle_device, so we have the information when
> we entered idle.
>
> Moreover, ktime_get() is called in do_idle() via:
>
> tick_nohz_idle_enter()
> tick_nohz_start_idle()
> ts->idle_entrytime = ktime_get();
>
> This is called at the first loop level. The idle loop is exiting and
> re-entering again without passing through tick_nohz_idle_enter() in the
> second loop level in case of interrupt processing, thus the
> idle_entrytime is not updated and the return of
> tick_nohz_get_sleep_length() will be greater than what is expected.
>
> May be we can consider ktime_get_mono_fast_ns() which is lockless with a
> particular care of the non-monotonic aspect if needed. Given the
> description at [1] the time jump could a few nanoseconds in case of NMI.
>
> The local_clock() can no be inspected across CPUs, the gap is too big
> and continues to increase during system lifetime.
I took the opportunity to measure the duration to a call to ktime_get,
ktime_get_mono_fast_ns and local_clock.
The result is an average of 10000 measurements and an average of 1000
run of those.
The duration is measured with local_clock(), ktime_get() and
ktime_get_mono_fast_ns()
Measurement with local_clock():
-------------------------------
ktime_get():
N min max sum mean stddev
1000 71 168 109052 109.052 13.0278
ktime_get_mono_fast_ns():
N min max sum mean stddev
1000 66 153 101135 101.135 11.9262
local_clock():
N min max sum mean stddev
1000 70 163 106896 106.896 12.8575
Measurement with ktime_get():
-----------------------------
ktime_get():
N min max sum mean stddev
1000 71 124 100465 100.465 10.0272
ktime_get_mono_fast_ns():
N min max sum mean stddev
1000 67 124 94451 94.451 9.67218
local_clock():
N min max sum mean stddev
1000 71 123 99765 99.765 10.0508
Measurement with ktime_get_mono_fast_ns():
------------------------------------------
ktime_get():
N min max sum mean stddev
1000 67 116 87562 87.562 4.38399
ktime_get_mono_fast_ns():
N min max sum mean stddev
1000 62 104 81017 81.017 4.12453
local_clock():
N min max sum mean stddev
1000 65 110 85919 85.919 4.24859
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
next prev parent reply other threads:[~2020-03-18 14:32 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-16 21:08 [PATCH RFC] cpuidle: consolidate calls to time capture Daniel Lezcano
2020-03-18 10:17 ` Rafael J. Wysocki
2020-03-18 11:04 ` Daniel Lezcano
2020-03-18 14:32 ` Daniel Lezcano [this message]
2020-03-18 21:29 ` Rafael J. Wysocki
2020-03-18 10:49 ` Ulf Hansson
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=93da445f-ebfd-4d2e-c8b4-cd81e54892c0@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=khilman@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=ulf.hansson@linaro.org \
/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;
as well as URLs for NNTP newsgroup(s).