On 21 October 2014 21:49, Daniel Lezcano <daniel.lezc...@linaro.org> wrote:
> On 10/21/2014 03:47 PM, Tuukka Tikkanen wrote:
>>
>> On Tue, Oct 21, 2014 at 3:42 PM, Daniel Lezcano
>> <daniel.lezc...@linaro.org> wrote:
>>>
>>> On 10/04/2014 06:33 AM, pi-cheng.chen wrote:
>>>>
>>>>
>>>> Initialize struct cpufreq_pstates with initial P-state of CPUs and
>>>> allocate
>>>> struct cpufreq_pstate dynamically when parsing trace file to solve the
>>>> issue
>>>> caused by missing "scaling_avaialable_freqs" attr when using
>>>> intel_pstate
>>>> driver.
>>>>
>>>> Changes v2 to v3:
>>>> Initialize struct cpufreq_pstates with initial P-state of all CPUs and
>>>> beginning timestamp before trace acquisition and close all P-states with
>>>> ending timestamp
>>>>
>>>> hanges v1 to v2:
>>>> Sort the cpufreq_pstate list when parsing events
>>>>
>>>> Signed-off-by: Pi-Cheng Chen <pi-cheng.c...@linaro.org>
>>>
>>>
>>>
>>> At the first glance it looks ok for me.
>>>
>>> Thanks
>>> -- Daniel
>>
>>
>> Good enough for a reviewed-by with your name?
>
>
> Good enough for an Acked-by: Daniel Lezcano <daniel.lezc...@linaro.org>
Thanks for reviewing.
Pi-Cheng
>
>
>
>> Tuukka
>>
>>>
>>>
>>>> ---
>>>> idlestat.c | 259
>>>> ++++++++++++++++++++++++++++++++++++++++++++-----------------
>>>> idlestat.h | 7 ++
>>>> trace.h | 1 +
>>>> 3 files changed, 195 insertions(+), 72 deletions(-)
>>>>
>>>> diff --git a/idlestat.c b/idlestat.c
>>>> index da615cb..8230067 100644
>>>> --- a/idlestat.c
>>>> +++ b/idlestat.c
>>>> @@ -536,6 +536,121 @@ static struct cpuidle_cstates
>>>> *build_cstate_info(int
>>>> nrcpus)
>>>> return cstates;
>>>> }
>>>>
>>>> +#define TRACE_STAT_FORMAT "%*[^:]:%lf"
>>>> +
>>>> +static double get_trace_ts(void)
>>>> +{
>>>> + FILE *f;
>>>> + double ts;
>>>> +
>>>> + f = fopen(TRACE_STAT_FILE, "r");
>>>> + if (!f)
>>>> + return -1;
>>>> +
>>>> + while (fgets(buffer, BUFSIZE, f)) {
>>>> + if (!strstr(buffer, "now ts"))
>>>> + continue;
>>>> + if (!sscanf(buffer, TRACE_STAT_FORMAT, &ts))
>>>> + ts = -1;
>>>> + break;
>>>> + }
>>>> + fclose(f);
>>>> +
>>>> + return ts;
>>>> +}
>>>> +
>>>> +static void release_init_pstates(struct init_pstates *init)
>>>> +{
>>>> + free(init->freqs);
>>>> + free(init);
>>>> +}
>>>> +
>>>> +static struct init_pstates *build_init_pstates(void)
>>>> +{
>>>> + struct init_pstates *init;
>>>> + int nr_cpus, cpu;
>>>> + unsigned int *freqs;
>>>> +
>>>> + nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
>>>> + if (nr_cpus < 0)
>>>> + return NULL;
>>>> +
>>>> + init = malloc(sizeof(*init));
>>>> + if (!init)
>>>> + return NULL;
>>>> +
>>>> + freqs = calloc(nr_cpus, sizeof(*freqs));
>>>> + if (!freqs) {
>>>> + free(init);
>>>> + return NULL;
>>>> + }
>>>> + memset(freqs, 0, sizeof(*freqs) * nr_cpus);
>>>> +
>>>> + for (cpu = 0; cpu < nr_cpus; cpu++) {
>>>> + char *fpath;
>>>> + unsigned int *freq = &(freqs[cpu]);
>>>> +
>>>> + if (asprintf(&fpath, CPUFREQ_CURFREQ_PATH_FORMAT, cpu) <
>>>> 0) {
>>>> + release_init_pstates(init);
>>>> + return NULL;
>>>> + }
>>>> + if (read_int(fpath, (int *)freq))
>>>> + *freq = 0;
>>>> + free(fpath);
>>>> + }
>>>> + init->nr_cpus = nr_cpus;
>>>> + init->freqs = freqs;
>>>> +
>>>> + return init;
>>>> +}
>>>> +
>>>> +/**
>>>> + * alloc_pstate - allocate, sort, and initialize pstate struct
>>>> + * to maintain statistics of P-state transitions
>>>> + * @pstates: per-CPU P-state statistics struct
>>>> + * @freq: frequency for which the newly pstate is allocated
>>>> + *
>>>> + * Return: the index of the newly allocated pstate struct
>>>> + */
>>>> +static int alloc_pstate(struct cpufreq_pstates *pstates, unsigned int
>>>> freq)
>>>> +{
>>>> + struct cpufreq_pstate *pstate, *tmp;
>>>> + int nrfreq, i, next = 0;
>>>> +
>>>> + pstate = pstates->pstate;
>>>> + nrfreq = pstates->max;
>>>> +
>>>> + tmp = realloc(pstate, sizeof(*pstate) * (nrfreq + 1));
>>>> + if (!tmp) {
>>>> + perror("realloc pstate");
>>>> + return -1;
>>>> + }
>>>> + pstate = tmp;
>>>> + pstates->pstate = tmp;
>>>> + pstates->max = nrfreq + 1;
>>>> +
>>>> + for (i = 0; i < nrfreq && freq <= pstate[i].freq; i++)
>>>> + ;
>>>> +
>>>> + next = i;
>>>> + for (i = nrfreq; i > next && i > 0; i--) {
>>>> + pstate[i] = pstate[i - 1];
>>>> + pstate[i].id = i;
>>>> + pstates->current = (pstates->current == i - 1)?
>>>> + i : pstates->current;
>>>> + }
>>>> +
>>>> + pstate[next].id = next;
>>>> + pstate[next].freq = freq;
>>>> + pstate[next].count = 0;
>>>> + pstate[next].min_time = DBL_MAX;
>>>> + pstate[next].max_time = 0;
>>>> + pstate[next].avg_time = 0;
>>>> + pstate[next].duration = 0;
>>>> +
>>>> + return next;
>>>> +}
>>>> +
>>>> /**
>>>> * release_pstate_info - free all P-state related structs
>>>> * @pstates: per-cpu array of P-state statistics structs
>>>> @@ -560,14 +675,16 @@ static void release_pstate_info(struct
>>>> cpufreq_pstates *pstates, int nrcpus)
>>>> return;
>>>> }
>>>>
>>>> -/**
>>>> - * build_pstate_info - parse cpufreq sysfs entries and build per-CPU
>>>> - * structs to maintain statistics of P-state transitions
>>>> +/* build_pstate_info - allocate and initialize per-CPU structs to
>>>> + * maintain statistics of P-state transitions
>>>> * @nrcpus: number of CPUs
>>>> + * @initp: initial P-state of CPUs before trace acquistion
>>>> + * @start_ts: timestamp when trace acquisition started
>>>> *
>>>> * Return: per-CPU array of structs (success) or NULL (error)
>>>> */
>>>> -static struct cpufreq_pstates *build_pstate_info(int nrcpus)
>>>> +static struct cpufreq_pstates *build_pstate_info(int nrcpus,
>>>> + struct init_pstates *initp, double start_ts)
>>>> {
>>>> int cpu;
>>>> struct cpufreq_pstates *pstates;
>>>> @@ -577,67 +694,28 @@ static struct cpufreq_pstates
>>>> *build_pstate_info(int
>>>> nrcpus)
>>>> return NULL;
>>>> memset(pstates, 0, sizeof(*pstates) * nrcpus);
>>>>
>>>> - for (cpu = 0; cpu < nrcpus; cpu++) {
>>>> - struct cpufreq_pstate *pstate;
>>>> - int nrfreq;
>>>> - char *fpath, *freq, line[256];
>>>> - FILE *sc_av_freq;
>>>> -
>>>> - if (asprintf(&fpath, CPUFREQ_AVFREQ_PATH_FORMAT, cpu) <
>>>> 0)
>>>> - goto clean_exit;
>>>> -
>>>> - /* read scaling_available_frequencies for the CPU */
>>>> - sc_av_freq = fopen(fpath, "r");
>>>> - free(fpath);
>>>> - if (!sc_av_freq) {
>>>> - fprintf(stderr, "warning: P-states not supported
>>>> for "
>>>> - "CPU%d\n", cpu);
>>>> - continue;
>>>> - }
>>>> - freq = fgets(line, sizeof(line)/sizeof(line[0]),
>>>> sc_av_freq);
>>>> - fclose(sc_av_freq);
>>>> - if (!freq) {
>>>> - /* unlikely to be here, but just in case... */
>>>> - fprintf(stderr, "warning: P-state info not found
>>>> for "
>>>> - "CPU%d\n", cpu);
>>>> - continue;
>>>> - }
>>>> + if (initp)
>>>> + assert(initp->nr_cpus == nrcpus);
>>>>
>>>> - /* tokenize line and populate each frequency */
>>>> - nrfreq = 0;
>>>> - pstate = NULL;
>>>> - while ((freq = strtok(freq, "\n ")) != NULL) {
>>>> - struct cpufreq_pstate *tmp = realloc(pstate,
>>>> sizeof(*pstate) * (nrfreq+1));
>>>> - if (!tmp)
>>>> - goto clean_exit;
>>>> - pstate = tmp;
>>>> -
>>>> - /* initialize pstate record */
>>>> - pstate[nrfreq].id = nrfreq;
>>>> - pstate[nrfreq].freq = atol(freq);
>>>> - pstate[nrfreq].count = 0;
>>>> - pstate[nrfreq].min_time = DBL_MAX;
>>>> - pstate[nrfreq].max_time = 0.;
>>>> - pstate[nrfreq].avg_time = 0.;
>>>> - pstate[nrfreq].duration = 0.;
>>>> - nrfreq++;
>>>> - freq = NULL;
>>>> + for (cpu = 0; cpu < nrcpus; cpu++) {
>>>> + struct cpufreq_pstates *ps = &(pstates[cpu]);
>>>> +
>>>> + ps->pstate = NULL;
>>>> + ps->max = 0;
>>>> + ps->current = -1; /* unknown */
>>>> + ps->idle = -1; /* unknown */
>>>> + ps->time_enter = 0.;
>>>> + ps->time_exit = 0.;
>>>> +
>>>> + if (initp && initp->freqs[cpu] > 0 && start_ts > 0) {
>>>> + ps->current = alloc_pstate(ps,
>>>> initp->freqs[cpu]);
>>>> + assert(ps->current >= 0);
>>>> + ps->time_enter = start_ts;
>>>> + ps->idle = 0;
>>>> }
>>>> -
>>>> - /* now populate cpufreq_pstates for this CPU */
>>>> - pstates[cpu].pstate = pstate;
>>>> - pstates[cpu].max = nrfreq;
>>>> - pstates[cpu].current = -1; /* unknown */
>>>> - pstates[cpu].idle = -1; /* unknown */
>>>> - pstates[cpu].time_enter = 0.;
>>>> - pstates[cpu].time_exit = 0.;
>>>> }
>>>>
>>>> return pstates;
>>>> -
>>>> -clean_exit:
>>>> - release_pstate_info(pstates, nrcpus);
>>>> - return NULL;
>>>> }
>>>>
>>>> static int get_current_pstate(struct cpuidle_datas *datas, int cpu,
>>>> @@ -712,6 +790,9 @@ static void cpu_change_pstate(struct cpuidle_datas
>>>> *datas, int cpu,
>>>>
>>>> cur = get_current_pstate(datas, cpu, &ps, &p);
>>>> next = freq_to_pstate_index(ps, freq);
>>>> + if (next < 0)
>>>> + next = alloc_pstate(ps, freq);
>>>> + assert(next >= 0);
>>>>
>>>> switch (cur) {
>>>> case 1:
>>>> @@ -742,6 +823,19 @@ static void cpu_change_pstate(struct cpuidle_datas
>>>> *datas, int cpu,
>>>> }
>>>> }
>>>>
>>>> +static void cpu_close_all_pstate(struct cpuidle_datas *datas, double
>>>> time)
>>>> +{
>>>> + struct cpufreq_pstates *ps;
>>>> + struct cpufreq_pstate *p;
>>>> + int cpu, cur;
>>>> +
>>>> + for (cpu = 0; cpu < datas->nrcpus; cpu++) {
>>>> + cur = get_current_pstate(datas, cpu, &ps, &p);
>>>> + if (p && !cur && time > 0)
>>>> + close_current_pstate(ps, time);
>>>> + }
>>>> +}
>>>> +
>>>> static void cpu_pstate_idle(struct cpuidle_datas *datas, int cpu,
>>>> double
>>>> time)
>>>> {
>>>> struct cpufreq_pstates *ps = &(datas->pstates[cpu]);
>>>> @@ -763,7 +857,6 @@ static int store_data(double time, int state, int
>>>> cpu,
>>>> struct cpuidle_datas *datas, int count)
>>>> {
>>>> struct cpuidle_cstates *cstates = &datas->cstates[cpu];
>>>> - struct cpufreq_pstate *pstate = datas->pstates[cpu].pstate;
>>>> struct cpuidle_cstate *cstate;
>>>> struct cpuidle_data *data, *tmp;
>>>> int nrdata, last_cstate = cstates->last_cstate;
>>>> @@ -826,9 +919,8 @@ static int store_data(double time, int state, int
>>>> cpu,
>>>> /* need indication if CPU is idle or not */
>>>> cstates->last_cstate = -1;
>>>>
>>>> - /* update P-state stats if supported */
>>>> - if (pstate)
>>>> - cpu_pstate_running(datas, cpu, time);
>>>> + /* update P-state stats */
>>>> + cpu_pstate_running(datas, cpu, time);
>>>>
>>>> return 0;
>>>> }
>>>> @@ -846,9 +938,8 @@ static int store_data(double time, int state, int
>>>> cpu,
>>>> cstates->cstate_max = MAX(cstates->cstate_max, state);
>>>> cstates->last_cstate = state;
>>>> cstates->wakeirq = NULL;
>>>> - /* update P-state stats if supported */
>>>> - if (pstate)
>>>> - cpu_pstate_idle(datas, cpu, time);
>>>> + /* update P-state stats*/
>>>> + cpu_pstate_idle(datas, cpu, time);
>>>>
>>>> return 0;
>>>> }
>>>> @@ -932,7 +1023,8 @@ static int get_wakeup_irq(struct cpuidle_datas
>>>> *datas, char *buffer, int count)
>>>> return -1;
>>>> }
>>>>
>>>> -static struct cpuidle_datas *idlestat_load(struct program_options
>>>> *options)
>>>> +static struct cpuidle_datas *idlestat_load(struct program_options
>>>> *options,
>>>> + struct init_pstates *initp, double start_ts,
>>>> double end_ts)
>>>> {
>>>> FILE *f;
>>>> unsigned int state = 0, freq = 0, cpu = 0, nrcpus = 0;
>>>> @@ -988,14 +1080,13 @@ static struct cpuidle_datas *idlestat_load(struct
>>>> program_options *options)
>>>> return ptrerror("build_cstate_info: out of memory");
>>>> }
>>>>
>>>> - datas->pstates = build_pstate_info(nrcpus);
>>>> + datas->pstates = build_pstate_info(nrcpus, initp, start_ts);
>>>> if (!datas->pstates) {
>>>> free(datas->cstates);
>>>> free(datas);
>>>> fclose(f);
>>>> return ptrerror("build_pstate_info: out of memory");
>>>> }
>>>> -
>>>> datas->nrcpus = nrcpus;
>>>>
>>>> /* read topology information */
>>>> @@ -1018,7 +1109,6 @@ static struct cpuidle_datas *idlestat_load(struct
>>>> program_options *options)
>>>> } else if (strstr(buffer, "cpu_frequency")) {
>>>> assert(sscanf(buffer, TRACE_FORMAT, &time,
>>>> &freq,
>>>> &cpu) == 3);
>>>> - assert(datas->pstates[cpu].pstate != NULL);
>>>> cpu_change_pstate(datas, cpu, freq, time);
>>>> count++;
>>>> continue;
>>>> @@ -1031,6 +1121,9 @@ static struct cpuidle_datas *idlestat_load(struct
>>>> program_options *options)
>>>>
>>>> fclose(f);
>>>>
>>>> + /* close all p-state with end timestamp */
>>>> + cpu_close_all_pstate(datas, end_ts);
>>>> +
>>>> fprintf(stderr, "Log is %lf secs long with %zd events\n",
>>>> end - begin, count);
>>>>
>>>> @@ -1481,7 +1574,9 @@ int main(int argc, char *argv[], char *const
>>>> envp[])
>>>> {
>>>> struct cpuidle_datas *datas;
>>>> struct program_options options;
>>>> + struct init_pstates *initp = NULL;
>>>> int args;
>>>> + double start_ts, end_ts;
>>>>
>>>> args = getoptions(argc, argv, &options);
>>>> if (args <= 0)
>>>> @@ -1527,6 +1622,14 @@ int main(int argc, char *argv[], char *const
>>>> envp[])
>>>> if (idlestat_flush_trace())
>>>> return -1;
>>>>
>>>> + /* Get current P-state of all CPUs */
>>>> + if (options.display & FREQUENCY_DISPLAY)
>>>> + initp = build_init_pstates();
>>>> +
>>>> + /* Get timestamp before trace acquisition */
>>>> + if (options.display & FREQUENCY_DISPLAY)
>>>> + start_ts = get_trace_ts();
>>>> +
>>>> /* Start the recording */
>>>> if (idlestat_trace_enable(true))
>>>> return -1;
>>>> @@ -1549,6 +1652,17 @@ int main(int argc, char *argv[], char *const
>>>> envp[])
>>>> if (idlestat_trace_enable(false))
>>>> return -1;
>>>>
>>>> + /* Get timestamp after trace acquisition */
>>>> + if (options.display & FREQUENCY_DISPLAY)
>>>> + end_ts = get_trace_ts();
>>>> +
>>>> + /* Emit warning messages when P-state info might be
>>>> partial */
>>>> + if (options.display & FREQUENCY_DISPLAY &&
>>>> + (!initp || start_ts < 0 || end_ts < 0))
>>>> + fprintf(stderr, "Unable to get initial P-state,"
>>>> + " beginning timestamp, or ending
>>>> timestamp!\n"
>>>> + "P-state statistics data might be
>>>> partial!\n");
>>>> +
>>>> /* At this point we should have some spurious wake up
>>>> * at the beginning of the traces and at the end (wake
>>>> * up all cpus and timer expiration for the timer
>>>> @@ -1559,7 +1673,7 @@ int main(int argc, char *argv[], char *const
>>>> envp[])
>>>> }
>>>>
>>>> /* Load the idle states information */
>>>> - datas = idlestat_load(&options);
>>>> + datas = idlestat_load(&options, initp, start_ts, end_ts);
>>>>
>>>> if (!datas)
>>>> return 1;
>>>> @@ -1590,6 +1704,7 @@ int main(int argc, char *argv[], char *const
>>>> envp[])
>>>> }
>>>> }
>>>>
>>>> + release_init_pstates(initp);
>>>> release_cpu_topo_cstates();
>>>> release_cpu_topo_info();
>>>> release_pstate_info(datas->pstates, datas->nrcpus);
>>>> diff --git a/idlestat.h b/idlestat.h
>>>> index 735f0fe..39ba4d7 100644
>>>> --- a/idlestat.h
>>>> +++ b/idlestat.h
>>>> @@ -41,6 +41,8 @@
>>>> "/sys/devices/system/cpu/cpu%d/cpuidle/state%d/residency"
>>>> #define CPUFREQ_AVFREQ_PATH_FORMAT \
>>>>
>>>> "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_available_frequencies"
>>>> +#define CPUFREQ_CURFREQ_PATH_FORMAT \
>>>> + "/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_cur_freq"
>>>> #define CPUIDLE_STATENAME_PATH_FORMAT \
>>>> "/sys/devices/system/cpu/cpu%d/cpuidle/state%d/name"
>>>>
>>>> @@ -63,6 +65,11 @@ struct cpuidle_cstate {
>>>> int target_residency; /* -1 if not available */
>>>> };
>>>>
>>>> +struct init_pstates {
>>>> + int nr_cpus;
>>>> + unsigned int *freqs;
>>>> +};
>>>> +
>>>> enum IRQ_TYPE {
>>>> HARD_IRQ = 0,
>>>> IPI_IRQ,
>>>> diff --git a/trace.h b/trace.h
>>>> index bef6703..90b4a6a 100644
>>>> --- a/trace.h
>>>> +++ b/trace.h
>>>> @@ -33,6 +33,7 @@
>>>> #define TRACE_EVENT_PATH TRACE_PATH "/events/enable"
>>>> #define TRACE_FREE TRACE_PATH "/free_buffer"
>>>> #define TRACE_FILE TRACE_PATH "/trace"
>>>> +#define TRACE_STAT_FILE TRACE_PATH "/per_cpu/cpu0/stats"
>>>> #define TRACE_IDLE_NRHITS_PER_SEC 10000
>>>> #define TRACE_IDLE_LENGTH 196
>>>> #define TRACE_CPUFREQ_NRHITS_PER_SEC 100
>>>>
>>>
>>>
>>> --
>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>>
>>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>>> <http://twitter.com/#!/linaroorg> Twitter |
>>> <http://www.linaro.org/linaro-blog/> Blog
>>>
>>>
>>>
>>> _______________________________________________
>>> linaro-dev mailing list
>>> linaro-dev@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
