| 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: <CAJZ5v0iG+vwuh1OgR1t09kpP=uVBMnoWB6UAA=d6eiq_g_dReQ@mail.gmail.com>
Date: Wed, 18 Dec 2024 14:53:34 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: rafael@...nel.org, quic_manafm@...cinc.com,
Zhang Rui <rui.zhang@...el.com>, Lukasz Luba <lukasz.luba@....com>,
"open list:THERMAL" <linux-pm@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] thermal/thresholds: Fix boundaries and detection routine
On Mon, Dec 16, 2024 at 10:27 PM Daniel Lezcano
<daniel.lezcano@...aro.org> wrote:
>
> The current implementation does not work if the thermal zone is
> interrupt driven only.
>
> The boundaries are not correctly checked and computed as it happens
> only when the temperature is increasing or decreasing.
>
> The problem arises because the routine to detect when we cross a
> threshold is correlated with the computation of the boundaries. We
> assume we have to recompute the boundaries when a threshold is crossed
> but actually we should do that even if the it is not the case.
>
> Mixing the boundaries computation and the threshold detection for the
> sake of optimizing the routine is much more complex as it appears
> intuitively and prone to errors.
>
> This fix separates the boundaries computation and the threshold
> crossing detection into different routines. The result is a code much
> more simple to understand, thus easier to maintain.
>
> The drawback is we browse the thresholds list several time but we can
> consider that as neglictible because that happens when the temperature
> is updated. There are certainly some aeras to improve in the
> temperature update routine but it would be not adequate as this change
> aims to fix the thresholds for v6.13.
>
> Fixes: 445936f9e258 ("thermal: core: Add user thresholds support")
> Tested-by: Daniel Lezcano <daniel.lezcano@...aro.org> # rock5b, Lenovo x13s
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
> -V2: Fix the conditions for the temperature crossing the way down
> ---
> drivers/thermal/thermal_thresholds.c | 68 +++++++++++++++-------------
> 1 file changed, 36 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/thermal/thermal_thresholds.c b/drivers/thermal/thermal_thresholds.c
> index d9b2a0bb44fc..38f5fd0e8930 100644
> --- a/drivers/thermal/thermal_thresholds.c
> +++ b/drivers/thermal/thermal_thresholds.c
> @@ -69,58 +69,60 @@ static struct user_threshold *__thermal_thresholds_find(const struct list_head *
> return NULL;
> }
>
> -static bool __thermal_threshold_is_crossed(struct user_threshold *threshold, int temperature,
> - int last_temperature, int direction,
> - int *low, int *high)
> +static bool thermal_thresholds_handle_raising(struct list_head *thresholds, int temperature,
> + int last_temperature)
> {
> + struct user_threshold *t;
>
> - if (temperature >= threshold->temperature) {
> - if (threshold->temperature > *low &&
> - THERMAL_THRESHOLD_WAY_DOWN & threshold->direction)
> - *low = threshold->temperature;
> + list_for_each_entry(t, thresholds, list_node) {
>
> - if (last_temperature < threshold->temperature &&
> - threshold->direction & direction)
> - return true;
> - } else {
> - if (threshold->temperature < *high && THERMAL_THRESHOLD_WAY_UP
> - & threshold->direction)
> - *high = threshold->temperature;
> + if (!(t->direction & THERMAL_THRESHOLD_WAY_UP))
> + continue;
>
> - if (last_temperature >= threshold->temperature &&
> - threshold->direction & direction)
> + if (temperature >= t->temperature &&
> + last_temperature < t->temperature)
> return true;
> }
>
> return false;
> }
>
> -static bool thermal_thresholds_handle_raising(struct list_head *thresholds, int temperature,
> - int last_temperature, int *low, int *high)
> +static bool thermal_thresholds_handle_dropping(struct list_head *thresholds, int temperature,
> + int last_temperature)
> {
> struct user_threshold *t;
>
> - list_for_each_entry(t, thresholds, list_node) {
> - if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
> - THERMAL_THRESHOLD_WAY_UP, low, high))
> + list_for_each_entry_reverse(t, thresholds, list_node) {
> +
> + if (!(t->direction & THERMAL_THRESHOLD_WAY_DOWN))
> + continue;
> +
> + if (temperature <= t->temperature &&
> + last_temperature > t->temperature)
> return true;
> }
>
> return false;
> }
>
> -static bool thermal_thresholds_handle_dropping(struct list_head *thresholds, int temperature,
> - int last_temperature, int *low, int *high)
> +static void thermal_threshold_find_boundaries(struct list_head *thresholds, int temperature,
> + int *low, int *high)
> {
> struct user_threshold *t;
>
> - list_for_each_entry_reverse(t, thresholds, list_node) {
> - if (__thermal_threshold_is_crossed(t, temperature, last_temperature,
> - THERMAL_THRESHOLD_WAY_DOWN, low, high))
> - return true;
> + list_for_each_entry(t, thresholds, list_node) {
> + if (temperature < t->temperature &&
> + (t->direction & THERMAL_THRESHOLD_WAY_UP) &&
> + *high > t->temperature)
> + *high = t->temperature;
> }
>
> - return false;
> + list_for_each_entry_reverse(t, thresholds, list_node) {
> + if (temperature > t->temperature &&
> + (t->direction & THERMAL_THRESHOLD_WAY_DOWN) &&
> + *low < t->temperature)
> + *low = t->temperature;
> + }
> }
>
> void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *high)
> @@ -132,6 +134,8 @@ void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *hi
>
> lockdep_assert_held(&tz->lock);
>
> + thermal_threshold_find_boundaries(thresholds, temperature, low, high);
> +
> /*
> * We need a second update in order to detect a threshold being crossed
> */
> @@ -151,12 +155,12 @@ void thermal_thresholds_handle(struct thermal_zone_device *tz, int *low, int *hi
> * - decreased : thresholds are crossed the way down
> */
> if (temperature > last_temperature) {
> - if (thermal_thresholds_handle_raising(thresholds, temperature,
> - last_temperature, low, high))
> + if (thermal_thresholds_handle_raising(thresholds,
> + temperature, last_temperature))
> thermal_notify_threshold_up(tz);
> } else {
> - if (thermal_thresholds_handle_dropping(thresholds, temperature,
> - last_temperature, low, high))
> + if (thermal_thresholds_handle_dropping(thresholds,
> + temperature, last_temperature))
> thermal_notify_threshold_down(tz);
> }
> }
> --
Applied as 6.13-rc material, thanks!
Powered by blists - more mailing lists