linux-kernel - Re: [PATCH] cpuidle: menu: Optimize bucket assignment when next_timer_ns equals KTIME

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: <1988a6b2-4e25-42b9-8197-5599760c6755@quicinc.com>
Date: Thu, 3 Apr 2025 18:01:07 +0800
From: Zhongqiu Han <quic_zhonhan@...cinc.com>
To: Christian Loehle <christian.loehle@....com>, <rafael@...nel.org>,
        <daniel.lezcano@...aro.org>
CC: <linux-pm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cpuidle: menu: Optimize bucket assignment when
 next_timer_ns equals KTIME_MAX

On 4/3/2025 5:34 PM, Christian Loehle wrote:
> On 4/3/25 10:28, Zhongqiu Han wrote:
>> Directly assign the last bucket value instead of calling which_bucket()
>> when next_timer_ns equals KTIME_MAX, the largest possible value that
>> always falls into the last bucket. This avoids unnecessary calculations
>> and enhances performance.
>>
>> Signed-off-by: Zhongqiu Han <quic_zhonhan@...cinc.com>
> 
> Reviewed-by: Christian Loehle <christian.loehle@....com>
> 
>> ---
>>   drivers/cpuidle/governors/menu.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
>> index 39aa0aea61c6..8fc7fbed0052 100644
>> --- a/drivers/cpuidle/governors/menu.c
>> +++ b/drivers/cpuidle/governors/menu.c
>> @@ -255,7 +255,12 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
>>   		 */
>>   		data->next_timer_ns = KTIME_MAX;
>>   		delta_tick = TICK_NSEC / 2;
>> -		data->bucket = which_bucket(KTIME_MAX);
>> +		/*
>> +		 * Assign the last bucket value directly instead of calling
>> +		 * which_bucket(), since KTIME_MAX is the largest possible
>> +		 * value that always falls into the last bucket.
>> +		 */
> 
> comment almost seems overkill.
> 
>> +		data->bucket = BUCKETS - 1;
>>   	}
>>   
>>   	if (unlikely(drv->state_count <= 1 || latency_req == 0) ||
> 
Thanks Christian for the review~

Actually I just want to add a comment to indicate that which_bucket()
was once called here, in case which_bucket() changes in the future,
and however, we stayed with the original approach, leading to the
inconsistency.

Could you please review the comment below and let me know if it's okay
or if I should not add any log? Thanks a lot~

/* KTIME_MAX falls into the last bucket, skip which_bucket(). */



I will collect review comments before arise patch V2.


-- 
Thx and BRs,
Zhongqiu Han