From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Leo Yan <leo.yan@linaro.org>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
"Peter Zijlstra (Intel)" <peterz@infradead.org>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Vincent Guittot <vincent.guittot@linaro.org>,
Ramesh Thomas <ramesh.thomas@intel.com>,
linux-kernel@vger.kernel.org, Linux PM <linux-pm@vger.kernel.org>
Subject: Re: [PATCH v1 0/5] Improvement stopping tick decision making in 'menu' idle governor
Date: Tue, 21 Aug 2018 10:37:42 +0200 [thread overview]
Message-ID: <1980448.5HStMnL72j@aspire.rjw.lan> (raw)
In-Reply-To: <1534090171-14464-1-git-send-email-leo.yan@linaro.org>
On Sunday, August 12, 2018 6:09:26 PM CEST Leo Yan wrote:
> We found the CPU cannot stay in deepest idle state as expected with
> running synthetic workloads with mainline kernel on Arm platform
> (96boards Hikey620 with octa CA53 CPUs).
>
> The main issue is the criteria for decision stopping tick; now
> the criteria is checking expected interval is less than TICK_USEC, but
> this doesn't consider the next tick detla is float due CPU randomly
> eneters and exits idle states; furthermore, it's stick to checking
> TICK_USEC as boundary for decision stopping tick, unfortunately this has
> hole to select a shallow state with stopping tick, so the CPU stays in
> shallow state for long time.
>
> This patch series is to explore more reasonable making decision for
> stopping tick and the most important fixing is to avoid powernightmares
> issue after we apply these criterias for making decisions. Patches
> 0001 ~ 0003 are used to refactor the variables and structures for more
> readable code, it also provides a function menu_decide_stopping_tick()
> which can be used to encapsulate the making decision logics. The last
> two patches are primary for improvement, patch 0004 'cpuidle: menu:
> Don't stay in shallow state for a long time' introduces a new criteria
> (it's a more strict criteria than before) for not stopping tick for
> shallow state cases; patch 0005 is use the dynamic tick detla to replace
> the static value TICK_USEC for decision if the tick is expired before or
> after the prediction, according this comparison we can get conclusion if
> need to stop tick or not.
>
> With more accurate decision for stopping tick, one immediate benefit is
> the CPUs have more chance to stay in deepest state, it also can avoid to
> run tick unnecessarily and so avoid a shallower state introduced by tick
> event. For the testing result in below table, we can see the result
> proves the improvement by better stopping tick decision making in this
> patch series, we run the workload generated by rt-app (a single task
> with period 5ms and duty cycle 1%/3%/5%/10%/20%/30%/40%), the total
> running time is 60s. We do statistics for all CPUs for all idle states
> duration, the unit is second (s), for cases (dutycycle=1%/3%/5%/10%/20%)
> we can see the shallow state C0/C1 duration are reduced and the time
> has been moved to deepest state, so the deepest state C2 duration can
> have improvement for ~9s to ~21s. for cases (dutycycle=30%/40%) though
> we can see the deepest state durations are parity between with and
> without patch series, but it has a minor improvement for C1 state
> duration by stealing C0 state duration.
>
> Some notations are used in the table:
>
> state: C0: WFI; C1: CPU OFF; C2: Cluster OFF
>
> All testing cases have single task with 5ms period:
>
> Without patches With patches Difference
> ----------------------- ----------------------- --------------------------
> Duty cycle C0 C1 C2 C0 C1 C2 C0 C1 C2
> 1% 2.397 16.528 471.905 0.916 2.688 487.328 -1.481 -13.840 +15.422
> 3% 3.957 20.541 464.434 1.510 2.398 485.914 -2.447 -18.143 +21.480
> 5% 2.866 8.609 474.777 1.166 2.250 483.983 -1.699 -6.359 +9.205
> 10% 2.893 28.753 453.277 1.147 14.134 469.190 -1.745 -14.618 +15.913
> 20% 7.620 41.086 431.735 1.595 35.055 442.482 -6.024 -6.030 +10.747
> 30% 4.394 38.328 431.442 1.964 40.857 430.973 -2.430 +2.529 -0.468
> 40% 7.390 29.415 430.914 1.789 34.832 431.588 -5.600 +5.417 -0.673
>
>
> P.s. for the testing, applied Rafael's patch 'cpuidle: menu: Handle
> stopped tick more aggressively' [1] to avoid select unexpected shallow
> state after tick has been stopped.
>
> [1] https://lkml.org/lkml/2018/8/10/259
>
> Leo Yan (5):
> cpuidle: menu: Clean up variables usage in menu_select()
> cpuidle: menu: Record tick delta value in struct menu_device
> cpuidle: menu: Provide menu_decide_stopping_tick()
> cpuidle: menu: Don't stay in shallow state for a long time
> cpuidle: menu: Change to compare prediction with tick delta
>
> drivers/cpuidle/governors/menu.c | 104 ++++++++++++++++++++++++++++-----------
> 1 file changed, 76 insertions(+), 28 deletions(-)
>
>
Overall, I don't like this series, sorry about that.
The majority of changes in it are code reorganization, quite questionable
in a couple of cases, and a similar goal can be achieved with a very simple
patch that I'm going to post shortly.
Thanks,
Rafael
prev parent reply other threads:[~2018-08-21 8:40 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-12 16:09 [PATCH v1 0/5] Improvement stopping tick decision making in 'menu' idle governor Leo Yan
2018-08-12 16:09 ` [PATCH v1 1/5] cpuidle: menu: Clean up variables usage in menu_select() Leo Yan
2018-08-21 8:32 ` Rafael J. Wysocki
2018-08-12 16:09 ` [PATCH v1 2/5] cpuidle: menu: Record tick delta value in struct menu_device Leo Yan
2018-08-21 8:34 ` Rafael J. Wysocki
2018-08-12 16:09 ` [PATCH v1 3/5] cpuidle: menu: Provide menu_decide_stopping_tick() Leo Yan
2018-08-12 16:09 ` [PATCH v1 4/5] cpuidle: menu: Don't stay in shallow state for a long time Leo Yan
2018-08-21 8:35 ` Rafael J. Wysocki
2018-08-12 16:09 ` [PATCH v1 5/5] cpuidle: menu: Change to compare prediction with tick delta Leo Yan
2018-08-21 8:37 ` Rafael J. Wysocki [this message]
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=1980448.5HStMnL72j@aspire.rjw.lan \
--to=rjw@rjwysocki.net \
--cc=daniel.lezcano@linaro.org \
--cc=leo.yan@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rafael.j.wysocki@intel.com \
--cc=ramesh.thomas@intel.com \
--cc=vincent.guittot@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).