diff mbox

[RFC] cpuidle: reduce unnecessary loop in c-state selection

Message ID 1389876316-27161-1-git-send-email-alex.shi@linaro.org (mailing list archive)
State RFC, archived
Headers show

Commit Message

Alex Shi Jan. 16, 2014, 12:45 p.m. UTC
All deeper c-state have the longer target_residency and exit_latency
So, if the one can not meet our prediction, neither any later.
So, just break out the for loop to save few checking instructions.

Signed-off-by: Alex Shi <alex.shi@linaro.org>
---
 drivers/cpuidle/governors/menu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Rafael J. Wysocki Jan. 16, 2014, 3:53 p.m. UTC | #1
Hi Len, what do you think?

On Thursday, January 16, 2014 08:45:16 PM Alex Shi wrote:
> All deeper c-state have the longer target_residency and exit_latency
> So, if the one can not meet our prediction, neither any later.
> So, just break out the for loop to save few checking instructions.
> 
> Signed-off-by: Alex Shi <alex.shi@linaro.org>
> ---
>  drivers/cpuidle/governors/menu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index cf7f2f0..48ed3fb 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -352,11 +352,11 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>  		if (s->disabled || su->disable)
>  			continue;
>  		if (s->target_residency > data->predicted_us)
> -			continue;
> +			break;
>  		if (s->exit_latency > latency_req)
> -			continue;
> +			break;
>  		if (s->exit_latency * multiplier > data->predicted_us)
> -			continue;
> +			break;
>  
>  		data->last_state_idx = i;
>  		data->exit_us = s->exit_latency;
>
Alex Shi Jan. 22, 2014, 7:53 a.m. UTC | #2
On 01/16/2014 11:53 PM, Rafael J. Wysocki wrote:
> Hi Len, what do you think?

Is this patch correct?
> 
> On Thursday, January 16, 2014 08:45:16 PM Alex Shi wrote:
>> All deeper c-state have the longer target_residency and exit_latency
>> So, if the one can not meet our prediction, neither any later.
>> So, just break out the for loop to save few checking instructions.
>>
>> Signed-off-by: Alex Shi <alex.shi@linaro.org>
>> ---
>>  drivers/cpuidle/governors/menu.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
>> index cf7f2f0..48ed3fb 100644
>> --- a/drivers/cpuidle/governors/menu.c
>> +++ b/drivers/cpuidle/governors/menu.c
>> @@ -352,11 +352,11 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>>  		if (s->disabled || su->disable)
>>  			continue;
>>  		if (s->target_residency > data->predicted_us)
>> -			continue;
>> +			break;
>>  		if (s->exit_latency > latency_req)
>> -			continue;
>> +			break;
>>  		if (s->exit_latency * multiplier > data->predicted_us)
>> -			continue;
>> +			break;
>>  
>>  		data->last_state_idx = i;
>>  		data->exit_us = s->exit_latency;
>>
>
Daniel Lezcano Jan. 22, 2014, 8:19 a.m. UTC | #3
On 01/22/2014 08:53 AM, Alex Shi wrote:
> On 01/16/2014 11:53 PM, Rafael J. Wysocki wrote:
>> Hi Len, what do you think?
>
> Is this patch correct?

Well, there is no rule telling in what order the idle states are, except 
they are ordered by power consumption.

But regarding how are defined the different idle states in the drivers, 
I think this patch makes sense.

>>
>> On Thursday, January 16, 2014 08:45:16 PM Alex Shi wrote:
>>> All deeper c-state have the longer target_residency and exit_latency
>>> So, if the one can not meet our prediction, neither any later.
>>> So, just break out the for loop to save few checking instructions.
>>>
>>> Signed-off-by: Alex Shi <alex.shi@linaro.org>
>>> ---
>>>   drivers/cpuidle/governors/menu.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
>>> index cf7f2f0..48ed3fb 100644
>>> --- a/drivers/cpuidle/governors/menu.c
>>> +++ b/drivers/cpuidle/governors/menu.c
>>> @@ -352,11 +352,11 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
>>>   		if (s->disabled || su->disable)
>>>   			continue;
>>>   		if (s->target_residency > data->predicted_us)
>>> -			continue;
>>> +			break;
>>>   		if (s->exit_latency > latency_req)
>>> -			continue;
>>> +			break;
>>>   		if (s->exit_latency * multiplier > data->predicted_us)
>>> -			continue;
>>> +			break;
>>>
>>>   		data->last_state_idx = i;
>>>   		data->exit_us = s->exit_latency;
>>>
>>
>
>
diff mbox

Patch

diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index cf7f2f0..48ed3fb 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -352,11 +352,11 @@  static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
 		if (s->disabled || su->disable)
 			continue;
 		if (s->target_residency > data->predicted_us)
-			continue;
+			break;
 		if (s->exit_latency > latency_req)
-			continue;
+			break;
 		if (s->exit_latency * multiplier > data->predicted_us)
-			continue;
+			break;
 
 		data->last_state_idx = i;
 		data->exit_us = s->exit_latency;