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: <0c02c5a3-9eea-495e-aa39-3aac862b5427@arm.com>
Date: Mon, 21 Oct 2024 23:11:23 +0100
From: Lukasz Luba <lukasz.luba@....com>
To: ZhengShaobo <zhengshaobo1@...omi.com>
Cc: zhuzhangwei <chuci@...omi.com>, Zhang Rui <rui.zhang@...el.com>,
 Daniel Lezcano <daniel.lezcano@...aro.org>,
 "Rafael J . Wysocki" <rafael@...nel.org>,
 dingchongchong <dingchongchong@...omi.com>, chendejun
 <chendejun@...omi.com>, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] thermal: gov_power_allocator: Granted power set to max
 when nobody request power

Hello ZhengShaobo,

On 10/21/24 13:11, ZhengShaobo wrote:
> From: zhengshaobo1 <zhengshaobo1@...omi.com>
> 
> When total_req_power is 0, divvy_up_power() will set granted_power to 0,
> and cdev will be limited to the lowest performance. If our polling delay
> is set to 200ms, it means that cdev cannot perform better within 200ms
> even if cdev has a sudden load. This will affect the performance of cdev
> and is not as expected.
> 
> For this reason, if nobody requests power, then set the granted power to
> the max_power.
> 
> Signed-off-by: zhengshaobo1 <zhengshaobo1@...omi.com>
> ---
>   drivers/thermal/gov_power_allocator.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index 1b2345a697c5..4301516c0938 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -356,11 +356,19 @@ static void divvy_up_power(struct power_actor *power, int num_actors,
>   	u32 extra_power = 0;
>   	int i;
>   
> -	/*
> -	 * Prevent division by 0 if none of the actors request power.
> -	 */
> -	if (!total_req_power)
> -		total_req_power = 1;
> +	if (!total_req_power) {
> +		/*
> +		 * Nobody requested anything, just give everybody
> +		 * the maximum power
> +		 */
> +		for (i = 0; i < num_actors; i++) {
> +			struct power_actor *pa = &power[i];
> +
> +			pa->granted_power = pa->max_power;
> +		}
> +
> +		return;
> +	}
>   
>   	for (i = 0; i < num_actors; i++) {
>   		struct power_actor *pa = &power[i];

Thank you for the patch. Good catch of that corner case.

If there is no load on those devices, then the temperature should
be low, lower than the 1st trip point. In that case we should
reset the PID and call allow_maximum_power()...

Although, what if the temperature is higher, e.g. due to
ambient temperature and no load on the devices. Then we
hit that corner case and your code would help.

LGTM,

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