Hi Colin,
On Wed, Jun 01, 2016 at 10:06:48AM +0100, Colin Ian King wrote:
On 01/06/16 09:15, Leo Yan 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 corresponding C-state and P-state:
- Firstly gather every CPU's starting frequency and time stamp;
- Then gather 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.
- With these info, finally initialize every CPU's C-state and P-state before analyse trace logs.
Here should note one thing is: when CPU is idle at beginning, we don't know exact idle state, so just assume CPU is in idle state 0; but this will not impact too much for statistics, due usually idlestat will wakeup all CPUs at the beginning. So it will introduce very small deviation.
Signed-off-by: Leo Yan leo.yan@linaro.org
tracefile_idlestat.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+)
diff --git a/tracefile_idlestat.c b/tracefile_idlestat.c index 3430693..d0cd366 100644 --- a/tracefile_idlestat.c +++ b/tracefile_idlestat.c @@ -152,6 +152,127 @@ 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) +{
- char buffer[BUFSIZE];
- int state, cpu;
- double time;
- struct cpufreq_pstates *ps;
- unsigned long *cpu_start_idle;
- int *cpu_start_freq;
- double cpu_start_time;
- fseek(f, 0, SEEK_SET);
perhaps rewind() is better
Will fix all related issues you meantioned.
Thanks a lot for review.
- cpu_start_freq = malloc(sizeof(int) * datas->nrcpus);
malloc return NULL, so check for that.
- for (cpu = 0; cpu < datas->nrcpus; cpu++)
cpu_start_freq[cpu] = 0xdeadbeef;- /*
* Find the start time stamp and the CPU's frequency at beginning;* So we can use these info to add dummy info.*/- while (fgets(buffer, BUFSIZE, f)) {
if (strstr(buffer, "cpu_frequency")) {if (sscanf(buffer, TRACE_FORMAT, &time, &state, &cpu)!= 3) {fprintf(stderr, "warning: Unrecognized cpuidle ""record. The result of analysis might ""be wrong.\n");return;}} elsecontinue;if (cpu_start_freq[cpu] != 0xdeadbeef)continue;if (cpu == 0)cpu_start_time = time;cpu_start_freq[cpu] = state;break;- }
- /* After traverse file, reset offset */
- fseek(f, 0, SEEK_SET);
rewind
- /*
* Find the CPU's idle state at beginning*/- cpu_start_idle = malloc(sizeof(long) * datas->nrcpus);
check for malloc returning NULL
- for (cpu = 0; cpu < datas->nrcpus; cpu++)
cpu_start_idle[cpu] = 0xdeadbeef;- while (fgets(buffer, BUFSIZE, f)) {
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;}} elsecontinue;/* CPU's state has been initialized, skip it */if (cpu_start_idle[cpu] != 0xdeadbeef)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)cpu_start_idle[cpu] = 0;elsecpu_start_idle[cpu] = 4294967295;should that be cpu_start_idle[cpu] = ULONG_MAX; ?
- }
- /* After traverse file, reset offset */
- fseek(f, 0, SEEK_SET);
rewind
- /* Initialize every CPU's cstate and pstate */
- for (cpu = 0; cpu < datas->nrcpus; cpu++) {
ps = &(datas->pstates[cpu]);if (cpu_start_idle[cpu] == 0) {/** CPU is idle at beginning, init cstate;** here don't know exact idle state, so just assume CPU* is in idle state 0; but this will not impace too much* for statistics, due usually idlestat will wakeup all* CPUs at the beginning.*/ps->idle = 1;store_data(cpu_start_time, 0, cpu, datas);} else {/* CPU is busy at beginning, init pstate */ps->idle = 0;cpu_change_pstate(datas, cpu, cpu_start_freq[cpu],cpu_start_time);}- }
+}
void load_text_data_lines(FILE *f, char *buffer, struct cpuidle_datas *datas) { double begin = 0, end = 0; @@ -159,6 +280,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) {