From: "Zhou Ti (x2019cwm)" <x2019cwm@stfx.ca>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
Linux PM <linux-pm@vger.kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Frederic Weisbecker <frederic@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v1 5/5] cpuidle: menu: Take negative "sleep length" values into account
Date: Tue, 30 Mar 2021 01:59:40 +0000 [thread overview]
Message-ID: <YTBPR01MB3262DB49961DBCCE96C51AB8C47D9@YTBPR01MB3262.CANPRD01.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <7927358.NyiUUSuA9g@kreacher>
On Mon 2021-03-29 14:37 Rafael J. Wysocki wrote:
> Make the menu governor check the tick_nohz_get_next_hrtimer()
> return value so as to avoid dealing with negative "sleep length"
> values and make it use that value directly when the tick is stopped.
>
> While at it, rename local variable delta_next in menu_select() to
> delta_tick which better reflects its purpose.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpuidle/governors/menu.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> Index: linux-pm/drivers/cpuidle/governors/menu.c
> ===================================================================
> --- linux-pm.orig/drivers/cpuidle/governors/menu.c
> +++ linux-pm/drivers/cpuidle/governors/menu.c
> @@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_dr
> u64 predicted_ns;
> u64 interactivity_req;
> unsigned long nr_iowaiters;
> - ktime_t delta_next;
> + ktime_t delta, delta_tick;
> int i, idx;
>
> if (data->needs_update) {
> @@ -280,7 +280,12 @@ static int menu_select(struct cpuidle_dr
> }
>
> /* determine the expected residency time, round up */
> - data->next_timer_ns = tick_nohz_get_sleep_length(&delta_next);
> + delta = tick_nohz_get_sleep_length(&delta_tick);
> + if (unlikely(delta < 0)) {
> + delta = 0;
> + delta_tick = 0;
> + }
> + data->next_timer_ns = delta;
>
> nr_iowaiters = nr_iowait_cpu(dev->cpu);
> data->bucket = which_bucket(data->next_timer_ns, nr_iowaiters);
> @@ -318,7 +323,7 @@ static int menu_select(struct cpuidle_dr
> * state selection.
> */
> if (predicted_ns < TICK_NSEC)
> - predicted_ns = delta_next;
> + predicted_ns = data->next_timer_ns;
> } else {
> /*
> * Use the performance multiplier and the user-configurable
> @@ -377,7 +382,7 @@ static int menu_select(struct cpuidle_dr
> * stuck in the shallow one for too long.
> */
> if (drv->states[idx].target_residency_ns < TICK_NSEC &&
> - s->target_residency_ns <= delta_next)
> + s->target_residency_ns <= delta_tick)
> idx = i;
>
> return idx;
> @@ -399,7 +404,7 @@ static int menu_select(struct cpuidle_dr
> predicted_ns < TICK_NSEC) && !tick_nohz_tick_stopped()) {
> *stop_tick = false;
>
> - if (idx > 0 && drv->states[idx].target_residency_ns > delta_next) {
> + if (idx > 0 && drv->states[idx].target_residency_ns > delta_tick) {
> /*
> * The tick is not going to be stopped and the target
> * residency of the state to be returned is not within
> @@ -411,7 +416,7 @@ static int menu_select(struct cpuidle_dr
> continue;
>
> idx = i;
> - if (drv->states[i].target_residency_ns <= delta_next)
> + if (drv->states[i].target_residency_ns <= delta_tick)
> break;
> }
> }
How about this.
I think it's possible to avoid the new variable delta.
---
--- linux-pm/drivers/cpuidle/governors/menu.c.orig 2021-03-29 22:44:02.316971970 -0300
+++ linux-pm/drivers/cpuidle/governors/menu.c 2021-03-29 22:51:15.804377168 -0300
@@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_dr
u64 predicted_ns;
u64 interactivity_req;
unsigned long nr_iowaiters;
- ktime_t delta_next;
+ ktime_t delta_tick;
int i, idx;
if (data->needs_update) {
@@ -280,7 +280,12 @@ static int menu_select(struct cpuidle_dr
}
/* determine the expected residency time, round up */
- data->next_timer_ns = tick_nohz_get_sleep_length(&delta_next);
+ data->next_timer_ns = tick_nohz_get_sleep_length(&delta_tick);
+
+ if (unlikely(data->next_timer_ns >> 63)) {
+ data->next_timer_ns = 0;
+ delta_tick = 0;
+ }
nr_iowaiters = nr_iowait_cpu(dev->cpu);
data->bucket = which_bucket(data->next_timer_ns, nr_iowaiters);
@@ -318,7 +323,7 @@ static int menu_select(struct cpuidle_dr
* state selection.
*/
if (predicted_ns < TICK_NSEC)
- predicted_ns = delta_next;
+ predicted_ns = data->next_timer_ns;
} else {
/*
* Use the performance multiplier and the user-configurable
@@ -377,7 +382,7 @@ static int menu_select(struct cpuidle_dr
* stuck in the shallow one for too long.
*/
if (drv->states[idx].target_residency_ns < TICK_NSEC &&
- s->target_residency_ns <= delta_next)
+ s->target_residency_ns <= delta_tick)
idx = i;
return idx;
@@ -399,7 +404,7 @@ static int menu_select(struct cpuidle_dr
predicted_ns < TICK_NSEC) && !tick_nohz_tick_stopped()) {
*stop_tick = false;
- if (idx > 0 && drv->states[idx].target_residency_ns > delta_next) {
+ if (idx > 0 && drv->states[idx].target_residency_ns > delta_tick) {
/*
* The tick is not going to be stopped and the target
* residency of the state to be returned is not within
@@ -411,7 +416,7 @@ static int menu_select(struct cpuidle_dr
continue;
idx = i;
- if (drv->states[i].target_residency_ns <= delta_next)
+ if (drv->states[i].target_residency_ns <= delta_tick)
break;
}
}
next prev parent reply other threads:[~2021-03-30 2:00 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-29 18:12 [PATCH v1 0/5] cpuidle: Take possible negative "sleep length" values into account Rafael J. Wysocki
2021-03-29 18:13 ` [PATCH v1 1/5] tick/nohz: Improve tick_nohz_get_next_hrtimer() kerneldoc Rafael J. Wysocki
2021-03-29 18:15 ` [PATCH v1 2/5] cpuidle: Use s64 as exit_latency_ns and target_residency_ns data type Rafael J. Wysocki
2021-03-29 18:19 ` [PATCH v1 3/5] cpuidle: teo: Adjust handling of very short idle times Rafael J. Wysocki
2021-03-29 18:21 ` [PATCH v1 4/5] cpuidle: teo: Take negative "sleep length" values into account Rafael J. Wysocki
2021-03-29 18:37 ` [PATCH v1 5/5] cpuidle: menu: " Rafael J. Wysocki
2021-03-30 1:59 ` Zhou Ti (x2019cwm) [this message]
2021-03-30 14:47 ` Rafael J. Wysocki
2021-04-07 17:24 ` [PATCH v1 0/5] cpuidle: Take possible " 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=YTBPR01MB3262DB49961DBCCE96C51AB8C47D9@YTBPR01MB3262.CANPRD01.PROD.OUTLOOK.COM \
--to=x2019cwm@stfx.ca \
--cc=frederic@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=tglx@linutronix.de \
/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).