[v9,09/12] eal/windows: improve CPU and NUMA node detection

Message ID 20200615004354.14380-10-dmitry.kozliuk@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series Windows basic memory management |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Dmitry Kozlyuk June 15, 2020, 12:43 a.m. UTC
  1. Map CPU cores to their respective NUMA nodes as reported by system.
2. Support systems with more than 64 cores (multiple processor groups).
3. Fix magic constants, styling issues, and compiler warnings.
4. Add EAL private function to map DPDK socket ID to NUMA node number.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/librte_eal/windows/eal.c         |   7 +-
 lib/librte_eal/windows/eal_lcore.c   | 205 +++++++++++++++++----------
 lib/librte_eal/windows/eal_windows.h |  15 +-
 3 files changed, 152 insertions(+), 75 deletions(-)
  

Comments

Thomas Monjalon June 15, 2020, 3:21 p.m. UTC | #1
15/06/2020 02:43, Dmitry Kozlyuk:
> +	infos_size = 0;
> +	if (!GetLogicalProcessorInformationEx(
> +			RelationNumaNode, NULL, &infos_size)) {
> +		DWORD error = GetLastError();
> +		if (error != ERROR_INSUFFICIENT_BUFFER) {
> +			log_early("Cannot get NUMA node info size, error %lu\n",
> +				GetLastError());
> +			rte_errno = ENOMEM;
> +			return -1;
> +		}
> +	}
> +
> +	infos = malloc(infos_size);
> +	if (infos == NULL) {
> +		log_early("Cannot allocate memory for NUMA node information\n");
> +		rte_errno = ENOMEM;
> +		return -1;
> +	}
> +
> +	if (!GetLogicalProcessorInformationEx(
> +			RelationNumaNode, infos, &infos_size)) {
> +		log_early("Cannot get NUMA node information, error %lu\n",
> +			GetLastError());
> +		rte_errno = EINVAL;
> +		return -1;
> +	}

rte_errno is unknown

It seems to be fixed in patch 12:

--- a/lib/librte_eal/windows/eal_windows.h
+++ b/lib/librte_eal/windows/eal_windows.h
@@ -9,8 +9,24 @@
  * @file Facilities private to Windows EAL
  */
 
+#include <rte_errno.h>
 #include <rte_windows.h>


I'll merge it in patch 9
  
Dmitry Kozlyuk June 15, 2020, 3:39 p.m. UTC | #2
[snip]
> > +		rte_errno = EINVAL;
> > +		return -1;
> > +	}  
> 
> rte_errno is unknown
> 
> It seems to be fixed in patch 12:
> 
> --- a/lib/librte_eal/windows/eal_windows.h
> +++ b/lib/librte_eal/windows/eal_windows.h
> @@ -9,8 +9,24 @@
>   * @file Facilities private to Windows EAL
>   */
>  
> +#include <rte_errno.h>
>  #include <rte_windows.h>
> 
> 
> I'll merge it in patch 9

OK. Thanks for both fixes while merging and sorry for the mess. I'll try to
automate local per-commit testing next time (test-meson-builds, etc).
  

Patch

diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c
index e7461f731..dfc10b494 100644
--- a/lib/librte_eal/windows/eal.c
+++ b/lib/librte_eal/windows/eal.c
@@ -263,8 +263,11 @@  rte_eal_init(int argc, char **argv)
 
 	eal_log_level_parse(argc, argv);
 
