Re: [PATCH] cpuidle: Improve perf for certain workloads

[Ross Lagerwall <rosslagerwall@citrix.com>]

On 06/13/2014 12:37 PM, Jan Beulich wrote:
>>>> Ross Lagerwall <ross.lagerwall@citrix.com> 06/13/14 12:23 PM >>>
>> The existing mechanism of using interrupt frequency as a heuristic does
>> not work well for certain workloads.  As an example, synchronous dd on a
>> small block size uses deep C-states because much of the time is spent
>> doing processing so the interrupt frequency is not too high, but when an
>> IOP is submitted, the interrupt occurs soon after going idle.  This
>> causes exit latency to be a significant factor.
>>
>> To fix this, add a new factor which limits the exit latency to be no
>> more than 10% of the decaying measured idle time.  This improves
>> performance for workloads with a medium interrupt frequency but a short
>> idle duration.
>
> Does this have a Linux counterpart (after all the code here is a clone from
> Linux'es)? If so, adding a cross reference would be appreciated. If not, I'd
> expect you to explain why Xen needs what Linux doesn't need.

No it does not have a Linux counterpart. The Linux equivalent code for 
exit latency uses a combination of the PM QOS interface, the load 
average of the system and the number of processes in IO wait state on 
that CPU. If a process is in IO wait state, it compares the exit latency 
with the predicted residency reduced by a factor of 10, which is 
somewhat similar to what this patch does.

The use of average interrupt frequency was introduced by Keir Fraser in 
353533232730 ("cpuidle: fix the menu governor to enhance IO 
performance") when porting a Linux patch to Xen.

>
>> In the workload given previously, throughput improves by 20% with this
>> patch.
>
> This is the positive side. Did you also check for no negative effects?

 From a performance perspective, this patch will never cause the machine 
to go in a deeper C-state than it would have previously.

 From a power perspective, I don't have a power meter to do actual 
measurements but watching xenpm on an idle system shows that almost all 
of the time is spent in the deepest C-state.

>
>> A side effect of this patch is to fix the use of MAX_INTERESTING.
>
> What does this sentence refer to?

I will make that more explicit in the commit message.

>
>> @@ -88,6 +89,10 @@
>    >* the average interrupt interval is, the smaller C state latency should be
>    >* and thus the less likely a busy CPU will hit such a deep C state.
>    >*
>> + * As an additional rule to reduce the performance impact, menu tries to
>> + * limit the exit latency duration to be no more than 10% of the decaying
>> + * measured idle time.
>> + *
>    >*/
>
> Even if previously there was a blank comment line at the end, please avoid
> retaining such when you add further text anyway.
>

OK.

Thanks for the review,
-- 
Ross Lagerwall