| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20221105174225.28673-1-rui.zhang@intel.com>
Date: Sun, 6 Nov 2022 01:42:23 +0800
From: Zhang Rui <rui.zhang@...el.com>
To: rjw@...ysocki.net, daniel.lezcano@...aro.org
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [RFC PATCH 1/3] cpuidle: ladder: Fix bogus comparison between s64 and u64
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@...el.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
Powered by blists - more mailing lists