Re: [Sched-tools] [RFC PATCH] idlestat: Support energy modeling of idle consumption at different OPPs

Hi Xunlei, This looks fine to me. I do have some comments inline.

On Mon, May 25, 2015 at 6:22 PM, Xunlei Pang xlpang@126.com wrote:

From: Xunlei Pang pang.xunlei@linaro.org

BACKGROUND Overview: Currently idlestat only handles a energy model with a single value for energy savings when going to WFI. IOW, idlestat does not know what OPP (frequency, voltage) is this WFI energy savings calculated at.

Add funtionality to allow a table of OPP and energy saving values to be listed so that the savings at various frequencies can be modeled correctly.

As an example, a platform may have different power consumption when idled at different OPPs, as follow:

@1989 MHz: single core in WFI consumes about 120 mW @1001 MHz: single core in WFI consumes about 45 mW @ 507 MHz: single core in WFI consumes about 30 mW

This data is useful to optimise a platform's idle policy.

MODIFICATION METHOD Now we want to modify the format of idlestat's energy model to accomodate the different C-states energy data at different OPPs.

Let's take a simple example to explicate the way: Assuming different C-States energy data below: 1200Mhz: WFI 80 10 1200Mhz: cluster-sleep-b 30 5

1000Mhz: WFI 70 0 1000Mhz: cluster-sleep-b 25 0

Before modification: clusterA: 2 cap states 2 C states

P-states: 1200 15200 6997 1000 8446 3846

C-states: WFI 70 0 cluster-sleep-b 25 0

After modification: clusterA: 2 cap states 2 C states

P-state C-states pair 1: 1200 15200 6997 WFI 80 10 cluster-sleep-b 30 5

P-state C-states pair 2: 1000 8446 3846 WFI 70 0 cluster-sleep-b 25 0

In this way, we ensure P-states and C-States always appear in pairs.

Signed-off-by: Xunlei Pang pang.xunlei@linaro.org

energy_model.c | 104 ++++++++++++++++++++++----------------------------- idlestat.c | 64 +++++++++++++++++++++++++++++-- idlestat.h | 10 +++++ trace_ops.h | 1 - tracefile_idlestat.c | 4 ++ 5 files changed, 120 insertions(+), 63 deletions(-)

diff --git a/energy_model.c b/energy_model.c index 2ea6fdd..3932c23 100644 --- a/energy_model.c +++ b/energy_model.c @@ -75,7 +75,7 @@ static int make_energy_model_template(struct program_options *options) list_for_each_entry(s_phy, &cpu_topo->physical_head, list_physical) { unsigned int num_cap_states = 0; unsigned int num_c_states = 0;

•           int i;
  

•           int i, j;
  
            s_core = list_entry((&s_phy->core_head)->prev, struct
  

cpu_core, list_core); s_cpu = list_entry((&s_core->cpu_head)->prev, struct cpu_cpu, list_cpu); @@ -89,27 +89,23 @@ static int make_energy_model_template(struct program_options *options) num_cap_states++; }

•           fprintf(f, "\nC-states:\n");
  

•           fprintf(f, "cluster%c: %d cap states %d C states\n\n",
  

cluster_number + 'A', num_cap_states, num_c_states);

•           fprintf(f, "P-states:\n");
  

•           fprintf(f, "# speed, cluster power, core power\n");
  

