linux-kernel - Re: [PATCH] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0h8MyqEgC8Hf1_s4=kCj2W3Y0TWdVnbhwAiVLqED7WPow@mail.gmail.com>
Date: Thu, 20 Feb 2025 20:40:09 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Yu-Che Cheng <giver@...omium.org>
Cc: Lukasz Luba <lukasz.luba@....com>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	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] thermal: gov_power_allocator: Update total_weight on bind
 and cdev updates

On Wed, Feb 19, 2025 at 8:07 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.
>
> Update total_weight when bind and cdev updates to ensure total_weight is
> correct.
>
> Fixes: a3cd6db4cc2e ("thermal: gov_power_allocator: Support new update callback of weights")
> Signed-off-by: Yu-Che Cheng <giver@...omium.org>
> ---
>  drivers/thermal/gov_power_allocator.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index 3b644de3292e..600cb0e367c3 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -656,11 +656,10 @@ 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;

This is basically fine, but I would add a new function, say
power_allocator_update_weight(), for updating the weight.

>         case THERMAL_INSTANCE_WEIGHT_CHANGED:

And I'd call it from here and from power_allocator_bind() below.

>                 params->total_weight = 0;
>                 list_for_each_entry(instance, &td->thermal_instances, trip_node)
> @@ -731,6 +730,8 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>
>         tz->governor_data = params;
>
> +       power_allocator_update_tz(tz, THERMAL_INSTANCE_WEIGHT_CHANGED);

Because this is kind of confusing (although it will work AFAICS).

> +
>         return 0;
>
>  free_params:
>
> ---
> base-commit: 2408a807bfc3f738850ef5ad5e3fd59d66168996
> change-id: 20250218-fix-power-allocator-weight-5dac6d4b5797
>
> Best regards,
> --
> Yu-Che Cheng <giver@...omium.org>
>