Re: [Xen-devel] [V5] x86/cpuidle: get accurate C0 value with xenpm tool

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.05.15 at 07:23, <huaitong.han@xxxxxxxxx> wrote:
> @@ -574,6 +597,7 @@ static void acpi_processor_idle(void)
>              t1 = cpuidle_get_tick();
>              /* Trace cpu idle entry */
>              TRACE_4D(TRC_PM_IDLE_ENTRY, cx->idx, t1, exp, pred);
> +            update_last_cx_stat(power, cx, t1);
>              /* Invoke C2 */
>              acpi_idle_do_entry(cx);
>              /* Get end time (ticks) */
> @@ -602,7 +626,7 @@ static void acpi_processor_idle(void)
>          t1 = cpuidle_get_tick();
>          /* Trace cpu idle entry */
>          TRACE_4D(TRC_PM_IDLE_ENTRY, cx->idx, t1, exp, pred);
> -
> +        update_last_cx_stat(power, cx, t1);
>          /*

Please instead of deleting the blank line here, add another one after
the added line and add ones around the addition in the earlier hunk.

> @@ -1172,7 +1196,10 @@ int pmstat_get_cx_stat(uint32_t cpuid, struct 
> pm_cx_stat *stat)
>  {
>      struct acpi_processor_power *power = processor_powers[cpuid];
>      uint64_t idle_usage = 0, idle_res = 0;
> -    uint64_t usage[ACPI_PROCESSOR_MAX_POWER], res[ACPI_PROCESSOR_MAX_POWER];
> +    uint64_t last_state_update_tick, current_stime, current_tick;
> +    uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 };
> +    uint64_t res_ticks[ACPI_PROCESSOR_MAX_POWER] = { 0 };
> +    uint64_t res[ACPI_PROCESSOR_MAX_POWER] = { 0 };

Not yet another array on the stack please - I can't see why you
can't get away with just res[].

> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -536,7 +536,6 @@ static void mwait_idle(void)
>               return;
>       }
>  
> -     power->last_state = cx;
>       eax = cx->address;
>       cstate = ((eax >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1;
>  
> @@ -554,6 +553,8 @@ static void mwait_idle(void)
>  
>       before = cpuidle_get_tick();
>       TRACE_4D(TRC_PM_IDLE_ENTRY, cx->type, before, exp, pred);
> +     /* Now in CX */
> +     update_last_cx_stat(power, cx, before);

Again - blank line ahead of the addition please. Also the comment is
both wrong and (as pointed out before) lacking a stop. Perhaps -
just like in the ACPI driver - just omit it (and fix only the other one a
few lines down)?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel