Re: [V7] x86/cpuidle: get accurate C0 value with xenpm tool

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 21/05/15 10:39, Huaitong Han wrote:
> When checking the ACPI funciton of C-status, after 100 seconds sleep,
> the sampling value of C0 status from the xenpm tool decreases.
> Because C0=NOW()-C1-C2-C3-C4, when NOW() value is during idle time,
> NOW() value is bigger than last C-status update time, and C0 value
> is also bigger than ture value. if margin of the second error cannot
> make up for margin of the first error, the value of C0 would decrease.
>
> Signed-off-by: Huaitong Han <huaitong.han@intel.com>
>
>  static void print_acpi_power(uint32_t cpu, struct acpi_processor_power *power)
>  {
> -    uint32_t i, idle_usage = 0;
> -    uint64_t res, idle_res = 0;
> -    u32 usage;
> +    uint64_t idle_res = 0, idle_usage = 0;
> +    uint64_t last_state_update_tick, current_tick, current_stime;
> +    uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 };
> +    uint64_t res_tick[ACPI_PROCESSOR_MAX_POWER] = { 0 };
> +    unsigned int i;
>      u8 last_state_idx;
>  
>      printk("==cpu%d==\n", cpu);
> @@ -264,28 +286,37 @@ static void print_acpi_power(uint32_t cpu, struct acpi_processor_power *power)
>      printk("active state:\t\tC%d\n", last_state_idx);
>      printk("max_cstate:\t\tC%d\n", max_cstate);
>      printk("states:\n");
> -    
> +
> +    spin_lock_irq(&power->stat_lock);
> +    current_tick = cpuidle_get_tick();
> +    current_stime = NOW();
>      for ( i = 1; i < power->count; i++ )
>      {
> -        spin_lock_irq(&power->stat_lock);	
> -        res = tick_to_ns(power->states[i].time);
> -        usage = power->states[i].usage;
> -        spin_unlock_irq(&power->stat_lock);
> +        res_tick[i] = power->states[i].time;
> +        usage[i] = power->states[i].usage;
> +    }
> +    last_state_update_tick = power->last_state_update_tick;
> +    spin_unlock_irq(&power->stat_lock);
>  
> -        idle_usage += usage;
> -        idle_res += res;
> +    res_tick[last_state_idx] += ticks_elapsed(last_state_update_tick, current_tick);
> +    usage[last_state_idx]++;

These two arrays get used with last_state_idx as -1, which underflows to
256.

Please see about fixing, or we can see about reverting if you can't get
to it quickly.

~Andrew