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

Message ID 1389876316-27161-1-git-send-email-alex.shi@linaro.org
State New
Headers show

Commit Message

Alex Shi Jan. 16, 2014, 12:45 p.m.
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

Alex Shi Jan. 22, 2014, 7:53 a.m. | #1
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. | #2
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;
>>>
>>
>
>

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;