linux-kernel - Re: [PATCH v2] thermal: gov_power_allocator: Update total

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJZ5v0jgRXoqx4g=iHSUteXs99ageQmcQDQfjp6eSqnVo9Ty8Q@mail.gmail.com>
Date: Tue, 25 Feb 2025 12:35:56 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Yu-Che Cheng <giver@...omium.org>, Lukasz Luba <lukasz.luba@....com>
Cc: Daniel Lezcano <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>, 
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Chen-Yu Tsai <wenst@...omium.org>
Subject: Re: [PATCH v2] thermal: gov_power_allocator: Update total_weight on
 bind and cdev updates

On Sat, Feb 22, 2025 at 4:20 AM Yu-Che Cheng <giver@...omium.org> wrote:
>
> params->total_weight is not initialized during bind and not updated when
> the bound cdev changes. The cooling device weight will not be used due
> to the uninitialized total_weight, until we trigger an update via sysfs.
>
> The bound cdev update will be triggered during thermal zone registration,
> where each cooling device will be bound to the thermal zone one by one.
>
> The power_allocator_bind can be called without additional cdev update
> when manually changing the policy of a thermal zone via sysfs.
>
> Add a new function to handle weight update logic, including updating
> total_weight, and call it when bind, weight changes, and cdev updates to
> ensure total_weight is always correct.
>
> Fixes: a3cd6db4cc2e ("thermal: gov_power_allocator: Support new update callback of weights")
> Signed-off-by: Yu-Che Cheng <giver@...omium.org>

Applied as 6.14-rc material with some minor edits in the changelog.

Lukasz, if you have any objections, please let me know.

Thanks!

> ---
> Changes in v2:
> - Move the total_weight update to a new function for clarity.
> - Found v1 may cause crash when there are thermal zones without any
>   active or passive trip points.
>   Check trip_max before accessing its trip_desc.
> - Link to v1: https://lore.kernel.org/r/20250219-fix-power-allocator-weight-v1-1-79b1a99e94eb@chromium.org
> ---
>  drivers/thermal/gov_power_allocator.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index 3b644de3292e..126452fb470e 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -641,6 +641,22 @@ static int allocate_actors_buffer(struct power_allocator_params *params,
>         return ret;
>  }
>
> +static void power_allocator_update_weight(struct power_allocator_params *params)
> +{
> +       const struct thermal_trip_desc *td;
> +       struct thermal_instance *instance;
> +
> +       if (!params->trip_max)
> +               return;
> +
> +       td = trip_to_trip_desc(params->trip_max);
> +
> +       params->total_weight = 0;
> +       list_for_each_entry(instance, &td->thermal_instances, trip_node)
> +               if (power_actor_is_valid(instance))
> +                       params->total_weight += instance->weight;
> +}
> +
>  static void power_allocator_update_tz(struct thermal_zone_device *tz,
>                                       enum thermal_notify_event reason)
>  {
> @@ -656,16 +672,12 @@ static void power_allocator_update_tz(struct thermal_zone_device *tz,
>                         if (power_actor_is_valid(instance))
>                                 num_actors++;
>
> -               if (num_actors == params->num_actors)
> -                       return;
> +               if (num_actors != params->num_actors)
> +                       allocate_actors_buffer(params, num_actors);
>
> -               allocate_actors_buffer(params, num_actors);
> -               break;
> +               fallthrough;
>         case THERMAL_INSTANCE_WEIGHT_CHANGED:
> -               params->total_weight = 0;
> -               list_for_each_entry(instance, &td->thermal_instances, trip_node)
> -                       if (power_actor_is_valid(instance))
> -                               params->total_weight += instance->weight;
> +               power_allocator_update_weight(params);
>                 break;
>         default:
>                 break;
> @@ -731,6 +743,8 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>
>         tz->governor_data = params;
>
> +       power_allocator_update_weight(params);
> +
>         return 0;
>
>  free_params:
>
> ---
> base-commit: 2408a807bfc3f738850ef5ad5e3fd59d66168996
> change-id: 20250218-fix-power-allocator-weight-5dac6d4b5797
>
> Best regards,
> --
> Yu-Che Cheng <giver@...omium.org>
>