-	/* create a map of all processors in the system */
-	eal_create_cpu_map();
+	if (eal_create_cpu_map() < 0) {
+		rte_eal_init_alert("Cannot discover CPU and NUMA.");
+		/* rte_errno is set */
+		return -1;
+	}
 
 	if (rte_eal_cpu_init() < 0) {
 		rte_eal_init_alert("Cannot detect lcores.");
diff --git a/lib/librte_eal/windows/eal_lcore.c b/lib/librte_eal/windows/eal_lcore.c
index 82ee45413..d5ff721e0 100644
--- a/lib/librte_eal/windows/eal_lcore.c
+++ b/lib/librte_eal/windows/eal_lcore.c
@@ -3,103 +3,164 @@ 
  */
 
 #include <pthread.h>
+#include <stdbool.h>
 #include <stdint.h>
 
 #include <rte_common.h>
+#include <rte_debug.h>
+#include <rte_lcore.h>
+#include <rte_os.h>
 
 #include "eal_private.h"
 #include "eal_thread.h"
 #include "eal_windows.h"
 
-/* global data structure that contains the CPU map */
-static struct _wcpu_map {
-	unsigned int total_procs;
-	unsigned int proc_sockets;
-	unsigned int proc_cores;
-	unsigned int reserved;
-	struct _win_lcore_map {
-		uint8_t socket_id;
-		uint8_t core_id;
-	} wlcore_map[RTE_MAX_LCORE];
-} wcpu_map = { 0 };
-
-/*
- * Create a map of all processors and associated cores on the system
- */
-void
-eal_create_cpu_map()
+/** Number of logical processors (cores) in a processor group (32 or 64). */
+#define EAL_PROCESSOR_GROUP_SIZE (sizeof(KAFFINITY) * CHAR_BIT)
+
+struct lcore_map {
+	uint8_t socket_id;
+	uint8_t core_id;
+};
+
+struct socket_map {
+	uint16_t node_id;
+};
+
+struct cpu_map {
+	unsigned int socket_count;
+	unsigned int lcore_count;
+	struct lcore_map lcores[RTE_MAX_LCORE];
+	struct socket_map sockets[RTE_MAX_NUMA_NODES];
+};
+
+static struct cpu_map cpu_map = { 0 };
+
+/* eal_create_cpu_map() is called before logging is initialized */
+static void
+log_early(const char *format, ...)
+{
+	va_list va;
+
+	va_start(va, format);
+	vfprintf(stderr, format, va);
+	va_end(va);
+}
+
+int
+eal_create_cpu_map(void)
 {
-	wcpu_map.total_procs =
-		GetActiveProcessorCount(ALL_PROCESSOR_GROUPS);
-
-	LOGICAL_PROCESSOR_RELATIONSHIP lprocRel;
-	DWORD lprocInfoSize = 0;
-	BOOL ht_enabled = FALSE;
-
-	/* First get the processor package information */
-	lprocRel = RelationProcessorPackage;
-	/* Determine the size of buffer we need (pass NULL) */
-	GetLogicalProcessorInformationEx(lprocRel, NULL, &lprocInfoSize);
-	wcpu_map.proc_sockets = lprocInfoSize / 48;
-
-	lprocInfoSize = 0;
-	/* Next get the processor core information */
-	lprocRel = RelationProcessorCore;
-	GetLogicalProcessorInformationEx(lprocRel, NULL, &lprocInfoSize);
-	wcpu_map.proc_cores = lprocInfoSize / 48;
-
-	if (wcpu_map.total_procs > wcpu_map.proc_cores)
-		ht_enabled = TRUE;
-
-	/* Distribute the socket and core ids appropriately
-	 * across the logical cores. For now, split the cores
-	 * equally across the sockets.
-	 */
-	unsigned int lcore = 0;
-	for (unsigned int socket = 0; socket <
-			wcpu_map.proc_sockets; ++socket) {
-		for (unsigned int core = 0;
-			core < (wcpu_map.proc_cores / wcpu_map.proc_sockets);
-			++core) {
-			wcpu_map.wlcore_map[lcore]
-					.socket_id = socket;
-			wcpu_map.wlcore_map[lcore]
-					.core_id = core;
-			lcore++;
-			if (ht_enabled) {
-				wcpu_map.wlcore_map[lcore]
-					.socket_id = socket;
-				wcpu_map.wlcore_map[lcore]
-					.core_id = core;
-				lcore++;
+	SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *infos, *info;
+	DWORD infos_size;
+	bool full = false;
+
+	infos_size = 0;
+	if (!GetLogicalProcessorInformationEx(
+			RelationNumaNode, NULL, &infos_size)) {
+		DWORD error = GetLastError();
+		if (error != ERROR_INSUFFICIENT_BUFFER) {
+			log_early("Cannot get NUMA node info size, error %lu\n",
+				GetLastError());
+			rte_errno = ENOMEM;
+			return -1;
+		}
+	}
+
+	infos = malloc(infos_size);
+	if (infos == NULL) {
+		log_early("Cannot allocate memory for NUMA node information\n");
+		rte_errno = ENOMEM;
+		return -1;
+	}
+
+	if (!GetLogicalProcessorInformationEx(
+			RelationNumaNode, infos, &infos_size)) {
+		log_early("Cannot get NUMA node information, error %lu\n",
+			GetLastError());
+		rte_errno = EINVAL;
+		return -1;
+	}
+
+	info = infos;
+	while ((uint8_t *)info - (uint8_t *)infos < infos_size) {
+		unsigned int node_id = info->NumaNode.NodeNumber;
+		GROUP_AFFINITY *cores = &info->NumaNode.GroupMask;
+		struct lcore_map *lcore;
+		unsigned int i, socket_id;
+
+		/* NUMA node may be reported multiple times if it includes
+		 * cores from different processor groups, e. g. 80 cores
+		 * of a physical processor comprise one NUMA node, but two
+		 * processor groups, because group size is limited by 32/64.
+		 */
+		for (socket_id = 0; socket_id < cpu_map.socket_count;
+		    socket_id++) {
+			if (cpu_map.sockets[socket_id].node_id == node_id)
+				break;
+		}
+
+		if (socket_id == cpu_map.socket_count) {
+			if (socket_id == RTE_DIM(cpu_map.sockets)) {
+				full = true;
+				goto exit;
+			}
+
+			cpu_map.sockets[socket_id].node_id = node_id;
+			cpu_map.socket_count++;
+		}
+
+		for (i = 0; i < EAL_PROCESSOR_GROUP_SIZE; i++) {
+			if ((cores->Mask & ((KAFFINITY)1 << i)) == 0)
+				continue;
+
+			if (cpu_map.lcore_count == RTE_DIM(cpu_map.lcores)) {
+				full = true;
+				goto exit;
 			}
+
+			lcore = &cpu_map.lcores[cpu_map.lcore_count];
+			lcore->socket_id = socket_id;
+			lcore->core_id =
+				cores->Group * EAL_PROCESSOR_GROUP_SIZE + i;
+			cpu_map.lcore_count++;
 		}
+
+		info = (SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *)(
+			(uint8_t *)info + info->Size);
 	}
+
+exit:
+	if (full) {
+		/* Not a fatal error, but important for troubleshooting. */
+		log_early("Enumerated maximum of %u NUMA nodes and %u cores\n",
+			cpu_map.socket_count, cpu_map.lcore_count);
+	}
+
+	free(infos);
+
+	return 0;
 }
 
-/*
- * Check if a cpu is present by the presence of the cpu information for it
- */
 int
 eal_cpu_detected(unsigned int lcore_id)
 {
-	return (lcore_id < wcpu_map.total_procs);
+	return lcore_id < cpu_map.lcore_count;
 }
 
-/*
- * Get CPU socket id for a logical core
- */
 unsigned
 eal_cpu_socket_id(unsigned int lcore_id)
 {
-	return wcpu_map.wlcore_map[lcore_id].socket_id;
+	return cpu_map.lcores[lcore_id].socket_id;
 }
 
-/*
- * Get CPU socket id (NUMA node) for a logical core
- */
 unsigned
 eal_cpu_core_id(unsigned int lcore_id)
 {
-	return wcpu_map.wlcore_map[lcore_id].core_id;
+	return cpu_map.lcores[lcore_id].core_id;
+}
+
+unsigned int
+eal_socket_numa_node(unsigned int socket_id)
+{
+	return cpu_map.sockets[socket_id].node_id;
 }
diff --git a/lib/librte_eal/windows/eal_windows.h b/lib/librte_eal/windows/eal_windows.h
index fadd676b2..f3ed8c37f 100644
--- a/lib/librte_eal/windows/eal_windows.h
+++ b/lib/librte_eal/windows/eal_windows.h
@@ -13,8 +13,11 @@ 
 
 /**
  * Create a map of processors and cores on the system.
+ *
+ * @return
+ *  0 on success, (-1) on failure and rte_errno is set.
  */
-void eal_create_cpu_map(void);
+int eal_create_cpu_map(void);
 
 /**
  * Create a thread.
@@ -26,4 +29,14 @@  void eal_create_cpu_map(void);
  */
 int eal_thread_create(pthread_t *thread);
 
+/**
+ * Get system NUMA node number for a socket ID.
+ *
+ * @param socket_id
+ *  Valid EAL socket ID.
+ * @return
+ *  NUMA node number to use with Win32 API.
+ */
+unsigned int eal_socket_numa_node(unsigned int socket_id);
+
 #endif /* _EAL_WINDOWS_H_ */