Hi Tuukka,
On Fri, Jun 03, 2016 at 06:05:21PM +0300, Tuukka Tikkanen wrote:
[...]
+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
Well, it depends. I'd rather use fseek and check the return value. If the trace happens to come from a pipe (I can think of some user cases for that), checking the return value would at least alert the user that something's not quite right when the seek fails and they can go back to using a temp file. Rewind itself is hardly anything more than a wrapper to fseek that eats up the return value.
Have refined patch to use fseek and check return value. Be honest, I usually use command like "./idlestat --import -f trace.log -p -c -w" so never use pipe as input file.
Thanks reviewing and I have sent out new version patch. Please let me know if you have any further question.
In general I'd try to shy away from multiple pass input processing, but that is an entirely different story.
Agree.
Tuukka
- 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) {
Sched-tools mailing list Sched-tools@lists.linaro.org https://lists.linaro.org/mailman/listinfo/sched-tools