Re: [dpdk-dev] [PATCH v9 04/10] eal: implement functions for thread affinity management

On Wed, Jun 09, 2021 at 02:03:57AM +0300, Dmitry Kozlyuk wrote:

2021-06-04 16:44 (UTC-0700), Narcisa Ana Maria Vasile:
[...]

quoted

diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index 6ea1dc2a05..9e74a538c2 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c

@@ -7,7 +7,8 @@
 #include <rte_errno.h>
 #include <rte_debug.h>
 #include <rte_thread.h>
-#include <rte_windows.h>
+
+#include "eal_windows.h"
 
 struct eal_tls_key {
 	DWORD thread_index;

@@ -77,6 +78,130 @@ rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
 	return t1.opaque_id == t2.opaque_id;
 }
 
+static int
+rte_convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
+			       PGROUP_AFFINITY affinity)
+{
+	int ret = 0;
+	PGROUP_AFFINITY cpu_affinity = NULL;
+
+	memset(affinity, 0, sizeof(GROUP_AFFINITY));
+	affinity->Group = (USHORT)-1;
+
+	/* Check that all cpus of the set belong to the same processor group and
+	 * accumulate thread affinity to be applied.
+	 */
+	for (unsigned int cpu_idx = 0; cpu_idx < CPU_SETSIZE; cpu_idx++) {
+		if (!CPU_ISSET(cpu_idx, cpuset))
+			continue;
+
+		cpu_affinity = eal_get_cpu_affinity(cpu_idx);
+
+		if (affinity->Group == (USHORT)-1) {
+			affinity->Group = cpu_affinity->Group;
+		} else if (affinity->Group != cpu_affinity->Group) {
+			ret = EINVAL;
+			goto cleanup;
+		}
+
+		affinity->Mask |= cpu_affinity->Mask;
+	}
+
+	if (affinity->Mask == 0) {
+		ret = EINVAL;
+		goto cleanup;
+	}
+
+cleanup:
+	return ret;
+}

For v5 I asked a question that possibly got lost among other comments.
Repeating the question for convenience:

	Just to be clear: is it a kernel limitation that a thread can only
	run on cores of one processor group, or do we impose it so that API
	is atomic (transactional), i.e. because one of multiple
	SetThreadGroupAffinity() calls may fail and leave thread partially
	affinitized?

The second reason (to ensure full affinitization). I am not aware of a kernel limitation,
but I'll double check with Dmitry as we co-engineered this patch.