•           for (i = 0; i < s_cpu->pstates->max; i++) {
                    struct cpufreq_pstate *p =
  

&s_cpu->pstates->pstate[i];

                    if (p->freq == 0)
                            continue;

•                   fprintf(f, "P-state C-states pair %d:\n", i + 1);
  

•                   fprintf(f, "# P-state: speed, cluster power, core
  

power\n"); fprintf(f, "%d\t\t?\t?\n", p->freq/1000);

•           }
  

•           fprintf(f, "\nC-states:\n");
  

•           fprintf(f, "# name, cluster power, core power\n");
  

•           for (i = 0; i < s_cpu->cstates->cstate_max + 1; i++) {
  

•                   struct cpuidle_cstate *c =
  

&s_cpu->cstates->cstate[i];

•                   fprintf(f, "# C-states: name, cluster power, core
  

power\n");

•                   for (j = 0; j < s_cpu->cstates->cstate_max + 1;
  

j++) {

•                           struct cpuidle_cstate *c =
  

&s_cpu->cstates->cstate[j];

•                   fprintf(f, "%s\t\t?\t?\n", c->name);
  

•                           fprintf(f, "%s\t\t?\t?\n", c->name);
  

•                   }
            }
            fprintf(f, "\n");
            cluster_number++;
  

@@ -130,8 +126,8 @@ int parse_energy_model(struct program_options *options) struct cluster_energy_info *clustp = NULL; unsigned int number_cap_states, number_c_states; int current_cluster = -1;

•   unsigned int current_pstate = 0;
  

•   unsigned int current_cstate = 0;
  

•   unsigned int current_pstate, speed;
  

•   unsigned int current_cstate;
    unsigned int clust_p, core_p;
    char buffer[BUFSIZE];
    char *path = options->energy_model_filename;
  

@@ -196,12 +192,13 @@ int parse_energy_model(struct program_options *options) clustp->number_c_states = number_c_states; clustp->p_energy = calloc(number_cap_states, sizeof(struct pstate_energy_info));

•                   clustp->c_energy = calloc(number_c_states,
  

•                   clustp->c_energy =
  

calloc(number_c_states*number_cap_states,

add space before and after the asterisk?

                            sizeof(struct cstate_energy_info));
                    clustp->state = parsed_cluster_info;

•                   current_pstate = 0;
  
  

initializing current_pstates here and removing initialzation in declaration are not intuitive to me. And it may causes compiler warning some times.

                    continue;
            }

•           if (strstr(buffer, "P-states")) {
  

•           if (strstr(buffer, "P-state C-states pair")) {
                    if (current_cluster == -1) {
                            fprintf(stderr, "%s: unknown cluster (cap
  

states) in %s\n", __func__, path); @@ -214,27 +211,9 @@ int parse_energy_model(struct program_options *options) fclose(f); return -1; }

•                   current_pstate = 0;
                    clustp->state = parsing_cap_states;
                    continue;
            }
  

•           if (strstr(buffer, "C-states")) {
  

•                   if (current_cluster == -1) {
  

•                           fprintf(stderr, "%s: unknown cluster (c
  

states) in %s\n",

•                                   __func__, path);
  

•                           fclose(f);
  

•                           return -1;
  

•                   }
  

•                   if (clustp->state < parsed_cluster_info) {
  

•                           fprintf(stderr, "%s: number of c states
  

for cluster%c not specified in %s\n",

•                                   __func__, current_cluster, path);
  

•                           fclose(f);
  

•                           return -1;
  

•                   }
  

•                   current_cstate = 0;
  

•                   clustp->state = parsing_c_states;
  

•                   continue;
  

•           }
            if (strstr(buffer, "wakeup")) {
                    unsigned int clust_w, core_w;
  
  

@@ -254,10 +233,9 @@ int parse_energy_model(struct program_options *options) __func__, path); fclose(f); return -1;

•                   }
  

•           }
            if (clustp->state == parsing_cap_states) {
                    struct pstate_energy_info *pp;
  

•                   unsigned int speed;
  
                    if (sscanf(buffer, "%d %d %d", &speed, &clust_p,
  

&core_p) != 3) { fprintf(stderr, "%s: expected P state (speed cluster core) for cluster%c in %s\n", @@ -276,6 +254,8 @@ int parse_energy_model(struct program_options *options) pp->speed = speed; pp->cluster_power = clust_p; pp->core_power = core_p;

•                   clustp->state = parsing_c_states;
  

•                   current_cstate = 0;
                    continue;
            }
            if (clustp->state == parsing_c_states) {
  

@@ -295,8 +275,9 @@ int parse_energy_model(struct program_options *options) fclose(f); return -1; }

•                   cp = &clustp->c_energy[current_cstate++];
  

•                   cp = &clustp->c_energy[(current_pstate - 1) *
  

clustp->number_c_states + current_cstate++]; strncpy(cp->cstate_name, name, NAMELEN);

•                   cp->speed = speed;
                    cp->cluster_idle_power = clust_p;
                    cp->core_idle_power = core_p;
                    continue;
  

@@ -309,7 +290,7 @@ int parse_energy_model(struct program_options *options) return 0; }

-static struct cstate_energy_info *find_cstate_energy_info(const unsigned int cluster, const char *name) +static struct cstate_energy_info *find_cstate_energy_info(const unsigned int cluster, unsigned int speed, const char *name) { struct cluster_energy_info *clustp; struct cstate_energy_info *cp; @@ -317,8 +298,8 @@ static struct cstate_energy_info *find_cstate_energy_info(const unsigned int clu

    clustp = cluster_energy_table + cluster;
    cp = &clustp->c_energy[0];

•   for (i = 0; i < clustp->number_c_states; i++, cp++) {
  

•           if (!strcmp(cp->cstate_name, name)) return cp;
  

•   for (i = 0; i < clustp->number_c_states *
  

clustp->number_cap_states; i++, cp++) {

•           if (cp->speed == speed && !strcmp(cp->cstate_name, name))
  

return cp; } return NULL; } @@ -377,6 +358,7 @@ void calculate_energy_consumption(struct cpu_topology *cpu_topo)

            for (j = 0; j < s_phy->cstates->cstate_max + 1; j++) {
                    struct cpuidle_cstate *c =

&s_phy->cstates->cstate[j];

•                   struct duration_opp *opps = c->duration_opps;
  
                    if (c->nrdata == 0) {
                            verbose_fprintf(stderr, 2,
  

@@ -385,19 +367,20 @@ void calculate_energy_consumption(struct cpu_topology *cpu_topo) continue; }

•                   cp = find_cstate_energy_info(current_cluster,
  

c->name);

•                   if (!cp) {
  

•                           verbose_fprintf(stderr, 2, "      C%-2d no
  

energy model for [%s] (%d hits, %f duration)\n",

•                                           j, c->name, c->nrdata,
  

c->duration);

•                           continue;
  

•                   }
  

•                   for (i = 0; i < c->nropp; i++) {
  

•                           cp =
  

find_cstate_energy_info(current_cluster, opps[i].speed/1000, c->name);

•                           if (!cp) {
  

•                                   verbose_fprintf(stderr, 2, "
  

C%-2d no energy model for [%s] (%d hits, %f duration) %d speed\n",

•                                                   j, c->name,
  

c->nrdata, opps[i].duration, opps[i].speed);

•                                   continue;
  

•                           }
  
  

•                   cluster_idl += c->duration *
  

cp->cluster_idle_power;

•                           cluster_idl += opps[i].duration *
  

cp->cluster_idle_power;

•                   }
  
  

•                   verbose_fprintf(stderr, 1, "      C%-2d +%7d hits
  

for [%15s] | %13.0f | %7d | %7s | %12s | %12.0f | %12s |\n",

•                   verbose_fprintf(stderr, 1, "      C%-2d +%7d hits
  

for [%15s] | %13.0f | %7s | %12s | %12.0f | %12s |\n", j, c->nrdata, c->name, c->duration,

•                                   cp->cluster_idle_power,
                                    "", "",
                                    cluster_idl,
                                    "");
  

@@ -415,7 +398,7 @@ void calculate_energy_consumption(struct cpu_topology *cpu_topo)

                    pp = find_pstate_energy_info(current_cluster,

p->freq/1000); if (!pp) {

•                           verbose_fprintf(stderr, 2, "Cluster %c
  

frequency %u MHz no energy model for [%d] (%d hits, %f duration)\n",

•                           verbose_fprintf(stderr, 1, "Cluster %c
  

frequency %u MHz no energy model for [%d] (%d hits, %f duration)\n", s_phy->physical_id + 'A', p->freq/1000, p->count, p->duration); continue; @@ -442,25 +425,28 @@ void calculate_energy_consumption(struct cpu_topology *cpu_topo)

                            for (i = 0; i < s_cpu->cstates->cstate_max

• 1; i++) { struct cpuidle_cstate *c =

&s_cpu->cstates->cstate[i];

•                                   struct duration_opp *opps =
  

c->duration_opps;

•                                   if (c->nrdata == 0) {
                                            verbose_fprintf(stderr, 2,
                                                    "Cpu%d  C%-2d
  

no hits for [%15s] | 0 | 0 | | | 0 | |\n", s_cpu->cpu_id, i, c->name); continue; }

•                                   cp =
  

find_cstate_energy_info(current_cluster, c->name);

•                                   if (!cp) {
  

•                                           verbose_fprintf(stderr, 2,
  

"Cpu%d C%-2d no energy model for [%s] (%d hits, %f duration)\n",

•                                                   s_cpu->cpu_id, i,
  

c->name,

•                                                   c->nrdata,
  

c->duration);

•                                           continue;
  

•                                   for (j = 0; j < c->nropp; j++) {
  

•                                           cp =
  

find_cstate_energy_info(current_cluster, opps[j].speed/100, c->name);

•                                           if (!cp) {
  

verbose_fprintf(stderr, 2, "Cpu%d C%-2d no energy model for [%s] (%d hits, %f duration, %d speed)\n",

s_cpu->cpu_id, i, c->name,

•                                                           c->nrdata,
  

opps[j].duration, opps[j].speed);

•                                                   continue;
  

•                                           }
  

•                                           cluster_idl +=
  

opps[j].duration * cp->core_idle_power; }

•                                   cluster_idl += c->duration *
  

cp->core_idle_power;

•                                   verbose_fprintf(stderr, 1, "Cpu%d
  

C%-2d +%7d hits for [%15s] | %13.0f | %7d | %7s | %12s | %12.0f | %12s |\n",

•                                   verbose_fprintf(stderr, 2, "Cpu%d
  

C%-2d +%7d hits for [%15s] | %13.0f | %7s | %12s | %12.0f | %12s |\n", s_cpu->cpu_id, i, c->nrdata, c->name, c->duration,

cp->core_idle_power, "", "", cluster_idl, ""); diff --git a/idlestat.c b/idlestat.c index 4d773f4..75ac1fc 100644 --- a/idlestat.c +++ b/idlestat.c @@ -335,9 +335,10 @@ struct cpuidle_cstates *build_cstate_info(int nrcpus) c->avg_time = 0.; c->max_time = 0.; c->min_time = DBL_MAX;

•                   c->duration = 0.;
                    c->target_residency =
                            cpuidle_get_target_residency(cpu, i);
  

•                   c->duration_opps = NULL;
  

•                   c->nropp = 0;
            }
    }
    return cstates;
  

@@ -739,6 +740,7 @@ static int cstate_begin(double time, int state, struct cpuidle_cstates *cstates) memset(data + nrdata, 0, sizeof(*data));

    data[nrdata].begin = time;

•   data[nrdata].speed = cstates->current_speed;
  
    cstate->data = data;
    cstates->cstate_max = MAX(cstates->cstate_max, state);
  

@@ -747,7 +749,34 @@ static int cstate_begin(double time, int state, struct cpuidle_cstates *cstates) return 0; }

-static void cstate_end(double time, struct cpuidle_cstates *cstates) +static int update_duration_opp(struct cpuidle_cstate *cstate, unsigned int freq, double duration) +{

•   struct duration_opp *opps = cstate->duration_opps;
  

•   int i;
  

•   for (i = 0; i < cstate->nropp; i++) {
  

•           if (opps[i].speed == freq) {
  

•                   opps[i].duration += duration;
  

•                   return 0;
  

•           }
  

•   }
  

•   opps = realloc(cstate->duration_opps, sizeof(*opps) *
  

(cstate->nropp + 1));

•   if (!opps) {
  

•           free(cstate->duration_opps);
  

•           return error(__func__);
  

•   }
  

•   cstate->duration_opps = opps;
  

•   memset(opps + cstate->nropp, 0, sizeof(*opps));
  

•   opps[cstate->nropp].speed = freq;
  

•   opps[cstate->nropp].duration += duration;
  

•   cstate->nropp++;
  

•   return 0;
  

+}

+static int cstate_end(double time, struct cpuidle_cstates *cstates) { int last_cstate = cstates->current_cstate; struct cpuidle_cstate *cstate = &cstates->cstate[last_cstate]; @@ -792,12 +821,17 @@ static void cstate_end(double time, struct cpuidle_cstates *cstates) cstate->max_time = MAX(cstate->max_time, data->duration); cstate->avg_time = AVG(cstate->avg_time, data->duration, cstate->nrdata + 1);

•   if (update_duration_opp(cstate, data->speed, data->duration) < 0)
  

•           return error(__func__);
  

•   cstate->duration += data->duration;
    cstate->nrdata++;
  
  

skip_entry: /* CPU is no longer idle */ cstates->current_cstate = -1;

•   return 0;
  

}

Don't see why changing to return a int here. Can you elabrate a little bit?

int record_cstate_event(struct cpuidle_cstates *cstates, @@ -810,13 +844,35 @@ int record_cstate_event(struct cpuidle_cstates *cstates, return 0;

    if (cstates->current_cstate != -1)

•           cstate_end(time, cstates);
  

•           ret = cstate_end(time, cstates);
  
  

Same here, the ret here is obvious to me

    if (state != -1)
            ret = cstate_begin(time, state, cstates);

    return ret;

}

+static void core_update_current_speed(struct cpu_core *core) +{

•   struct cpu_cpu *cpu;
  

•   unsigned int max_speed = 0;
  

•   core_for_each_cpu(cpu, core)
  

•           max_speed = MAX(max_speed, cpu->cstates->current_speed);
  

•   core->cstates->current_speed = max_speed;
  

+}

+static void cluster_update_current_speed(struct cpu_physical *clust) +{

•   struct cpu_cpu *cpu;
  

•   unsigned int max_speed = 0;
  

•   cluster_for_each_cpu(cpu, clust)
  

•           max_speed = MAX(max_speed, cpu->cstates->current_speed);
  

•   clust->cstates->current_speed = max_speed;
  

+}

int store_data(double time, int state, int cpu, struct cpuidle_datas *datas) { @@ -843,11 +899,13 @@ int store_data(double time, int state, int cpu, /* Update core and cluster */ aff_core = cpu_to_core(cpu, datas->topo); state = core_get_least_cstate(aff_core);

•   core_update_current_speed(aff_core);
    if (record_cstate_event(aff_core->cstates, time, state) == -1)
            return -1;
  
    aff_cluster = cpu_to_cluster(cpu, datas->topo);
    state = cluster_get_least_cstate(aff_cluster);
  

•   cluster_update_current_speed(aff_cluster);
    if (record_cstate_event(aff_cluster->cstates, time,state) == -1)
            return -1;
  
  

diff --git a/idlestat.h b/idlestat.h index e030f6a..2eef6c9 100644 --- a/idlestat.h +++ b/idlestat.h @@ -48,11 +48,17 @@ "/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_cur_freq"

struct cpuidle_data {

•   unsigned int speed;
    double begin;
    double end;
    double duration;
  

};

+struct duration_opp {

•   unsigned int speed;
  

•   double duration;
  

+};

struct cpuidle_cstate { char *name; struct cpuidle_data *data; @@ -62,7 +68,9 @@ struct cpuidle_cstate { double avg_time; double max_time; double min_time;

•   struct duration_opp *duration_opps;
    double duration;
  

•   int nropp;
    int target_residency; /* -1 if not available */
  

};

@@ -83,6 +91,7 @@ struct cpuidle_cstates { struct cpuidle_cstate cstate[MAXCSTATE]; struct wakeup_info wakeinfo; int current_cstate;

•   unsigned int current_speed;
    int cstate_max;
    struct wakeup_irq *wakeirq;
    enum {as_expected, too_long, too_short} actual_residency;
  

@@ -157,6 +166,7 @@ struct pstate_energy_info {

struct cstate_energy_info { char cstate_name[NAMELEN];

•   unsigned int speed;
    unsigned int cluster_idle_power;
    unsigned int core_idle_power;
    double cluster_duration;
  

diff --git a/trace_ops.h b/trace_ops.h index 6d097ff..41258b6 100644 --- a/trace_ops.h +++ b/trace_ops.h @@ -46,5 +46,4 @@ extern void load_text_data_lines(FILE *f, char *buffer, struct cpuidle_datas *da * tracetype_name ## _trace_ptr = &tracetype_name##_trace_ops

extern const struct trace_ops *trace_ops_head;

#endif diff --git a/tracefile_idlestat.c b/tracefile_idlestat.c index 3430693..79f8578 100644 --- a/tracefile_idlestat.c +++ b/tracefile_idlestat.c @@ -108,6 +108,8 @@ static struct cpuidle_cstates *load_and_build_cstate_info(FILE* f, char *buffer, c->max_time = 0.; c->min_time = DBL_MAX; c->duration = 0.;

•                   c->duration_opps = NULL;
  

•                   c->nropp = 0;
                    c->target_residency = residency;
            }
            fgets(buffer, BUFSIZE, f);
  

@@ -146,6 +148,8 @@ int load_text_data_line(char *buffer, struct cpuidle_datas *datas, char *format, "be wrong.\n"); return -1; }

•           datas->cstates[cpu].current_speed = freq;
            return cpu_change_pstate(datas, cpu, freq, time);
    }
  
  

-- 1.9.1

---

Sched-tools mailing list Sched-tools@lists.linaro.org https://lists.linaro.org/mailman/listinfo/sched-tools