linux-kernel - Re: [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding without trip points

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]

Date: Wed, 3 Apr 2024 13:48:40 +0100
From: Lukasz Luba <lukasz.luba@....com>
To: Nikita Travkin <nikitos.tr@...il.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
 "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
 Daniel Lezcano <daniel.lezcano@...aro.org>, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org, Zhang Rui <rui.zhang@...el.com>
Subject: Re: [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding
 without trip points



On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> From: Nikita Travkin <nikita@...n.ru>
> 
> IPA probe function was recently refactored to perform extra error checks
> and make sure the thermal zone has trip points necessary for the IPA
> operation. With this change, if a thermal zone is probed such that it
> has no trip points that IPA can use, IPA will fail and the TZ won't be
> created. This is the case if a platform defines a TZ without cooling
> devices and only with "hot"/"critical" trip points, often found on some
> Qualcomm devices [1].
> 
> Documentation across IPA code (notably get_governor_trips() kerneldoc)
> suggests that IPA is supposed to handle such TZ even if it won't
> actually do anything.
> 
> This commit partially reverts the previous change to allow IPA to bind
> to such "empty" thermal zones.
> 
> [1] arch/arm64/boot/dts/qcom/sc7180.dtsi#n4776
> 
> Fixes: e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier")
> Signed-off-by: Nikita Travkin <nikita@...n.ru>
> ---
>   drivers/thermal/gov_power_allocator.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index ec637071ef1f..e25e48d76aa7 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -679,11 +679,6 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>   		return -ENOMEM;
>   
>   	get_governor_trips(tz, params);
> -	if (!params->trip_max) {
> -		dev_warn(&tz->device, "power_allocator: missing trip_max\n");
> -		kfree(params);
> -		return -EINVAL;
> -	}
>   
>   	ret = check_power_actors(tz, params);
>   	if (ret < 0) {
> @@ -714,9 +709,10 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>   	else
>   		params->sustainable_power = tz->tzp->sustainable_power;
>   
> -	estimate_pid_constants(tz, tz->tzp->sustainable_power,
> -			       params->trip_switch_on,
> -			       params->trip_max->temperature);
> +	if (params->trip_max)
> +		estimate_pid_constants(tz, tz->tzp->sustainable_power,
> +				       params->trip_switch_on,
> +				       params->trip_max->temperature);
>   
>   	reset_pid_controller(params);
>   
> 

LGTM

Reviewed-by: Lukasz Luba <lukasz.luba@....com>