Idlestat: Calculate core/cluster data properly

Current implementation for core/cluster data calculation is to look
for intersections of same state residency in cpu data after loading
all of the data. This fails to correctly record core/cluster data
for cases where all the contained cpus are in a low power state, but
are not using the exact same state.

To resolve this, core/cluster state needs to be evaluated at the
same time as the cpu events are being processes. The store_data()
function is modified to record not only the affected cpu changes,
but also the affected core and cluster. Supporting changes are added
throughout the source tree.

Signed-off-by: Tuukka Tikkanen <tuukka.tikkanen@linaro.org>

6 files changed, 110 insertions, 88 deletions

diff --git a/energy_model.c b/energy_model.c
index 6a88a16..05b9123 100644
--- a/energy_model.c
+++ b/energy_model.c
@@ -40,7 +40,6 @@ static int make_energy_model_template(struct program_options *options)
 		return -1;
 	}
 	cpu_topo = datas->topo;
-	establish_idledata_to_topo(datas);
 
 	list_for_each_entry(s_phy, &cpu_topo->physical_head, list_physical)
 		cluster_count++;
diff --git a/idlestat.c b/idlestat.c
index 82b01ed..a9dc12a 100644
--- a/idlestat.c
+++ b/idlestat.c
@@ -780,7 +780,7 @@ static int cstate_begin(double time, int state, struct cpuidle_cstates *cstates)
 	return 0;
 }
 
-static int cstate_end(double time, struct cpuidle_cstates *cstates)
+static void cstate_end(double time, struct cpuidle_cstates *cstates)
 {
 	int last_cstate = cstates->current_cstate;
 	struct cpuidle_cstate *cstate = &cstates->cstate[last_cstate];
@@ -824,8 +824,23 @@ static int cstate_end(double time, struct cpuidle_cstates *cstates)
 
 	/* CPU is no longer idle */
 	cstates->current_cstate = -1;
+}
 
