Re: [Sched-tools] [PATCH] idlestat: Initialize CPU's idle state properly

Hi Vincent,

On Tue, May 17, 2016 at 10:00:58AM +0200, Vincent Guittot wrote:

• adding sched-tools mailing list

Thanks. Please hold on for this patch, from testing found some regression introduced by this patch.

After I fix all related issue, will resent patch.

Thanks, Leo Yan

On 17 May 2016 at 03:02, Leo Yan leo.yan@linaro.org wrote:

...

In current code the CPU's idle state cpufreq_pstates::idle is initialized to '-1'; and until parse first "cpu_idle" event for the CPU then set CPU's idle state to '0' or '1' corresponding to active or idle. This will cause error for P-state's statistics: from the beginning to first "cpu_idle" event, during this period the CPU's idle state is '-1' so function cpu_change_pstate() will always think it's first update and finally abandon previous time.

This will introduce very big error if the CPU is always running and never run into idle state. So this patch is to fix this issue by initialize CPU's idle state before parse P-state and C-state's time. Initialize CPU's idle state according to first cpu_idle log:

• If the CPU first cpu_idle state is '-1', that means from the beginning the CPU is stayed on idle state;
• If the CPU first cpu_idle state is other value, means the CPU is active.

Signed-off-by: Leo Yan leo.yan@linaro.org

tracefile_idlestat.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)

diff --git a/tracefile_idlestat.c b/tracefile_idlestat.c index 3430693..fb0b3d8 100644 --- a/tracefile_idlestat.c +++ b/tracefile_idlestat.c @@ -152,6 +152,55 @@ int load_text_data_line(char *buffer, struct cpuidle_datas *datas, char *format, return get_wakeup_irq(datas, buffer); }

+/**

• • init_cpu_idle_state - Init CPU's idle state according to first cpu_idle log.
• • For a specific cpu_idle event, its state is '-1' then that means from the
• • beginning the CPU is stayed on idle state; Otherwise means the CPU is active.
• • So initilize per-CPU idle flag to get more accurate time.
• • @datas: structure for P-state and C-state's statistics
• • @f: the file handle of the idlestat trace file
• */

+void init_cpu_idle_state(struct cpuidle_datas *datas, FILE *f) +{

•   struct cpufreq_pstates *ps;
  

•   unsigned int state, freq, cpu;
  

•   double time;
  

•   char buffer[BUFSIZE];
  

•   do {
  

•           if (strstr(buffer, "cpu_idle")) {
  

•                   if (sscanf(buffer, TRACE_FORMAT, &time, &state, &cpu)
  

•                       != 3) {
  

•                           fprintf(stderr, "warning: Unrecognized cpuidle "
  

•                                   "record. The result of analysis might "
  

•                                   "be wrong.\n");
  

•                           return -1;
  

•                   }
  

•           }
  

•           ps = &(datas->pstates[cpu]);
  

•           /* CPU's state has been initialized, skip it */
  

•           if (ps->idle != -1)
  

•                   continue;
  

•           /*
  

•            * The CPU's first cpu_idle is '-1', means CPU is staying in
  

•            * idle state and exit from idle until first cpu_idle event.
  

•            * Otherwise, means the CPU is active at beginning.
  

•            */
  

•           if (state == -1)
  

•                   ps->idle = 1;
  

•           else
  

•                   ps->idle = 0;
  

•   } while (fgets(buffer, BUFSIZE, f));
  

•   /* After traverse file, reset offset */
  

•   fseek(f, 0, SEEK_SET);
  

+}

void load_text_data_lines(FILE *f, char *buffer, struct cpuidle_datas *datas) { double begin = 0, end = 0; @@ -159,6 +208,8 @@ void load_text_data_lines(FILE *f, char *buffer, struct cpuidle_datas *datas)

    setup_topo_states(datas);

•   init_cpu_idle_state(datas, f);
  

•   do {
            if (load_text_data_line(buffer, datas, TRACE_FORMAT,
                                    &begin, &end, &start) != -1) {
  

-- 1.9.1