diff mbox series

tools/power/turbostat: Fix logical node enumeration to allow for non-sequential physical nodes

Message ID 20180726130854.5139-1-prarit@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Len Brown
Headers show
Series tools/power/turbostat: Fix logical node enumeration to allow for non-sequential physical nodes | expand

Commit Message

Prarit Bhargava July 26, 2018, 1:08 p.m. UTC 
turbostat fails on some multi-package topologies because the logical node
enumeration assumes that the nodes are sequentially numbered which
causes the logical numa nodes to not be enumerated, or enumerated
incorrectly.

Use a more robust enumeration algorithm which allows for non-seqential
physical nodes.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: lenb@kernel.org
Cc: artem.bityutskiy@intel.com
---
 tools/power/x86/turbostat/turbostat.c | 106 ++++++++++++++++------------------
 1 file changed, 50 insertions(+), 56 deletions(-)

Comments

Len Brown July 26, 2018, 6:46 p.m. UTC | #1 
Applied for 4.18 -- thanks for the fix, Prarit!
Artem Bityutskiy July 30, 2018, 3:35 p.m. UTC | #2 
On Thu, 2018-07-26 at 14:46 -0400, Len Brown wrote:
> Applied for 4.18 -- thanks for the fix, Prarit!

I do not see them in your 'turbostat' branch, could you please push it
out?

Artem.
<html><head></head><body bgcolor="#ffffff" text="#2e3436" link="#2a76c6" vlink="#2e3436"><div>On Thu, 2018-07-26 at 14:46 -0400, Len Brown wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><pre>Applied for 4.18 -- thanks for the fix, Prarit!
</pre></blockquote><div><br></div><div>I do not see them in your 'turbostat' branch, could you please push it out?</div><div><br></div><div>Artem.</div></body></html>
Len Brown Aug. 2, 2018, 8:38 p.m. UTC | #3 
FYI, this update is in the upstream linux tree now.

thanks for your help, everybody!
On Mon, Jul 30, 2018 at 11:35 AM Artem Bityutskiy <dedekind1@gmail.com> wrote:
>
> On Thu, 2018-07-26 at 14:46 -0400, Len Brown wrote:
>
> Applied for 4.18 -- thanks for the fix, Prarit!
>
>
> I do not see them in your 'turbostat' branch, could you please push it out?
>
> Artem.
diff mbox series

Patch

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 02e71accad16..2b0135599f37 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -2471,55 +2471,43 @@  int get_core_id(int cpu)
 
 void set_node_data(void)
 {
-	char path[80];
-	FILE *filep;
-	int pkg, node, cpu;
-
-	struct pkg_node_info {
-		int count;
-		int min;
-	} *pni;
-
-	pni = calloc(topo.num_packages, sizeof(struct pkg_node_info));
-	if (!pni)
-		err(1, "calloc pkg_node_count");
-
-	for (pkg = 0; pkg < topo.num_packages; pkg++)
-		pni[pkg].min = topo.num_cpus;
-
-	for (node = 0; node <= topo.max_node_num; node++) {
-		/* find the "first" cpu in the node */
-		sprintf(path, "/sys/bus/node/devices/node%d/cpulist", node);
-		filep = fopen(path, "r");
-		if (!filep)
-			continue;
-		fscanf(filep, "%d", &cpu);
-		fclose(filep);
-
-		pkg = cpus[cpu].physical_package_id;
-		pni[pkg].count++;
-
-		if (node < pni[pkg].min)
-			pni[pkg].min = node;
-	}
-
-	for (pkg = 0; pkg < topo.num_packages; pkg++)
-		if (pni[pkg].count > topo.nodes_per_pkg)
-			topo.nodes_per_pkg = pni[0].count;
-
-	/* Fake 1 node per pkg for machines that don't
-	 * expose nodes and thus avoid -nan results
-	 */
-	if (topo.nodes_per_pkg == 0)
-		topo.nodes_per_pkg = 1;
-
-	for (cpu = 0; cpu < topo.num_cpus; cpu++) {
-		pkg = cpus[cpu].physical_package_id;
-		node = cpus[cpu].physical_node_id;
-		cpus[cpu].logical_node_id = node - pni[pkg].min;
+	int pkg, node, lnode, cpu, cpux;
+	int cpu_count;
+
+	/* initialize logical_node_id */
+	for (cpu = 0; cpu <= topo.max_cpu_num; ++cpu)
+		cpus[cpu].logical_node_id = -1;
+
+	cpu_count = 0;
+	for (pkg = 0; pkg < topo.num_packages; pkg++) {
+		lnode = 0;
+		for (cpu = 0; cpu <= topo.max_cpu_num; ++cpu) {
+			if (cpus[cpu].physical_package_id != pkg)
+				continue;
+			/* find a cpu with an unset logical_node_id */
+			if (cpus[cpu].logical_node_id != -1)
+				continue;
+			cpus[cpu].logical_node_id = lnode;
+			node = cpus[cpu].physical_node_id;
+			cpu_count++;
+			/*
+			 * find all matching cpus on this pkg and set
+			 * the logical_node_id
+			 */
+			for (cpux = cpu; cpux <= topo.max_cpu_num; cpux++) {
+				if ((cpus[cpux].physical_package_id == pkg) &&
+				   (cpus[cpux].physical_node_id == node)) {
+					cpus[cpux].logical_node_id = lnode;
+					cpu_count++;
+				}
+			}
+			lnode++;
+			if (lnode > topo.nodes_per_pkg)
+				topo.nodes_per_pkg = lnode;
+		}
+		if (cpu_count >= topo.max_cpu_num)
+			break;
 	}
-	free(pni);
-
 }
 
 int get_physical_node_id(struct cpu_topology *thiscpu)
@@ -4840,14 +4828,6 @@  void topology_probe()
 			max_siblings = siblings;
 		if (cpus[i].thread_id == 0)
 			topo.num_cores++;
-
-		if (debug > 1)
-			fprintf(outf,
-				"cpu %d pkg %d node %d core %d thread %d\n",
-				i, cpus[i].physical_package_id,
-				cpus[i].physical_node_id,
-				cpus[i].physical_core_id,
-				cpus[i].thread_id);
 	}
 
 	topo.cores_per_node = max_core_id + 1;
@@ -4873,6 +4853,20 @@  void topology_probe()
 	topo.threads_per_core = max_siblings;
 	if (debug > 1)
 		fprintf(outf, "max_siblings %d\n", max_siblings);
+
+	if (debug < 1)
+		return;
+
+	for (i = 0; i <= topo.max_cpu_num; ++i) {
+		fprintf(outf,
+			"cpu %d pkg %d node %d lnode %d core %d thread %d\n",
+			i, cpus[i].physical_package_id,
+			cpus[i].physical_node_id,
+			cpus[i].logical_node_id,
+			cpus[i].physical_core_id,
+			cpus[i].thread_id);
+	}
+
 }
 
 void