Re: [PATCH v2] idlestat: Allocate struct cpufreq_pstate dynamically

On 09/10/2014 09:33 AM, pi-cheng.chen wrote:

When using intel_pstate driver, "scaling_available_freqs" attr is not exported to sysfs. It causes assertion of idlestat due to memory of struct cpufreq_pstate was not allocated.

Allocate struct cpufreq_pstate dynamically when getting frequency information from trace file instead of parsing available frequencies from sysfs

Changes v1 to v2: Sort the cpufreq_pstate list when parsing events

Signed-off-by: Pi-Cheng Chen pi-cheng.chen@linaro.org

Tested on intel_pstate and legacy cpufreq driver.

In complement of this feature, I think the cpus cpufreq must be initialized with the 'cpuinfo_cur_freq' with the start time initialized to the beginning of the acquisition. If a cpu is in a specific freq without any changes (understand: ftrace transitions which trigger the p-state creation), we will have a long freq residency (the initial one).

Today we don't have this, so the cpus without freq changes are not displayed.

At least the minimal number of p-state should be '1' not '0' (except if cpufreq is disabled).

-- Daniel

---

idlestat.c | 118 +++++++++++++++++++++++++++++-------------------------------- 1 file changed, 56 insertions(+), 62 deletions(-)

diff --git a/idlestat.c b/idlestat.c index da615cb..d03f12e 100644 --- a/idlestat.c +++ b/idlestat.c @@ -561,8 +561,8 @@ static void release_pstate_info(struct cpufreq_pstates *pstates, int nrcpus) }

/**

• • 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
  • Return: per-CPU array of structs (success) or NULL (error)

@@ -578,55 +578,8 @@ static struct cpufreq_pstates *build_pstate_info(int nrcpus) 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;
  

• }
  

• /* 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;
  

• }
  

• /* now populate cpufreq_pstates for this CPU */
  

• pstates[cpu].pstate = pstate;
  

• pstates[cpu].max = nrfreq;
  

• pstates[cpu].pstate = NULL;
  

• pstates[cpu].max = 0;
  

  pstates[cpu].current = -1; /* unknown */ pstates[cpu].idle = -1; /* unknown */ pstates[cpu].time_enter = 0.;

@@ -634,10 +587,52 @@ static struct cpufreq_pstates *build_pstate_info(int nrcpus) }

return pstates; +}

-clean_exit:

• release_pstate_info(pstates, nrcpus);
• return NULL;

+/**

• • 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; }

  static int get_current_pstate(struct cpuidle_datas *datas, int cpu,

@@ -712,6 +707,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:

@@ -763,7 +761,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 +823,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 +842,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; }

@@ -1018,7 +1013,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;