mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Zhang Rui <rui.zhang@intel.com>
To: rjw@rjwysocki.net, daniel.lezcano@linaro.org
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [RFC PATCH 1/3] cpuidle: ladder: Fix bogus comparison between s64 and u64
Date: Sun,  6 Nov 2022 01:42:23 +0800	[thread overview]
Message-ID: <20221105174225.28673-1-rui.zhang@intel.com> (raw)

ladder_device_state.threshold.promotion_time_ns/demotion_time_ns
are u64 type.

In ladder_select_state(), variable 'last_residency', as calculated by

last_residency = dev->last_residency_ns - drv->states[last_idx].exit_latency_ns

are s64 type, and it can be negative value.

When this happens, comparing between 'last_residency' and
'promotion_time_ns/demotion_time_ns' become bogus. As a result, the
ladder governor promotes or stays with current state errornously.

          <idle>-0       [001] d..1.   151.893396: ladder_select_state: last_idx 7, last_residency -373033
          <idle>-0       [001] d..1.   151.893399: ladder_select_state:    dev->last_residency_ns 106967, drv->states[last_idx].exit_latency_ns 480000
          <idle>-0       [001] d..1.   151.893402: ladder_select_state:    promote, last_state->threshold.promotion_time_ns 480000
          <idle>-0       [001] d..1.   151.893404: ladder_select_state:    ---> new state 7
          <idle>-0       [001] d..1.   151.893465: ladder_select_state: last_idx 7, last_residency -463800
          <idle>-0       [001] d..1.   151.893467: ladder_select_state:    dev->last_residency_ns 16200, drv->states[last_idx].exit_latency_ns 480000
          <idle>-0       [001] d..1.   151.893468: ladder_select_state:    promote, last_state->threshold.promotion_time_ns 480000
          <idle>-0       [001] dn.1.   151.893470: ladder_select_state:    ---> new state 8

Given that promotion_time_ns/demotion_time_ns are initialized with
cpuidle_state.exit_latency_ns, which is s64 type, and they are used to
compare with 'last_residency', which is also s64 type, there is no
reason to use u64 for promotion_time_ns/demotion_time_ns.

With this patch,
          <idle>-0       [001] d..1.   523.578531: ladder_select_state: last_idx 8, last_residency -879453
          <idle>-0       [001] d..1.   523.578531: ladder_select_state:    dev->last_residency_ns 10547, drv->states[last_idx].exit_latency_ns 890000
          <idle>-0       [001] d..1.   523.578532: ladder_select_state:    demote , last_state->threshold.demotion_time_ns 890000
          <idle>-0       [001] d..1.   523.578532: ladder_select_state:    ---> new state 7
          <idle>-0       [001] d..1.   523.580220: ladder_select_state: last_idx 7, last_residency -169629
          <idle>-0       [001] d..1.   523.580221: ladder_select_state:    dev->last_residency_ns 310371, drv->states[last_idx].exit_latency_ns 480000
          <idle>-0       [001] d..1.   523.580221: ladder_select_state:    demote , last_state->threshold.demotion_time_ns 480000
          <idle>-0       [001] d..1.   523.580222: ladder_select_state:    ---> new state 6

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/cpuidle/governors/ladder.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
index 8e9058c4ea63..fb61118aef37 100644
--- a/drivers/cpuidle/governors/ladder.c
+++ b/drivers/cpuidle/governors/ladder.c
@@ -27,8 +27,8 @@ struct ladder_device_state {
 	struct {
 		u32 promotion_count;
 		u32 demotion_count;
-		u64 promotion_time_ns;
-		u64 demotion_time_ns;
+		s64 promotion_time_ns;
+		s64 demotion_time_ns;
 	} threshold;
 	struct {
 		int promotion_count;
-- 
2.25.1


             reply	other threads:[~2022-11-05 17:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-05 17:42 Zhang Rui [this message]
2022-11-05 17:42 ` [RFC PATCH 2/3] cpuidle: ladder: Tune promotion/demotion threshold Zhang Rui
2022-11-23 17:50   ` Rafael J. Wysocki
2022-11-23 23:53     ` Doug Smythies
2022-11-25  6:38     ` Zhang Rui
2022-11-25 13:36       ` Rafael J. Wysocki
2022-11-27  3:18         ` Zhang Rui
2022-11-05 17:42 ` [RFC PATCH 3/3] cpuidle: ladder: Fix handling for disabled states Zhang Rui
2022-11-23 17:56   ` Rafael J. Wysocki
2022-11-23 17:37 ` [RFC PATCH 1/3] cpuidle: ladder: Fix bogus comparison between s64 and u64 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=20221105174225.28673-1-rui.zhang@intel.com \
    --to=rui.zhang@intel.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.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

all inboxes | Powered by JetHome®