The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Christian Loehle <christian.loehle@arm.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	vincent.guittot@linaro.org, qyousef@layalina.io,
	peterz@infradead.org, daniel.lezcano@linaro.org,
	ulf.hansson@linaro.org, anna-maria@linutronix.de,
	dsmythies@telus.net, kajetan.puchalski@arm.com,
	lukasz.luba@arm.com, dietmar.eggemann@arm.com
Subject: [PATCHv4 3/3] cpuidle: teo: Don't count non-existent intercepts
Date: Sat, 29 Jun 2024 09:22:48 +0100	[thread overview]
Message-ID: <c40acf72-010f-4a8b-80e4-33f133ba266b@arm.com> (raw)
In-Reply-To: <CAJZ5v0jaEt2yo9OvYqpzfcbPtAvTk63tKXjm6QCi7zeKuU2SUA@mail.gmail.com>

When bailing out early, teo will not query the sleep length anymore
since commit 6da8f9ba5a87 ("cpuidle: teo:
Skip tick_nohz_get_sleep_length() call in some cases") with an
expected sleep_length_ns value of KTIME_MAX.
This lead to state0 accumulating lots of 'intercepts' because
the actually measured sleep length was < KTIME_MAX, so query the sleep
length instead for teo to recognize if it still is in an
intercept-likely scenario without alternating between the two modes.

Fundamentally we can only do one of the two:
1. Skip sleep_length_ns query when we think intercept is likely
2. Have accurate data if sleep_length_ns is actually intercepted when
we believe it is currently intercepted.

Previously teo did the former while this patch chooses the latter as
the additional time it takes to query the sleep length was found to be
negligible and the variants of option 1 (count all unknowns as misses
or count all unknown as hits) had significant regressions (as misses
had lots of too shallow idle state selections and as hits had terrible
performance in intercept-heavy workloads).

Fixes: 6da8f9ba5a87 ("cpuidle: teo: Skip tick_nohz_get_sleep_length() call in some cases")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
v4: Skip constraint check if intercept logic selects state0

 drivers/cpuidle/governors/teo.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c
index 200a3598cbcf..6dc44197a80e 100644
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -287,6 +287,7 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
 	unsigned int hit_sum = 0;
 	int constraint_idx = 0;
 	int idx0 = 0, idx = -1;
+	int prev_intercept_idx;
 	s64 duration_ns;
 	int i;
 
@@ -364,6 +365,7 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
 	 * all of the deeper states a shallower idle state is likely to be a
 	 * better choice.
 	 */
+	prev_intercept_idx = idx;
 	if (2 * idx_intercept_sum > cpu_data->total - idx_hit_sum) {
 		int first_suitable_idx = idx;
 
@@ -415,6 +417,15 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
 			first_suitable_idx = i;
 		}
 	}
+	if (!idx && prev_intercept_idx) {
+		/*
+		 * We have to query the sleep length here otherwise we don't
+		 * know after wakeup if our guess was correct.
+		 */
+		duration_ns = tick_nohz_get_sleep_length(&delta_tick);
+		cpu_data->sleep_length_ns = duration_ns;
+		goto out_tick;
+	}
 
 	/*
 	 * If there is a latency constraint, it may be necessary to select an
-- 
2.34.1



  reply	other threads:[~2024-06-29  8:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-28  9:59 [PATCHv3 0/3] cpuidle: teo: Fixing utilization and intercept logic Christian Loehle
2024-06-28  9:59 ` [PATCH 1/3] Revert: "cpuidle: teo: Introduce util-awareness" Christian Loehle
2024-06-28  9:59 ` [PATCH 2/3] cpuidle: teo: Remove recent intercepts metric Christian Loehle
2024-06-28  9:59 ` [PATCH 3/3] cpuidle: teo: Don't count non-existent intercepts Christian Loehle
2024-06-28 18:48   ` Rafael J. Wysocki
2024-06-29  8:22     ` Christian Loehle [this message]
2024-07-01 16:59       ` [PATCHv4 " Rafael J. Wysocki
2024-06-28 19:06 ` [PATCHv3 0/3] cpuidle: teo: Fixing utilization and intercept logic Rafael J. Wysocki
2024-06-29  8:23   ` Christian Loehle
2024-08-05 14:53   ` Christian Loehle
2024-08-05 14:58     ` [PATCH 6.1.y] cpuidle: teo: Remove recent intercepts metric Christian Loehle
2024-08-12 12:42       ` Greg KH
2024-08-13 13:18         ` Christian Loehle
2024-08-14 11:40           ` Greg KH

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=c40acf72-010f-4a8b-80e4-33f133ba266b@arm.com \
    --to=christian.loehle@arm.com \
    --cc=anna-maria@linutronix.de \
    --cc=daniel.lezcano@linaro.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=dsmythies@telus.net \
    --cc=kajetan.puchalski@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lukasz.luba@arm.com \
    --cc=peterz@infradead.org \
    --cc=qyousef@layalina.io \
    --cc=rafael@kernel.org \
    --cc=ulf.hansson@linaro.org \
    --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