-	return 0;
+int record_cstate_event(struct cpuidle_cstates *cstates,
+		       double time, int state)
+{
+	int ret = 0;
+
+	/* Ignore when we enter the current state (cores and clusters) */
+	if (state == cstates->current_cstate)
+		return 0;
+
+	if (cstates->current_cstate != -1)
+		cstate_end(time, cstates);
+	if (state != -1)
+		ret = cstate_begin(time, state, cstates);
+
+	return ret;
 }
 
 int store_data(double time, int state, int cpu,
@@ -833,25 +848,30 @@ int store_data(double time, int state, int cpu,
 {
 	struct cpuidle_cstates *cstates = &datas->cstates[cpu];
 	struct cpufreq_pstate *pstate = datas->pstates[cpu].pstate;
-	int ret;
+	struct cpu_core *aff_core;
+	struct cpu_physical *aff_cluster;
 
 	/* ignore when we got a "closing" state first */
 	if (state == -1 && cstates->cstate_max == -1)
 		return 0;
 
-	if (state == -1) {
-		ret = cstate_end(time, cstates);
-		/* update P-state stats if supported */
-		if (!ret && pstate)
+	if (record_cstate_event(cstates, time, state) == -1)
+		return -1;
+
+	/* Update P-state stats if supported */
+	if (pstate) {
+		if (state == -1)
 			cpu_pstate_running(datas, cpu, time);
-	} else {
-		ret = cstate_begin(time, state, cstates);
-		/* update P-state stats if supported */
-		if (!ret && pstate)
+		else
 			cpu_pstate_idle(datas, cpu, time);
 	}
 
-	return ret;
+	/* Update core and cluster */
+	aff_core = cpu_to_core(cpu, datas->topo);
+	if (record_cstate_event(aff_core->cstates, time, state) == -1)
+		return -1;
+	aff_cluster = cpu_to_cluster(cpu, datas->topo);
+	return record_cstate_event(aff_cluster->cstates, time,state);
 }
 
 static void release_datas(struct cpuidle_datas *datas)
@@ -1635,6 +1655,8 @@ int main(int argc, char *argv[], char *const envp[])
 	if (is_err(datas))
 		return 1;
 
+	cpu_topo = datas->topo;
+
 	if (options.baseline_filename) {
 		baseline = idlestat_load(options.baseline_filename);
 		merge_pstates(datas, baseline);
@@ -1646,38 +1668,36 @@ int main(int argc, char *argv[], char *const envp[])
 		return 1;
 
 	datas->baseline = baseline;
-	cpu_topo = datas->topo;
+	assign_baseline_in_topo(datas);
 
-	/* Compute cluster idle intersection between cpus belonging to
-	 * the same cluster
-	 */
-	if (0 == establish_idledata_to_topo(datas)) {
-		if (output_handler->open_report_file(options.outfilename, report_data))
-			return 1;
+	if (output_handler->open_report_file(options.outfilename, report_data))
+		return 1;
 
-		if (options.display & IDLE_DISPLAY) {
-			output_handler->cstate_table_header(report_data);
-			dump_cpu_topo_info(output_handler, report_data, display_cstates, cpu_topo, 1);
-			output_handler->cstate_table_footer(report_data);
-		}
+	if (options.display & IDLE_DISPLAY) {
+		output_handler->cstate_table_header(report_data);
+		dump_cpu_topo_info(output_handler, report_data,
+				display_cstates, cpu_topo, 1);
+		output_handler->cstate_table_footer(report_data);
+	}
 
-		if (options.display & FREQUENCY_DISPLAY) {
-			output_handler->pstate_table_header(report_data);
-			dump_cpu_topo_info(output_handler, report_data, display_pstates, cpu_topo, 0);
-			output_handler->pstate_table_footer(report_data);
-		}
+	if (options.display & FREQUENCY_DISPLAY) {
+		output_handler->pstate_table_header(report_data);
+		dump_cpu_topo_info(output_handler, report_data,
+				display_pstates, cpu_topo, 0);
+		output_handler->pstate_table_footer(report_data);
+	}
 
-		if (options.display & WAKEUP_DISPLAY) {
-			output_handler->wakeup_table_header(report_data);
-			dump_cpu_topo_info(output_handler, report_data, display_wakeup, cpu_topo, 1);
-			output_handler->wakeup_table_footer(report_data);
-		}
+	if (options.display & WAKEUP_DISPLAY) {
+		output_handler->wakeup_table_header(report_data);
+		dump_cpu_topo_info(output_handler, report_data,
+				display_wakeup, cpu_topo, 1);
+		output_handler->wakeup_table_footer(report_data);
+	}
 
-		if (options.energy_model_filename)
-			calculate_energy_consumption(cpu_topo, &options);
+	if (options.energy_model_filename)
+		calculate_energy_consumption(cpu_topo, &options);
 
-		output_handler->close_report_file(report_data);
-	}
+	output_handler->close_report_file(report_data);
 
 	release_init_pstates(initp);
 	release_datas(datas);
diff --git a/topology.c b/topology.c
index 8275cbb..699e467 100644
--- a/topology.c
+++ b/topology.c
@@ -475,62 +475,58 @@ int output_cpu_topo_info(struct cpu_topology *topo, FILE *f)
 	return 0;
 }
 
-int establish_idledata_to_topo(struct cpuidle_datas *datas)
+void assign_baseline_in_topo(struct cpuidle_datas *datas)
 {
-	struct cpu_physical *s_phy;
-	struct cpu_core     *s_core;
-	struct cpu_cpu      *s_cpu;
-	int    i;
-	int    has_topo = 0;
+	struct cpu_physical *main_phy;
+	struct cpu_core     *main_core;
+	struct cpu_cpu      *main_cpu;
+	struct cpu_physical *base_phy;
+	struct cpu_core     *base_core;
+	struct cpu_cpu      *base_cpu;
 	struct cpu_topology *topo;
+	struct cpu_topology *base_topo;
 	struct cpuidle_datas *baseline;
 
 	assert(datas != NULL);
-	topo = datas->topo;
-	assert(topo != NULL);
-
-	if (datas->baseline)
-		baseline = datas->baseline;
-	else
-		baseline = NULL;
-
-	for (i = 0; i < datas->nrcpus; i++) {
-		s_cpu = find_cpu_point(topo, i);
-		if (s_cpu) {
-			s_cpu->cstates = &datas->cstates[i];
-			s_cpu->pstates = &datas->pstates[i];
-			has_topo = 1;
-			if (baseline) {
-				s_cpu->base_cstates = baseline->cstates + i;
-				s_cpu->base_pstates = baseline->pstates + i;
-			}
-		}
-	}
 
-	if (!has_topo)
-		return -1;
+	baseline = datas->baseline;
+	if (!baseline)
+		return;
 
-	list_for_each_entry(s_phy, &topo->physical_head,
-			    list_physical) {
-		list_for_each_entry(s_core, &s_phy->core_head, list_core) {
-			s_core->cstates = core_cluster_data(s_core);
-			if (is_err(s_core->cstates)) {
-				s_core->cstates = NULL;
-				return -1;
+	topo = datas->topo;
+	base_topo = baseline->topo;
+	assert(topo != NULL);
+	assert(base_topo != NULL);
+
+	/* Cluster loop */
+	base_phy = list_first_entry(&base_topo->physical_head,
+				struct cpu_physical, list_physical);
+	topo_for_each_cluster(main_phy, topo) {
+		main_phy->base_cstates = base_phy->cstates;
+		/* Core loop */
+		base_core = list_first_entry(&base_phy->core_head,
+					struct cpu_core, list_core);
+		cluster_for_each_core(main_core, main_phy) {
+			main_core->base_cstates = base_core->cstates;
+			/* Cpu loop */
+			base_cpu = list_first_entry(&base_core->cpu_head,
+					struct cpu_cpu, list_cpu);
+			core_for_each_cpu(main_cpu, main_core) {
+				main_cpu->base_cstates = base_cpu->cstates;
+				main_cpu->base_pstates = base_cpu->pstates;
+
+				/* Step to next baseline cpu */
+				base_cpu = list_first_entry(&base_cpu->list_cpu,
+						struct cpu_cpu, list_cpu);
 			}
+			/* Step to next baseline core */
+			base_core = list_first_entry(&base_core->list_core,
+						struct cpu_core, list_core);
 		}
+		/* Step to next baseline cluster */
+		base_phy = list_first_entry(&base_phy->list_physical,
+				struct cpu_physical, list_physical);
 	}
-
-	list_for_each_entry(s_phy, &topo->physical_head,
-			    list_physical) {
-		s_phy->cstates = physical_cluster_data(s_phy);
-		if (is_err(s_phy->cstates)) {
-			s_phy->cstates = NULL;
-			return -1;
-		}
-	}
-
-	return 0;
 }
 
 int dump_cpu_topo_info(struct report_ops *ops, void *report_data, int (*dump)(struct report_ops *, void *, void *, char *, void *), struct cpu_topology *topo, int cstate)
@@ -546,13 +542,14 @@ int dump_cpu_topo_info(struct report_ops *ops, void *report_data, int (*dump)(st
 		sprintf(tmp, "cluster%c", s_phy->physical_id + 'A');
 
 		if (cstate)
-			dump(ops, s_phy->cstates, NULL, tmp, report_data);
+			dump(ops, s_phy->cstates, s_phy->base_cstates,
+				tmp, report_data);
 
 		list_for_each_entry(s_core, &s_phy->core_head, list_core) {
 			if (s_core->is_ht && cstate) {
 				sprintf(tmp, "core%d", s_core->core_id);
 				dump(ops, s_core->cstates,
-					NULL,
+					s_core->base_cstates,
 					tmp, report_data);
 			}
 
diff --git a/topology.h b/topology.h
index 9766092..e06f1dc 100644
--- a/topology.h
+++ b/topology.h
@@ -50,6 +50,7 @@ struct cpu_core {
 	int cpu_num;
 	bool is_ht;
 	struct cpuidle_cstates *cstates;
+	struct cpuidle_cstates *base_cstates;
 };
 
 struct cpu_physical {
@@ -59,6 +60,7 @@ struct cpu_physical {
 	int core_num;
 	struct list_head cpu_enum_head;
 	struct cpuidle_cstates *cstates;
+	struct cpuidle_cstates *base_cstates;
 };
 
 struct cpu_topology {
@@ -71,7 +73,7 @@ extern struct cpu_topology *read_cpu_topo_info(FILE *f, char *buf);
 extern struct cpu_topology *read_sysfs_cpu_topo(void);
 extern int release_cpu_topo_info(struct cpu_topology *topo);
 extern int output_cpu_topo_info(struct cpu_topology *topo, FILE *f);
-extern int establish_idledata_to_topo(struct cpuidle_datas *datas);
+extern void assign_baseline_in_topo(struct cpuidle_datas *datas);
 extern int release_cpu_topo_cstates(struct cpu_topology *topo);
 extern int dump_cpu_topo_info(struct report_ops *ops, void *report_data, int (*dump)(struct report_ops *, void *, void *, char *, void *), struct cpu_topology *topo, int cstate);
 
diff --git a/tracefile_idlestat.c b/tracefile_idlestat.c
index 2f59c80..1b76255 100644
--- a/tracefile_idlestat.c
+++ b/tracefile_idlestat.c
@@ -133,6 +133,8 @@ void load_text_data_lines(FILE *f, char *buffer, struct cpuidle_datas *datas)
 	double begin = 0, end = 0;
 	size_t count = 0, start = 1;
 
+	setup_topo_states(datas);
+
 	do {
 		load_text_data_line(buffer, datas, TRACE_FORMAT, &begin, &end, &count, &start);
 	} while (fgets(buffer, BUFSIZE, f));
diff --git a/tracefile_tracecmd.c b/tracefile_tracecmd.c
index cc7bf35..cf3ed89 100644
--- a/tracefile_tracecmd.c
+++ b/tracefile_tracecmd.c
@@ -16,6 +16,8 @@ void tracecmd_load_text_data_lines(FILE *f, char *buffer, struct cpuidle_datas *
 	double begin = 0, end = 0;
 	size_t count = 0, start = 1;
 
+	setup_topo_states(datas);
+
 	do {
 		load_text_data_line(buffer, datas, TRACE_CMD_REPORT_FORMAT, &begin, &end, &count, &start);
 	} while (fgets(buffer, BUFSIZE, f));