[PATCH 0/6] add thread lifetime and attributes API

[PATCH 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

add rte thread lifetime and attributes api. with these api additions
there is now sufficient platform abstracted thread api to remove the
use of pthread in the unit tests.

Tyler Retzlaff (6):
  eal: add thread attributes
  eal: add thread lifetime management
  eal: add basic rte thread ID equal API
  test/threads: add tests for thread lifetime API
  test/threads: add tests for thread attributes API
  test/threads: remove unit test use of pthread

 app/test/test_threads.c         | 130 ++++++++++++++++++++++--
 lib/eal/common/meson.build      |   1 +
 lib/eal/common/rte_thread.c     |  58 +++++++++++
 lib/eal/include/rte_thread.h    | 177 ++++++++++++++++++++++++++++++++
 lib/eal/unix/rte_thread.c       | 108 ++++++++++++++++++++
 lib/eal/version.map             |   8 ++
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 217 ++++++++++++++++++++++++++++++++--------
 8 files changed, 654 insertions(+), 47 deletions(-)
 create mode 100644 lib/eal/common/rte_thread.c

-- 
1.8.3.1

[PATCH 1/6] eal: add thread attributes

[Tyler Retzlaff]

Implement thread attributes for:

    * thread affinity
    * thread priority

Implement functions for managing thread attributes.

  Priority is represented through an enum that allows for two levels:

    * RTE_THREAD_PRIORITY_NORMAL
    * RTE_THREAD_PRIORITY_REALTIME_CRITICAL

  Affinity is described by the rte_cpuset_t type.

An rte_thread_attr_t object can be set to the default values
by calling rte_thread_attr_init().

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/common/meson.build   |  1 +
 lib/eal/common/rte_thread.c  | 52 +++++++++++++++++++++++++
 lib/eal/include/rte_thread.h | 93 ++++++++++++++++++++++++++++++++++++++++++++
 lib/eal/version.map          |  4 ++
 4 files changed, 150 insertions(+)
 create mode 100644 lib/eal/common/rte_thread.c

diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
index 917758c..1e77dba 100644
--- a/lib/eal/common/meson.build
+++ b/lib/eal/common/meson.build
@@ -36,6 +36,7 @@ sources += files(
         'rte_random.c',
         'rte_reciprocal.c',
         'rte_service.c',
+        'rte_thread.c',
         'rte_version.c',
 )
 if is_linux or is_windows
diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
new file mode 100644
index 0000000..10d6652
--- /dev/null
+++ b/lib/eal/common/rte_thread.c
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2022 Microsoft Corporation
+ */
+
+#include <rte_debug.h>
+#include <rte_thread.h>
+
+int
+rte_thread_attr_init(rte_thread_attr_t *attr)
+{
+	RTE_VERIFY(attr != NULL);
+
+	CPU_ZERO(&attr->cpuset);
+	attr->priority = RTE_THREAD_PRIORITY_NORMAL;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	RTE_VERIFY(thread_attr != NULL);
+	RTE_VERIFY(cpuset != NULL);
+
+	thread_attr->cpuset = *cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	RTE_VERIFY(thread_attr != NULL);
+	RTE_VERIFY(cpuset != NULL);
+
+	*cpuset = thread_attr->cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority)
+{
+	RTE_VERIFY(thread_attr != NULL);
+
+	thread_attr->priority = priority;
+
+	return 0;
+}
diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 9e261bf..062308d 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -40,6 +40,18 @@ enum rte_thread_priority {
 	/**< highest thread priority allowed */
 };
 
+#ifdef RTE_HAS_CPUSET
+
+/**
+ * Representation for thread attributes.
+ */
+typedef struct {
+	enum rte_thread_priority priority; /**< thread priority */
+	rte_cpuset_t cpuset; /**< thread affinity */
+} rte_thread_attr_t;
+
+#endif /* RTE_HAS_CPUSET */
+
 /**
  * TLS key type, an opaque pointer.
  */
@@ -63,6 +75,87 @@ enum rte_thread_priority {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
+ * Initialize the attributes of a thread.
+ * These attributes can be passed to the rte_thread_create() function
+ * that will create a new thread and set its attributes according to attr.
+ *
+ * @param attr
+ *   Thread attributes to initialize.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_init(rte_thread_attr_t *attr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the CPU affinity value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which affinity will be updated.
+ *
+ * @param cpuset
+ *   Points to the value of the affinity to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get the value of CPU affinity that is set in the thread attributes pointed
+ * to by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes from which affinity will be retrieved.
+ *
+ * @param cpuset
+ *   Pointer to the memory that will store the affinity.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the thread priority value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which priority will be updated.
+ *
+ * @param priority
+ *   Points to the value of the priority to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Set the affinity of thread 'thread_id' to the cpu set
  * specified by 'cpuset'.
  *
diff --git a/lib/eal/version.map b/lib/eal/version.map
index ab27a4b..72e0e14 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -422,6 +422,10 @@ EXPERIMENTAL {
 	rte_intr_type_set;
 
 	# added in 22.07
+	rte_thread_attr_get_affinity;
+	rte_thread_attr_init;
+	rte_thread_attr_set_affinity;
+	rte_thread_attr_set_priority;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
 	rte_thread_self;
-- 
1.8.3.1

[PATCH 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

The *rte_thread_create()* function can optionally receive an
rte_thread_attr_t object that will cause the thread to be created with
the affinity and priority described by the attributes object. If
no rte_thread_attr_t is passed (parameter is NULL), the default
affinity and priority are used.

On Windows, the function executed by a thread when the thread starts is
represeneted by a function pointer of type DWORD (*func) (void*).
On other platforms, the function pointer is a void* (*func) (void*).

Performing a cast between these two types of function pointers to
uniformize the API on all platforms may result in undefined behavior.
TO fix this issue, a wrapper that respects the signature required by
CreateThread() has been created on Windows.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/include/rte_thread.h    |  65 ++++++++++++
 lib/eal/unix/rte_thread.c       | 108 ++++++++++++++++++++
 lib/eal/version.map             |   3 +
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 217 ++++++++++++++++++++++++++++++++--------
 5 files changed, 354 insertions(+), 41 deletions(-)

diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 062308d..321fb59 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -30,6 +30,8 @@
 	uintptr_t opaque_id; /**< thread identifier */
 } rte_thread_t;
 
+typedef void * (*rte_thread_func) (void *);
+
 /**
  * Thread priority values.
  */
@@ -61,6 +63,69 @@ enum rte_thread_priority {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
+ * Create a new thread that will invoke the 'thread_func' routine.
+ *
+ * @param thread_id
+ *    A pointer that will store the id of the newly created thread.
+ *
+ * @param thread_attr
+ *    Attributes that are used at the creation of the new thread.
+ *
+ * @param thread_func
+ *    The routine that the new thread will invoke when starting execution.
+ *
+ * @param args
+ *    Arguments to be passed to the 'thread_func' routine.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *arg);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Waits for the thread identified by 'thread_id' to terminate
+ *
+ * @param thread_id
+ *    The identifier of the thread.
+ *
+ * @param value_ptr
+ *    Stores the exit status of the thread.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Indicate that the return value of the thread is not needed and
+ * all thread resources should be release when the thread terminates.
+ *
+ * @param thread_id
+ *    The id of the thread to be detached.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_detach(rte_thread_t thread_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Get the id of the calling thread.
  *
  * @return
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index 9126595..19c7b80 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -75,6 +75,114 @@ struct eal_tls_key {
 	return 0;
 }
 
+int
+rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	pthread_attr_t attr;
+	pthread_attr_t *attrp = NULL;
+	struct sched_param param = {
+		.sched_priority = 0,
+	};
+	int policy = SCHED_OTHER;
+
+	if (thread_attr != NULL) {
+		ret = pthread_attr_init(&attr);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_init failed\n");
+			goto cleanup;
+		}
+
+		attrp = &attr;
+
+		/*
+		 * Set the inherit scheduler parameter to explicit,
+		 * otherwise the priority attribute is ignored.
+		 */
+		ret = pthread_attr_setinheritsched(attrp,
+				PTHREAD_EXPLICIT_SCHED);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setinheritsched failed\n");
+			goto cleanup;
+		}
+
+
+		if (thread_attr->priority ==
+				RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+		ret = thread_map_priority_to_os_value(thread_attr->priority,
+				&param.sched_priority, &policy);
+		if (ret != 0)
+			goto cleanup;
+
+		ret = pthread_attr_setschedpolicy(attrp, policy);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedpolicy failed\n");
+			goto cleanup;
+		}
+
+		ret = pthread_attr_setschedparam(attrp, &param);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedparam failed\n");
+			goto cleanup;
+		}
+	}
+
+	ret = pthread_create((pthread_t *)&thread_id->opaque_id, attrp,
+		thread_func, args);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_create failed\n");
+		goto cleanup;
+	}
+
+	if (thread_attr != NULL && CPU_COUNT(&thread_attr->cpuset) > 0) {
+		ret = rte_thread_set_affinity_by_id(*thread_id,
+			&thread_attr->cpuset);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "rte_thread_set_affinity_by_id failed\n");
+			goto cleanup;
+		}
+	}
+
+cleanup:
+	if (attrp != NULL)
+		pthread_attr_destroy(&attr);
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
+{
+	int ret = 0;
+	void *res = NULL;
+	void **pres = NULL;
+
+	if (value_ptr != NULL)
+		pres = &res;
+
+	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
+		return ret;
+	}
+
+	if (value_ptr != NULL && *pres != NULL)
+		*value_ptr = *(unsigned long *)(*pres);
+
+	return 0;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	return pthread_detach((pthread_t)thread_id.opaque_id);
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 72e0e14..22e5c85 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -426,8 +426,11 @@ EXPERIMENTAL {
 	rte_thread_attr_init;
 	rte_thread_attr_set_affinity;
 	rte_thread_attr_set_priority;
+	rte_thread_create;
+	rte_thread_detach;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
+	rte_thread_join;
 	rte_thread_self;
 	rte_thread_set_affinity_by_id;
 	rte_thread_set_priority;
diff --git a/lib/eal/windows/include/sched.h b/lib/eal/windows/include/sched.h
index bc31cc8..912fed1 100644
--- a/lib/eal/windows/include/sched.h
+++ b/lib/eal/windows/include/sched.h
@@ -44,7 +44,7 @@
 	(1LL << _WHICH_BIT(b))) != 0LL)
 
 static inline int
-count_cpu(rte_cpuset_t *s)
+count_cpu(const rte_cpuset_t *s)
 {
 	unsigned int _i;
 	int count = 0;
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index 0771525..b5f2b04 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -13,6 +13,11 @@ struct eal_tls_key {
 	DWORD thread_index;
 };
 
+struct thread_routine_ctx {
+	rte_thread_func thread_func;
+	void *routine_args;
+};
+
 /* Translates the most common error codes related to threads */
 static int
 thread_translate_win32_error(DWORD error)
@@ -114,6 +119,178 @@ struct eal_tls_key {
 	return 0;
 }
 
+static int
+convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
+		PGROUP_AFFINITY affinity)
+{
+	int ret = 0;
+	PGROUP_AFFINITY cpu_affinity = NULL;
+	unsigned int cpu_idx;
+
+	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 (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) {
+			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+
+		affinity->Mask |= cpu_affinity->Mask;
+	}
+
+	if (affinity->Mask == 0) {
+		ret = EINVAL;
+		goto cleanup;
+	}
+
+cleanup:
+	return ret;
+}
+
+
+static DWORD
+thread_func_wrapper(void *args)
+{
+	struct thread_routine_ctx *pctx = args;
+	struct thread_routine_ctx ctx;
+
+	ctx.thread_func = pctx->thread_func;
+	ctx.routine_args = pctx->routine_args;
+
+	free(pctx);
+
+	return (DWORD)(uintptr_t)ctx.thread_func(ctx.routine_args);
+}
+
+int
+rte_thread_create(rte_thread_t *thread_id,
+		  const rte_thread_attr_t *thread_attr,
+		  rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	DWORD tid;
+	HANDLE thread_handle = NULL;
+	GROUP_AFFINITY thread_affinity;
+	struct thread_routine_ctx *ctx = NULL;
+
+	ctx = calloc(1, sizeof(*ctx));
+	if (ctx == NULL) {
+		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
+		ret = ENOMEM;
+		goto cleanup;
+	}
+	ctx->routine_args = args;
+	ctx->thread_func = thread_func;
+
+	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
+		CREATE_SUSPENDED, &tid);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("CreateThread()");
+		free(ctx);
+		goto cleanup;
+	}
+	thread_id->opaque_id = tid;
+
+	if (thread_attr != NULL) {
+		if (CPU_COUNT(&thread_attr->cpuset) > 0) {
+			ret = convert_cpuset_to_affinity(
+							&thread_attr->cpuset,
+							&thread_affinity
+							);
+			if (ret != 0) {
+				RTE_LOG(DEBUG, EAL, "Unable to convert cpuset to thread affinity\n");
+				goto cleanup;
+			}
+
+			if (!SetThreadGroupAffinity(thread_handle,
+						    &thread_affinity, NULL)) {
+				ret = thread_log_last_error("SetThreadGroupAffinity()");
+				goto cleanup;
+			}
+		}
+		ret = rte_thread_set_priority(*thread_id,
+				thread_attr->priority);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "Unable to set thread priority\n");
+			goto cleanup;
+		}
+	}
+
+	if (ResumeThread(thread_handle) == (DWORD)-1) {
+		ret = thread_log_last_error("ResumeThread()");
+		goto cleanup;
+	}
+
+cleanup:
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
+{
+	HANDLE thread_handle;
+	DWORD result;
+	DWORD exit_code = 0;
+	BOOL err;
+	int ret = 0;
+
+	thread_handle = OpenThread(SYNCHRONIZE | THREAD_QUERY_INFORMATION,
+				   FALSE, thread_id.opaque_id);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("OpenThread()");
+		goto cleanup;
+	}
+
+	result = WaitForSingleObject(thread_handle, INFINITE);
+	if (result != WAIT_OBJECT_0) {
+		ret = thread_log_last_error("WaitForSingleObject()");
+		goto cleanup;
+	}
+
+	if (value_ptr != NULL) {
+		err = GetExitCodeThread(thread_handle, &exit_code);
+		if (err == 0) {
+			ret = thread_log_last_error("GetExitCodeThread()");
+			goto cleanup;
+		}
+		*value_ptr = exit_code;
+	}
+
+cleanup:
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	/* No resources that need to be released. */
+	RTE_SET_USED(thread_id);
+
+	return 0;
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
@@ -278,46 +455,6 @@ struct eal_tls_key {
 	return output;
 }
 
-static int
-convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
-		PGROUP_AFFINITY affinity)
-{
-	int ret = 0;
-	PGROUP_AFFINITY cpu_affinity = NULL;
-	unsigned int cpu_idx;
-
-	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 (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) {
-			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
-			ret = ENOTSUP;
-			goto cleanup;
-		}
-
-		affinity->Mask |= cpu_affinity->Mask;
-	}
-
-	if (affinity->Mask == 0) {
-		ret = EINVAL;
-		goto cleanup;
-	}
-
-cleanup:
-	return ret;
-}
-
 int
 rte_thread_set_affinity_by_id(rte_thread_t thread_id,
 		const rte_cpuset_t *cpuset)
-- 
1.8.3.1

[PATCH 3/6] eal: add basic rte thread ID equal API

[Tyler Retzlaff]

Add rte_thread_equal() that tests if two rte_thread_id are equal.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/common/rte_thread.c  |  6 ++++++
 lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
 lib/eal/version.map          |  1 +
 3 files changed, 26 insertions(+)

diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
index 10d6652..21ed042 100644
--- a/lib/eal/common/rte_thread.c
+++ b/lib/eal/common/rte_thread.c
@@ -6,6 +6,12 @@
 #include <rte_thread.h>
 
 int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return t1.opaque_id == t2.opaque_id;
+}
+
+int
 rte_thread_attr_init(rte_thread_attr_t *attr)
 {
 	RTE_VERIFY(attr != NULL);
diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 321fb59..32ab745 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -134,6 +134,25 @@ int rte_thread_create(rte_thread_t *thread_id,
 __rte_experimental
 rte_thread_t rte_thread_self(void);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Check if 2 thread ids are equal.
+ *
+ * @param t1
+ *   First thread id.
+ *
+ * @param t2
+ *   Second thread id.
+ *
+ * @return
+ *   If the ids are equal, return nonzero.
+ *   Otherwise, return 0.
+ */
+__rte_experimental
+int rte_thread_equal(rte_thread_t t1, rte_thread_t t2);
+
 #ifdef RTE_HAS_CPUSET
 
 /**
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 22e5c85..4a52484 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -428,6 +428,7 @@ EXPERIMENTAL {
 	rte_thread_attr_set_priority;
 	rte_thread_create;
 	rte_thread_detach;
+	rte_thread_equal;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
 	rte_thread_join;
-- 
1.8.3.1

Re: [PATCH 3/6] eal: add basic rte thread ID equal API

[Konstantin Ananyev]

09/06/2022 14:58, Tyler Retzlaff пишет:
> Add rte_thread_equal() that tests if two rte_thread_id are equal.
> 
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>   lib/eal/common/rte_thread.c  |  6 ++++++
>   lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
>   lib/eal/version.map          |  1 +
>   3 files changed, 26 insertions(+)
> 
> diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
> index 10d6652..21ed042 100644
> --- a/lib/eal/common/rte_thread.c
> +++ b/lib/eal/common/rte_thread.c
> @@ -6,6 +6,12 @@
>   #include <rte_thread.h>
>   
>   int
> +rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
> +{
> +	return t1.opaque_id == t2.opaque_id;

for posix systems, why not:
return pthread_equal(t1.opaque_id, t2.opaque_id);
?


> +}
> +
> +int
>   rte_thread_attr_init(rte_thread_attr_t *attr)
>   {
>   	RTE_VERIFY(attr != NULL);
> diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> index 321fb59..32ab745 100644
> --- a/lib/eal/include/rte_thread.h
> +++ b/lib/eal/include/rte_thread.h
> @@ -134,6 +134,25 @@ int rte_thread_create(rte_thread_t *thread_id,
>   __rte_experimental
>   rte_thread_t rte_thread_self(void);
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Check if 2 thread ids are equal.
> + *
> + * @param t1
> + *   First thread id.
> + *
> + * @param t2
> + *   Second thread id.
> + *
> + * @return
> + *   If the ids are equal, return nonzero.
> + *   Otherwise, return 0.
> + */
> +__rte_experimental
> +int rte_thread_equal(rte_thread_t t1, rte_thread_t t2);
> +
>   #ifdef RTE_HAS_CPUSET
>   
>   /**
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index 22e5c85..4a52484 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -428,6 +428,7 @@ EXPERIMENTAL {
>   	rte_thread_attr_set_priority;
>   	rte_thread_create;
>   	rte_thread_detach;
> +	rte_thread_equal;
>   	rte_thread_get_affinity_by_id;
>   	rte_thread_get_priority;
>   	rte_thread_join;

Re: [PATCH 3/6] eal: add basic rte thread ID equal API

[Tyler Retzlaff]

On Thu, Jun 09, 2022 at 11:24:12PM +0100, Konstantin Ananyev wrote:
> 09/06/2022 14:58, Tyler Retzlaff пишет:
> >Add rte_thread_equal() that tests if two rte_thread_id are equal.
> >
> >Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> >Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >---
> >  lib/eal/common/rte_thread.c  |  6 ++++++
> >  lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
> >  lib/eal/version.map          |  1 +
> >  3 files changed, 26 insertions(+)
> >
> >diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
> >index 10d6652..21ed042 100644
> >--- a/lib/eal/common/rte_thread.c
> >+++ b/lib/eal/common/rte_thread.c
> >@@ -6,6 +6,12 @@
> >  #include <rte_thread.h>
> >  int
> >+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
> >+{
> >+	return t1.opaque_id == t2.opaque_id;
> 
> for posix systems, why not:
> return pthread_equal(t1.opaque_id, t2.opaque_id);

because it would require 2 implementations when this works for both
windows and posix platforms. (less code to maintain, no functional
difference).

Re: [PATCH 3/6] eal: add basic rte thread ID equal API

[Konstantin Ananyev]

>>> Add rte_thread_equal() that tests if two rte_thread_id are equal.
>>>
>>> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
>>> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>>> ---
>>>   lib/eal/common/rte_thread.c  |  6 ++++++
>>>   lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
>>>   lib/eal/version.map          |  1 +
>>>   3 files changed, 26 insertions(+)
>>>
>>> diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
>>> index 10d6652..21ed042 100644
>>> --- a/lib/eal/common/rte_thread.c
>>> +++ b/lib/eal/common/rte_thread.c
>>> @@ -6,6 +6,12 @@
>>>   #include <rte_thread.h>
>>>   int
>>> +rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
>>> +{
>>> +	return t1.opaque_id == t2.opaque_id;
>>
>> for posix systems, why not:
>> return pthread_equal(t1.opaque_id, t2.opaque_id);
> 
> because it would require 2 implementations 

We already have plenty of such cases for rte_thread implementation.
Why it became a problem here?

when this works for both
> windows and posix platforms. (less code to maintain, no functional
> difference).
> 

Well posix insists that the only safe way for applications to
directly compare two pthread_t values is to call pthread_equal().
So I'd suggest we do what is recommended.

Re: [PATCH 3/6] eal: add basic rte thread ID equal API

[Tyler Retzlaff]

On Sat, Jun 11, 2022 at 01:25:56PM +0100, Konstantin Ananyev wrote:
> 
> >>>Add rte_thread_equal() that tests if two rte_thread_id are equal.
> >>>
> >>>Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> >>>Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >>>---
> >>>  lib/eal/common/rte_thread.c  |  6 ++++++
> >>>  lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
> >>>  lib/eal/version.map          |  1 +
> >>>  3 files changed, 26 insertions(+)
> >>>
> >>>diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
> >>>index 10d6652..21ed042 100644
> >>>--- a/lib/eal/common/rte_thread.c
> >>>+++ b/lib/eal/common/rte_thread.c
> >>>@@ -6,6 +6,12 @@
> >>>  #include <rte_thread.h>
> >>>  int
> >>>+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
> >>>+{
> >>>+	return t1.opaque_id == t2.opaque_id;
> >>
> >>for posix systems, why not:
> >>return pthread_equal(t1.opaque_id, t2.opaque_id);
> >
> >because it would require 2 implementations
> 
> We already have plenty of such cases for rte_thread implementation.
> Why it became a problem here?
> 
> when this works for both
> >windows and posix platforms. (less code to maintain, no functional
> >difference).
> >
> 
> Well posix insists that the only safe way for applications to
> directly compare two pthread_t values is to call pthread_equal().
> So I'd suggest we do what is recommended.

agreed, will provide a v2 that uses pthread_equal().

Re: [PATCH 3/6] eal: add basic rte thread ID equal API

[fengchengwen]

Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2022/6/9 21:58, Tyler Retzlaff wrote:
> Add rte_thread_equal() that tests if two rte_thread_id are equal.
> 
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  lib/eal/common/rte_thread.c  |  6 ++++++
>  lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
>  lib/eal/version.map          |  1 +
>  3 files changed, 26 insertions(+)
> 
> diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
> index 10d6652..21ed042 100644
> --- a/lib/eal/common/rte_thread.c
> +++ b/lib/eal/common/rte_thread.c
> @@ -6,6 +6,12 @@
>  #include <rte_thread.h>
>  
>  int
> +rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
> +{
> +	return t1.opaque_id == t2.opaque_id;
> +}
> +
> +int
>  rte_thread_attr_init(rte_thread_attr_t *attr)
>  {
>  	RTE_VERIFY(attr != NULL);
> diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> index 321fb59..32ab745 100644
> --- a/lib/eal/include/rte_thread.h
> +++ b/lib/eal/include/rte_thread.h
> @@ -134,6 +134,25 @@ int rte_thread_create(rte_thread_t *thread_id,
>  __rte_experimental
>  rte_thread_t rte_thread_self(void);
>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Check if 2 thread ids are equal.
> + *
> + * @param t1
> + *   First thread id.
> + *
> + * @param t2
> + *   Second thread id.
> + *
> + * @return
> + *   If the ids are equal, return nonzero.
> + *   Otherwise, return 0.
> + */
> +__rte_experimental
> +int rte_thread_equal(rte_thread_t t1, rte_thread_t t2);
> +
>  #ifdef RTE_HAS_CPUSET
>  
>  /**
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index 22e5c85..4a52484 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -428,6 +428,7 @@ EXPERIMENTAL {
>  	rte_thread_attr_set_priority;
>  	rte_thread_create;
>  	rte_thread_detach;
> +	rte_thread_equal;
>  	rte_thread_get_affinity_by_id;
>  	rte_thread_get_priority;
>  	rte_thread_join;
> 

[PATCH 4/6] test/threads: add tests for thread lifetime API

[Tyler Retzlaff]

test basic functionality and demonstrate use of following thread
lifetime api.

    * rte_thread_create
    * rte_thread_detach
    * rte_thread_join

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index b9d8b4e..9a30af5 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -27,6 +27,54 @@
 }
 
 static int
+test_thread_create_join(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, &thread_main_id) == 0,
+		"Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_join(thread_id, NULL) == 0,
+		"Failed to join thread.");
+
+	return 0;
+}
+
+static int
+test_thread_create_detach(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main,
+		&thread_main_id) == 0, "Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_detach(thread_id) == 0,
+		"Failed to detach thread.");
+
+	return 0;
+}
+
+static int
 test_thread_priority(void)
 {
 	pthread_t id;
@@ -123,6 +171,8 @@
 	.setup = NULL,
 	.teardown = NULL,
 	.unit_test_cases = {
+		TEST_CASE(test_thread_create_join),
+		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
 		TEST_CASES_END()
-- 
1.8.3.1

[PATCH 5/6] test/threads: add tests for thread attributes API

[Tyler Retzlaff]

test basic functionality and demonstrate use of following thread
attributes api. additionally, test attributes are processed when
supplied to rte_thread_create().

    * rte_thread_attr_init
    * rte_thread_attr_set_affinity
    * rte_thread_attr_get_affinity
    * rte_thread_attr_set_priority

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 9a30af5..92a149a 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -17,7 +17,9 @@
 static void *
 thread_main(void *arg)
 {
-	*(rte_thread_t *)arg = rte_thread_self();
+	if (arg != NULL)
+		*(rte_thread_t *)arg = rte_thread_self();
+
 	__atomic_store_n(&thread_id_ready, 1, __ATOMIC_RELEASE);
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 1)
@@ -166,6 +168,73 @@
 	return 0;
 }
 
+static int
+test_thread_attributes_affinity(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	rte_cpuset_t cpuset0;
+	rte_cpuset_t cpuset1;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+
+	CPU_ZERO(&cpuset0);
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(rte_thread_self(), &cpuset0) == 0,
+		"Failed to get thread affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_set_affinity(&attr, &cpuset0) == 0,
+		"Failed to set thread attributes affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_get_affinity(&attr, &cpuset1) == 0,
+		"Failed to get thread attributes affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Affinity should be stable");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes affinity thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
+		"Failed to get attributes thread affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Failed to apply affinity attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
+static int
+test_thread_attributes_priority(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	enum rte_thread_priority priority;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+	RTE_TEST_ASSERT(rte_thread_attr_set_priority(&attr, RTE_THREAD_PRIORITY_NORMAL) == 0,
+		"Failed to set thread attributes priority");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes priority thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_priority(thread_id, &priority) == 0,
+		"Failed to get thread priority");
+	RTE_TEST_ASSERT(priority == RTE_THREAD_PRIORITY_NORMAL,
+		"Failed to apply priority attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
 static struct unit_test_suite threads_test_suite = {
 	.suite_name = "threads autotest",
 	.setup = NULL,
@@ -175,6 +244,8 @@
 		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
+		TEST_CASE(test_thread_attributes_affinity),
+		TEST_CASE(test_thread_attributes_priority),
 		TEST_CASES_END()
 	}
 };
-- 
1.8.3.1

[PATCH 6/6] test/threads: remove unit test use of pthread

[Tyler Retzlaff]

now that rte_thread provides thread lifetime functions stop using
pthread in unit tests.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 92a149a..034f82d 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -3,7 +3,6 @@
  */
 
 #include <string.h>
-#include <pthread.h>
 
 #include <rte_thread.h>
 #include <rte_debug.h>
@@ -79,12 +78,11 @@
 static int
 test_thread_priority(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	enum rte_thread_priority priority;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
@@ -131,13 +129,12 @@
 static int
 test_thread_affinity(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	rte_cpuset_t cpuset0;
 	rte_cpuset_t cpuset1;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
-- 
1.8.3.1

[PATCH v2 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

add rte thread lifetime and attributes api. with these api additions
there is now sufficient platform abstracted thread api to remove the
use of pthread in the unit tests.

v2:
  * split implementation of rte_thread_equal for windows / posix
    and use pthread_equal for posix platforms.
  * remove parameter validation assertions and instead return
    EINVAL for mandatory pointers to type that are NULL.
  * correct doxygen comment parameter name args -> arg

Tyler Retzlaff (6):
  eal: add thread attributes
  eal: add thread lifetime management
  eal: add basic rte thread ID equal API
  test/threads: add tests for thread lifetime API
  test/threads: add tests for thread attributes API
  test/threads: remove unit test use of pthread

 app/test/test_threads.c         | 130 +++++++++++++++++++++--
 lib/eal/common/meson.build      |   1 +
 lib/eal/common/rte_thread.c     |  60 +++++++++++
 lib/eal/include/rte_thread.h    | 177 +++++++++++++++++++++++++++++++
 lib/eal/unix/rte_thread.c       | 114 ++++++++++++++++++++
 lib/eal/version.map             |   8 ++
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 223 +++++++++++++++++++++++++++++++++-------
 8 files changed, 668 insertions(+), 47 deletions(-)
 create mode 100644 lib/eal/common/rte_thread.c

-- 
1.8.3.1

[PATCH v2 1/6] eal: add thread attributes

[Tyler Retzlaff]

Implement thread attributes for:

    * thread affinity
    * thread priority

Implement functions for managing thread attributes.

  Priority is represented through an enum that allows for two levels:

    * RTE_THREAD_PRIORITY_NORMAL
    * RTE_THREAD_PRIORITY_REALTIME_CRITICAL

  Affinity is described by the rte_cpuset_t type.

An rte_thread_attr_t object can be set to the default values
by calling rte_thread_attr_init().

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/common/meson.build   |  1 +
 lib/eal/common/rte_thread.c  | 60 ++++++++++++++++++++++++++++
 lib/eal/include/rte_thread.h | 93 ++++++++++++++++++++++++++++++++++++++++++++
 lib/eal/version.map          |  4 ++
 4 files changed, 158 insertions(+)
 create mode 100644 lib/eal/common/rte_thread.c

diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
index 917758c..1e77dba 100644
--- a/lib/eal/common/meson.build
+++ b/lib/eal/common/meson.build
@@ -36,6 +36,7 @@ sources += files(
         'rte_random.c',
         'rte_reciprocal.c',
         'rte_service.c',
+        'rte_thread.c',
         'rte_version.c',
 )
 if is_linux or is_windows
diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
new file mode 100644
index 0000000..d204cc5
--- /dev/null
+++ b/lib/eal/common/rte_thread.c
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2022 Microsoft Corporation
+ */
+
+#include <rte_debug.h>
+#include <rte_thread.h>
+
+int
+rte_thread_attr_init(rte_thread_attr_t *attr)
+{
+	if (attr == NULL)
+		return EINVAL;
+
+	CPU_ZERO(&attr->cpuset);
+	attr->priority = RTE_THREAD_PRIORITY_NORMAL;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	if (cpuset == NULL)
+		return EINVAL;
+
+	thread_attr->cpuset = *cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	if (cpuset == NULL)
+		return EINVAL;
+
+	*cpuset = thread_attr->cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	thread_attr->priority = priority;
+
+	return 0;
+}
diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 9e261bf..062308d 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -40,6 +40,18 @@ enum rte_thread_priority {
 	/**< highest thread priority allowed */
 };
 
+#ifdef RTE_HAS_CPUSET
+
+/**
+ * Representation for thread attributes.
+ */
+typedef struct {
+	enum rte_thread_priority priority; /**< thread priority */
+	rte_cpuset_t cpuset; /**< thread affinity */
+} rte_thread_attr_t;
+
+#endif /* RTE_HAS_CPUSET */
+
 /**
  * TLS key type, an opaque pointer.
  */
@@ -63,6 +75,87 @@ enum rte_thread_priority {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
+ * Initialize the attributes of a thread.
+ * These attributes can be passed to the rte_thread_create() function
+ * that will create a new thread and set its attributes according to attr.
+ *
+ * @param attr
+ *   Thread attributes to initialize.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_init(rte_thread_attr_t *attr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the CPU affinity value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which affinity will be updated.
+ *
+ * @param cpuset
+ *   Points to the value of the affinity to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get the value of CPU affinity that is set in the thread attributes pointed
+ * to by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes from which affinity will be retrieved.
+ *
+ * @param cpuset
+ *   Pointer to the memory that will store the affinity.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the thread priority value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which priority will be updated.
+ *
+ * @param priority
+ *   Points to the value of the priority to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Set the affinity of thread 'thread_id' to the cpu set
  * specified by 'cpuset'.
  *
diff --git a/lib/eal/version.map b/lib/eal/version.map
index ab27a4b..72e0e14 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -422,6 +422,10 @@ EXPERIMENTAL {
 	rte_intr_type_set;
 
 	# added in 22.07
+	rte_thread_attr_get_affinity;
+	rte_thread_attr_init;
+	rte_thread_attr_set_affinity;
+	rte_thread_attr_set_priority;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
 	rte_thread_self;
-- 
1.8.3.1

[PATCH v2 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

The *rte_thread_create()* function can optionally receive an
rte_thread_attr_t object that will cause the thread to be created with
the affinity and priority described by the attributes object. If
no rte_thread_attr_t is passed (parameter is NULL), the default
affinity and priority are used.

On Windows, the function executed by a thread when the thread starts is
represeneted by a function pointer of type DWORD (*func) (void*).
On other platforms, the function pointer is a void* (*func) (void*).

Performing a cast between these two types of function pointers to
uniformize the API on all platforms may result in undefined behavior.
TO fix this issue, a wrapper that respects the signature required by
CreateThread() has been created on Windows.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/include/rte_thread.h    |  65 ++++++++++++
 lib/eal/unix/rte_thread.c       | 108 ++++++++++++++++++++
 lib/eal/version.map             |   3 +
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 217 ++++++++++++++++++++++++++++++++--------
 5 files changed, 354 insertions(+), 41 deletions(-)

diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 062308d..c27e580 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -30,6 +30,8 @@
 	uintptr_t opaque_id; /**< thread identifier */
 } rte_thread_t;
 
+typedef void * (*rte_thread_func) (void *);
+
 /**
  * Thread priority values.
  */
@@ -61,6 +63,69 @@ enum rte_thread_priority {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
+ * Create a new thread that will invoke the 'thread_func' routine.
+ *
+ * @param thread_id
+ *    A pointer that will store the id of the newly created thread.
+ *
+ * @param thread_attr
+ *    Attributes that are used at the creation of the new thread.
+ *
+ * @param thread_func
+ *    The routine that the new thread will invoke when starting execution.
+ *
+ * @param arg
+ *    Argument to be passed to the 'thread_func' routine.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *arg);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Waits for the thread identified by 'thread_id' to terminate
+ *
+ * @param thread_id
+ *    The identifier of the thread.
+ *
+ * @param value_ptr
+ *    Stores the exit status of the thread.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Indicate that the return value of the thread is not needed and
+ * all thread resources should be release when the thread terminates.
+ *
+ * @param thread_id
+ *    The id of the thread to be detached.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_detach(rte_thread_t thread_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Get the id of the calling thread.
  *
  * @return
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index 9126595..19c7b80 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -75,6 +75,114 @@ struct eal_tls_key {
 	return 0;
 }
 
+int
+rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	pthread_attr_t attr;
+	pthread_attr_t *attrp = NULL;
+	struct sched_param param = {
+		.sched_priority = 0,
+	};
+	int policy = SCHED_OTHER;
+
+	if (thread_attr != NULL) {
+		ret = pthread_attr_init(&attr);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_init failed\n");
+			goto cleanup;
+		}
+
+		attrp = &attr;
+
+		/*
+		 * Set the inherit scheduler parameter to explicit,
+		 * otherwise the priority attribute is ignored.
+		 */
+		ret = pthread_attr_setinheritsched(attrp,
+				PTHREAD_EXPLICIT_SCHED);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setinheritsched failed\n");
+			goto cleanup;
+		}
+
+
+		if (thread_attr->priority ==
+				RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+		ret = thread_map_priority_to_os_value(thread_attr->priority,
+				&param.sched_priority, &policy);
+		if (ret != 0)
+			goto cleanup;
+
+		ret = pthread_attr_setschedpolicy(attrp, policy);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedpolicy failed\n");
+			goto cleanup;
+		}
+
+		ret = pthread_attr_setschedparam(attrp, &param);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedparam failed\n");
+			goto cleanup;
+		}
+	}
+
+	ret = pthread_create((pthread_t *)&thread_id->opaque_id, attrp,
+		thread_func, args);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_create failed\n");
+		goto cleanup;
+	}
+
+	if (thread_attr != NULL && CPU_COUNT(&thread_attr->cpuset) > 0) {
+		ret = rte_thread_set_affinity_by_id(*thread_id,
+			&thread_attr->cpuset);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "rte_thread_set_affinity_by_id failed\n");
+			goto cleanup;
+		}
+	}
+
+cleanup:
+	if (attrp != NULL)
+		pthread_attr_destroy(&attr);
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
+{
+	int ret = 0;
+	void *res = NULL;
+	void **pres = NULL;
+
+	if (value_ptr != NULL)
+		pres = &res;
+
+	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
+		return ret;
+	}
+
+	if (value_ptr != NULL && *pres != NULL)
+		*value_ptr = *(unsigned long *)(*pres);
+
+	return 0;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	return pthread_detach((pthread_t)thread_id.opaque_id);
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 72e0e14..22e5c85 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -426,8 +426,11 @@ EXPERIMENTAL {
 	rte_thread_attr_init;
 	rte_thread_attr_set_affinity;
 	rte_thread_attr_set_priority;
+	rte_thread_create;
+	rte_thread_detach;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
+	rte_thread_join;
 	rte_thread_self;
 	rte_thread_set_affinity_by_id;
 	rte_thread_set_priority;
diff --git a/lib/eal/windows/include/sched.h b/lib/eal/windows/include/sched.h
index bc31cc8..912fed1 100644
--- a/lib/eal/windows/include/sched.h
+++ b/lib/eal/windows/include/sched.h
@@ -44,7 +44,7 @@
 	(1LL << _WHICH_BIT(b))) != 0LL)
 
 static inline int
-count_cpu(rte_cpuset_t *s)
+count_cpu(const rte_cpuset_t *s)
 {
 	unsigned int _i;
 	int count = 0;
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index 0771525..b5f2b04 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -13,6 +13,11 @@ struct eal_tls_key {
 	DWORD thread_index;
 };
 
+struct thread_routine_ctx {
+	rte_thread_func thread_func;
+	void *routine_args;
+};
+
 /* Translates the most common error codes related to threads */
 static int
 thread_translate_win32_error(DWORD error)
@@ -114,6 +119,178 @@ struct eal_tls_key {
 	return 0;
 }
 
+static int
+convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
+		PGROUP_AFFINITY affinity)
+{
+	int ret = 0;
+	PGROUP_AFFINITY cpu_affinity = NULL;
+	unsigned int cpu_idx;
+
+	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 (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) {
+			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+
+		affinity->Mask |= cpu_affinity->Mask;
+	}
+
+	if (affinity->Mask == 0) {
+		ret = EINVAL;
+		goto cleanup;
+	}
+
+cleanup:
+	return ret;
+}
+
+
+static DWORD
+thread_func_wrapper(void *args)
+{
+	struct thread_routine_ctx *pctx = args;
+	struct thread_routine_ctx ctx;
+
+	ctx.thread_func = pctx->thread_func;
+	ctx.routine_args = pctx->routine_args;
+
+	free(pctx);
+
+	return (DWORD)(uintptr_t)ctx.thread_func(ctx.routine_args);
+}
+
+int
+rte_thread_create(rte_thread_t *thread_id,
+		  const rte_thread_attr_t *thread_attr,
+		  rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	DWORD tid;
+	HANDLE thread_handle = NULL;
+	GROUP_AFFINITY thread_affinity;
+	struct thread_routine_ctx *ctx = NULL;
+
+	ctx = calloc(1, sizeof(*ctx));
+	if (ctx == NULL) {
+		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
+		ret = ENOMEM;
+		goto cleanup;
+	}
+	ctx->routine_args = args;
+	ctx->thread_func = thread_func;
+
+	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
+		CREATE_SUSPENDED, &tid);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("CreateThread()");
+		free(ctx);
+		goto cleanup;
+	}
+	thread_id->opaque_id = tid;
+
+	if (thread_attr != NULL) {
+		if (CPU_COUNT(&thread_attr->cpuset) > 0) {
+			ret = convert_cpuset_to_affinity(
+							&thread_attr->cpuset,
+							&thread_affinity
+							);
+			if (ret != 0) {
+				RTE_LOG(DEBUG, EAL, "Unable to convert cpuset to thread affinity\n");
+				goto cleanup;
+			}
+
+			if (!SetThreadGroupAffinity(thread_handle,
+						    &thread_affinity, NULL)) {
+				ret = thread_log_last_error("SetThreadGroupAffinity()");
+				goto cleanup;
+			}
+		}
+		ret = rte_thread_set_priority(*thread_id,
+				thread_attr->priority);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "Unable to set thread priority\n");
+			goto cleanup;
+		}
+	}
+
+	if (ResumeThread(thread_handle) == (DWORD)-1) {
+		ret = thread_log_last_error("ResumeThread()");
+		goto cleanup;
+	}
+
+cleanup:
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
+{
+	HANDLE thread_handle;
+	DWORD result;
+	DWORD exit_code = 0;
+	BOOL err;
+	int ret = 0;
+
+	thread_handle = OpenThread(SYNCHRONIZE | THREAD_QUERY_INFORMATION,
+				   FALSE, thread_id.opaque_id);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("OpenThread()");
+		goto cleanup;
+	}
+
+	result = WaitForSingleObject(thread_handle, INFINITE);
+	if (result != WAIT_OBJECT_0) {
+		ret = thread_log_last_error("WaitForSingleObject()");
+		goto cleanup;
+	}
+
+	if (value_ptr != NULL) {
+		err = GetExitCodeThread(thread_handle, &exit_code);
+		if (err == 0) {
+			ret = thread_log_last_error("GetExitCodeThread()");
+			goto cleanup;
+		}
+		*value_ptr = exit_code;
+	}
+
+cleanup:
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	/* No resources that need to be released. */
+	RTE_SET_USED(thread_id);
+
+	return 0;
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
@@ -278,46 +455,6 @@ struct eal_tls_key {
 	return output;
 }
 
-static int
-convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
-		PGROUP_AFFINITY affinity)
-{
-	int ret = 0;
-	PGROUP_AFFINITY cpu_affinity = NULL;
-	unsigned int cpu_idx;
-
-	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 (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) {
-			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
-			ret = ENOTSUP;
-			goto cleanup;
-		}
-
-		affinity->Mask |= cpu_affinity->Mask;
-	}
-
-	if (affinity->Mask == 0) {
-		ret = EINVAL;
-		goto cleanup;
-	}
-
-cleanup:
-	return ret;
-}
-
 int
 rte_thread_set_affinity_by_id(rte_thread_t thread_id,
 		const rte_cpuset_t *cpuset)
-- 
1.8.3.1

Re: [PATCH v2 2/6] eal: add thread lifetime management

[Dmitry Kozlyuk]

2022-06-14 16:47 (UTC-0700), Tyler Retzlaff:
> On Windows, the function executed by a thread when the thread starts is
> represeneted by a function pointer of type DWORD (*func) (void*).
> On other platforms, the function pointer is a void* (*func) (void*).
> 
> Performing a cast between these two types of function pointers to
> uniformize the API on all platforms may result in undefined behavior.
> TO fix this issue, a wrapper that respects the signature required by
> CreateThread() has been created on Windows.

The interface issue is still there:
`rte_thread_func` allows the thread routine to have a pointer-sized result.
`rte_thread_join()` allows to obtain this value as `unsigned long`,
which is pointer-sized on 32-bit platforms
and less than that on 64-bit platforms.
This can lead to issues when developers assume they can return a pointer
and this works on 32 bits, but doesn't work on 64 bits.
If you want to keep API as close to phtread as possible,
the limitation must be at least clearly documented in Doxygen
(`rte_thread_func` is undocumented BTW).
I also suggest using `uint32_t` instead of `unsigned long`
precisely to avoid "is it pointer-sized?" doubts.
(I don't see much benefit in keeping pthread-like signature.
When moving from pthread to rte_thread,
it is as trivial to change the thread routine return type.)

> +int
> +rte_thread_create(rte_thread_t *thread_id,
> +		const rte_thread_attr_t *thread_attr,
> +		rte_thread_func thread_func, void *args)
> +{
> [...]
> +		if (thread_attr->priority ==
> +				RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
> +			ret = ENOTSUP;
> +			goto cleanup;
> +		}
> +		ret = thread_map_priority_to_os_value(thread_attr->priority,
> +				&param.sched_priority, &policy);
> +		if (ret != 0)
> +			goto cleanup;

thread_map_priority_to_os_value() already checks for unsupported values,
why not let it do this particular check?

> +int
> +rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
> +{
> +	int ret = 0;
> +	void *res = NULL;
> +	void **pres = NULL;
> +
> +	if (value_ptr != NULL)
> +		pres = &res;
> +
> +	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
> +	if (ret != 0) {
> +		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
> +		return ret;
> +	}
> +
> +	if (value_ptr != NULL && *pres != NULL)
> +		*value_ptr = *(unsigned long *)(*pres);
> +
> +	return 0;
> +}

What makes *pres == NULL special?

> +static DWORD
> +thread_func_wrapper(void *args)
> +{
> +	struct thread_routine_ctx *pctx = args;
> +	struct thread_routine_ctx ctx;
> +
> +	ctx.thread_func = pctx->thread_func;
> +	ctx.routine_args = pctx->routine_args;

ctx = *pctx?

> +
> +	free(pctx);
> +
> +	return (DWORD)(uintptr_t)ctx.thread_func(ctx.routine_args);
> +}
> +
> +int
> +rte_thread_create(rte_thread_t *thread_id,
> +		  const rte_thread_attr_t *thread_attr,
> +		  rte_thread_func thread_func, void *args)
> +{
> +	int ret = 0;
> +	DWORD tid;
> +	HANDLE thread_handle = NULL;
> +	GROUP_AFFINITY thread_affinity;
> +	struct thread_routine_ctx *ctx = NULL;
> +
> +	ctx = calloc(1, sizeof(*ctx));
> +	if (ctx == NULL) {
> +		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
> +		ret = ENOMEM;
> +		goto cleanup;
> +	}
> +	ctx->routine_args = args;
> +	ctx->thread_func = thread_func;
> +
> +	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
> +		CREATE_SUSPENDED, &tid);
> +	if (thread_handle == NULL) {
> +		ret = thread_log_last_error("CreateThread()");
> +		free(ctx);
> +		goto cleanup;

Missing `free(ctx)` from other error paths below.

> +	}
> +	thread_id->opaque_id = tid;
> +
> +	if (thread_attr != NULL) {
> +		if (CPU_COUNT(&thread_attr->cpuset) > 0) {
> +			ret = convert_cpuset_to_affinity(
> +							&thread_attr->cpuset,
> +							&thread_affinity
> +							);
> +			if (ret != 0) {
> +				RTE_LOG(DEBUG, EAL, "Unable to convert cpuset to thread affinity\n");
> +				goto cleanup;
> +			}
> +
> +			if (!SetThreadGroupAffinity(thread_handle,
> +						    &thread_affinity, NULL)) {
> +				ret = thread_log_last_error("SetThreadGroupAffinity()");
> +				goto cleanup;
> +			}
> +		}
> +		ret = rte_thread_set_priority(*thread_id,
> +				thread_attr->priority);
> +		if (ret != 0) {
> +			RTE_LOG(DEBUG, EAL, "Unable to set thread priority\n");
> +			goto cleanup;
> +		}
> +	}
> +
> +	if (ResumeThread(thread_handle) == (DWORD)-1) {
> +		ret = thread_log_last_error("ResumeThread()");
> +		goto cleanup;
> +	}
> +
> +cleanup:
> +	if (thread_handle != NULL) {
> +		CloseHandle(thread_handle);
> +		thread_handle = NULL;
> +	}
> +
> +	return ret;
> +}

Re: [PATCH v2 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

On Sat, Jun 18, 2022 at 03:59:08PM +0300, Dmitry Kozlyuk wrote:
> 2022-06-14 16:47 (UTC-0700), Tyler Retzlaff:
> > On Windows, the function executed by a thread when the thread starts is
> > represeneted by a function pointer of type DWORD (*func) (void*).
> > On other platforms, the function pointer is a void* (*func) (void*).
> > 
> > Performing a cast between these two types of function pointers to
> > uniformize the API on all platforms may result in undefined behavior.
> > TO fix this issue, a wrapper that respects the signature required by
> > CreateThread() has been created on Windows.
> 
> The interface issue is still there:
> `rte_thread_func` allows the thread routine to have a pointer-sized result.
> `rte_thread_join()` allows to obtain this value as `unsigned long`,
> which is pointer-sized on 32-bit platforms
> and less than that on 64-bit platforms.
> This can lead to issues when developers assume they can return a pointer
> and this works on 32 bits, but doesn't work on 64 bits.
> If you want to keep API as close to phtread as possible,
> the limitation must be at least clearly documented in Doxygen
> (`rte_thread_func` is undocumented BTW).
> I also suggest using `uint32_t` instead of `unsigned long`
> precisely to avoid "is it pointer-sized?" doubts.
> (I don't see much benefit in keeping pthread-like signature.
> When moving from pthread to rte_thread,
> it is as trivial to change the thread routine return type.)

thanks Dmitry, i wasn't aware this feedback had been given previously.
let me digest the problem and we'll figure out how we can address it.

> 
> > +int
> > +rte_thread_create(rte_thread_t *thread_id,
> > +		const rte_thread_attr_t *thread_attr,
> > +		rte_thread_func thread_func, void *args)
> > +{
> > [...]
> > +		if (thread_attr->priority ==
> > +				RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
> > +			ret = ENOTSUP;
> > +			goto cleanup;
> > +		}
> > +		ret = thread_map_priority_to_os_value(thread_attr->priority,
> > +				&param.sched_priority, &policy);
> > +		if (ret != 0)
> > +			goto cleanup;
> 
> thread_map_priority_to_os_value() already checks for unsupported values,
> why not let it do this particular check?

hm, i hadn't noticed that was duplicated. let me try to eliminate it.

> 
> > +int
> > +rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
> > +{
> > +	int ret = 0;
> > +	void *res = NULL;
> > +	void **pres = NULL;
> > +
> > +	if (value_ptr != NULL)
> > +		pres = &res;
> > +
> > +	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
> > +	if (ret != 0) {
> > +		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
> > +		return ret;
> > +	}
> > +
> > +	if (value_ptr != NULL && *pres != NULL)
> > +		*value_ptr = *(unsigned long *)(*pres);
> > +
> > +	return 0;
> > +}
> 
> What makes *pres == NULL special?
> 
> > +static DWORD
> > +thread_func_wrapper(void *args)
> > +{
> > +	struct thread_routine_ctx *pctx = args;
> > +	struct thread_routine_ctx ctx;
> > +
> > +	ctx.thread_func = pctx->thread_func;
> > +	ctx.routine_args = pctx->routine_args;
> 
> ctx = *pctx?

ack

> 
> > +
> > +	free(pctx);
> > +
> > +	return (DWORD)(uintptr_t)ctx.thread_func(ctx.routine_args);
> > +}
> > +
> > +int
> > +rte_thread_create(rte_thread_t *thread_id,
> > +		  const rte_thread_attr_t *thread_attr,
> > +		  rte_thread_func thread_func, void *args)
> > +{
> > +	int ret = 0;
> > +	DWORD tid;
> > +	HANDLE thread_handle = NULL;
> > +	GROUP_AFFINITY thread_affinity;
> > +	struct thread_routine_ctx *ctx = NULL;
> > +
> > +	ctx = calloc(1, sizeof(*ctx));
> > +	if (ctx == NULL) {
> > +		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
> > +		ret = ENOMEM;
> > +		goto cleanup;
> > +	}
> > +	ctx->routine_args = args;
> > +	ctx->thread_func = thread_func;
> > +
> > +	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
> > +		CREATE_SUSPENDED, &tid);
> > +	if (thread_handle == NULL) {
> > +		ret = thread_log_last_error("CreateThread()");
> > +		free(ctx);
> > +		goto cleanup;
> 
> Missing `free(ctx)` from other error paths below.

ack

> 
> > +	}
> > +	thread_id->opaque_id = tid;
> > +
> > +	if (thread_attr != NULL) {
> > +		if (CPU_COUNT(&thread_attr->cpuset) > 0) {
> > +			ret = convert_cpuset_to_affinity(
> > +							&thread_attr->cpuset,
> > +							&thread_affinity
> > +							);
> > +			if (ret != 0) {
> > +				RTE_LOG(DEBUG, EAL, "Unable to convert cpuset to thread affinity\n");
> > +				goto cleanup;
> > +			}
> > +
> > +			if (!SetThreadGroupAffinity(thread_handle,
> > +						    &thread_affinity, NULL)) {
> > +				ret = thread_log_last_error("SetThreadGroupAffinity()");
> > +				goto cleanup;
> > +			}
> > +		}
> > +		ret = rte_thread_set_priority(*thread_id,
> > +				thread_attr->priority);
> > +		if (ret != 0) {
> > +			RTE_LOG(DEBUG, EAL, "Unable to set thread priority\n");
> > +			goto cleanup;
> > +		}
> > +	}
> > +
> > +	if (ResumeThread(thread_handle) == (DWORD)-1) {
> > +		ret = thread_log_last_error("ResumeThread()");
> > +		goto cleanup;
> > +	}
> > +
> > +cleanup:
> > +	if (thread_handle != NULL) {
> > +		CloseHandle(thread_handle);
> > +		thread_handle = NULL;
> > +	}
> > +
> > +	return ret;
> > +}

thanks let me tidy those bits up, i'll submit a revision this week.

Re: [PATCH v2 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

On Sat, Jun 18, 2022 at 03:59:08PM +0300, Dmitry Kozlyuk wrote:
> 2022-06-14 16:47 (UTC-0700), Tyler Retzlaff:
> 
> > +int
> > +rte_thread_create(rte_thread_t *thread_id,
> > +		const rte_thread_attr_t *thread_attr,
> > +		rte_thread_func thread_func, void *args)
> > +{
> > [...]
> > +		if (thread_attr->priority ==
> > +				RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
> > +			ret = ENOTSUP;
> > +			goto cleanup;
> > +		}
> > +		ret = thread_map_priority_to_os_value(thread_attr->priority,
> > +				&param.sched_priority, &policy);
> > +		if (ret != 0)
> > +			goto cleanup;
> 
> thread_map_priority_to_os_value() already checks for unsupported values,
> why not let it do this particular check?

okay, so i looked at this more closely.

* thread_map_priority_to_os_value() just does mapping and
  RTE_THREAD_PRIORITY_REALTIME_CRITICAL is a valid mapping so it does not
  fail. by design it does one thing and one thing only perform the value
  mapping. admittedly it does not map every valid value right now, maybe it
  should?

  for consistency in the behavior of the function i'd suggest we don't
  make it have special cases in case we later decide to expand for
  additional valid mappings.

* rte_thread_set_priority() does fail with ENOTSUP on non-windows if
  provided RTE_THREAD_PRIORITY_REALTIME_CRITICAL however it cannot be
  used at this point since the thread has not been created.

if no further follow up on this i'm going to leave it as is.

thanks

Re: [PATCH v2 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

On Sat, Jun 18, 2022 at 03:59:08PM +0300, Dmitry Kozlyuk wrote:
> 2022-06-14 16:47 (UTC-0700), Tyler Retzlaff:
> > On Windows, the function executed by a thread when the thread starts is
> > represeneted by a function pointer of type DWORD (*func) (void*).
> > On other platforms, the function pointer is a void* (*func) (void*).
> > 
> > Performing a cast between these two types of function pointers to
> > uniformize the API on all platforms may result in undefined behavior.
> > TO fix this issue, a wrapper that respects the signature required by
> > CreateThread() has been created on Windows.
> 
> The interface issue is still there:
> `rte_thread_func` allows the thread routine to have a pointer-sized result.
> `rte_thread_join()` allows to obtain this value as `unsigned long`,
> which is pointer-sized on 32-bit platforms
> and less than that on 64-bit platforms.
> This can lead to issues when developers assume they can return a pointer
> and this works on 32 bits, but doesn't work on 64 bits.
> If you want to keep API as close to phtread as possible,
> the limitation must be at least clearly documented in Doxygen
> (`rte_thread_func` is undocumented BTW).
> I also suggest using `uint32_t` instead of `unsigned long`
> precisely to avoid "is it pointer-sized?" doubts.
> (I don't see much benefit in keeping pthread-like signature.
> When moving from pthread to rte_thread,
> it is as trivial to change the thread routine return type.)

i'll alter the rte api to use fixed width uint32_t for thread result. it
seems like an unnecessary feature to return a pointer and it can be
accomplished through void *arg if necessary.

> 
> > +int
> > +rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
> > +{
> > +	int ret = 0;
> > +	void *res = NULL;
> > +	void **pres = NULL;
> > +
> > +	if (value_ptr != NULL)
> > +		pres = &res;
> > +
> > +	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
> > +	if (ret != 0) {
> > +		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
> > +		return ret;
> > +	}
> > +
> > +	if (value_ptr != NULL && *pres != NULL)
> > +		*value_ptr = *(unsigned long *)(*pres);
> > +
> > +	return 0;
> > +}
> 
> What makes *pres == NULL special?

it's not clear what you mean, can you explain? maybe there is some
context i am missing from the original patch series?

> > +int
> > +rte_thread_create(rte_thread_t *thread_id,
> > +		  const rte_thread_attr_t *thread_attr,
> > +		  rte_thread_func thread_func, void *args)
> > +{
> > +	int ret = 0;
> > +	DWORD tid;
> > +	HANDLE thread_handle = NULL;
> > +	GROUP_AFFINITY thread_affinity;
> > +	struct thread_routine_ctx *ctx = NULL;
> > +
> > +	ctx = calloc(1, sizeof(*ctx));
> > +	if (ctx == NULL) {
> > +		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
> > +		ret = ENOMEM;
> > +		goto cleanup;
> > +	}
> > +	ctx->routine_args = args;
> > +	ctx->thread_func = thread_func;
> > +
> > +	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
> > +		CREATE_SUSPENDED, &tid);
> > +	if (thread_handle == NULL) {
> > +		ret = thread_log_last_error("CreateThread()");
> > +		free(ctx);
> > +		goto cleanup;
> 
> Missing `free(ctx)` from other error paths below.

beyond this point free(ctx) will happen in thread_func_wrapper. i will
add a comment to make it clear.

thanks.

Re: [PATCH v2 2/6] eal: add thread lifetime management

[Dmitry Kozlyuk]

2022-06-21 11:51 (UTC-0700), Tyler Retzlaff:
> > > +int
> > > +rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
> > > +{
> > > +	int ret = 0;
> > > +	void *res = NULL;
> > > +	void **pres = NULL;
> > > +
> > > +	if (value_ptr != NULL)
> > > +		pres = &res;
> > > +
> > > +	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
> > > +	if (ret != 0) {
> > > +		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	if (value_ptr != NULL && *pres != NULL)
> > > +		*value_ptr = *(unsigned long *)(*pres);
> > > +
> > > +	return 0;
> > > +}  
> > 
> > What makes *pres == NULL special?  
> 
> it's not clear what you mean, can you explain? maybe there is some
> context i am missing from the original patch series?

There's no previous context.
After ptread_join(), *pres holds the return value of the thread routine.
You only assign *value_ptr if value_ptr is not NULL (obviously correct)
and if *pres != NULL, that is, if the thread returned a non-NULL value.
But this value is opaque, why do you filter NULL?
Perhaps you meant if (pres != NULL), no dereference?

> > > +int
> > > +rte_thread_create(rte_thread_t *thread_id,
> > > +		  const rte_thread_attr_t *thread_attr,
> > > +		  rte_thread_func thread_func, void *args)
> > > +{
> > > +	int ret = 0;
> > > +	DWORD tid;
> > > +	HANDLE thread_handle = NULL;
> > > +	GROUP_AFFINITY thread_affinity;
> > > +	struct thread_routine_ctx *ctx = NULL;
> > > +
> > > +	ctx = calloc(1, sizeof(*ctx));
> > > +	if (ctx == NULL) {
> > > +		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
> > > +		ret = ENOMEM;
> > > +		goto cleanup;
> > > +	}
> > > +	ctx->routine_args = args;
> > > +	ctx->thread_func = thread_func;
> > > +
> > > +	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
> > > +		CREATE_SUSPENDED, &tid);
> > > +	if (thread_handle == NULL) {
> > > +		ret = thread_log_last_error("CreateThread()");
> > > +		free(ctx);
> > > +		goto cleanup;  
> > 
> > Missing `free(ctx)` from other error paths below.  
> 
> beyond this point free(ctx) will happen in thread_func_wrapper. i will
> add a comment to make it clear.

Not if you exit before ResumeThread()
and thread_func_wrapper() will never execute to call free().

Re: [PATCH v2 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

On Tue, Jun 21, 2022 at 10:44:21PM +0300, Dmitry Kozlyuk wrote:
> 2022-06-21 11:51 (UTC-0700), Tyler Retzlaff:
> > > > +int
> > > > +rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
> > > > +{
> > > > +	int ret = 0;
> > > > +	void *res = NULL;
> > > > +	void **pres = NULL;
> > > > +
> > > > +	if (value_ptr != NULL)
> > > > +		pres = &res;
> > > > +
> > > > +	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
> > > > +	if (ret != 0) {
> > > > +		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	if (value_ptr != NULL && *pres != NULL)
> > > > +		*value_ptr = *(unsigned long *)(*pres);
> > > > +
> > > > +	return 0;
> > > > +}  
> > > 
> > > What makes *pres == NULL special?  
> > 
> > it's not clear what you mean, can you explain? maybe there is some
> > context i am missing from the original patch series?
> 
> There's no previous context.
> After ptread_join(), *pres holds the return value of the thread routine.
> You only assign *value_ptr if value_ptr is not NULL (obviously correct)
> and if *pres != NULL, that is, if the thread returned a non-NULL value.
> But this value is opaque, why do you filter NULL?

i don't think it is opaque here? unsigned long * value_ptr says we have
to store an integer. which leads to a discussion of what should get
stored at the value_ptr location if pthread_join() itself returns no
result but the caller of rte_thread_join() requests the result.

> Perhaps you meant if (pres != NULL), no dereference?

that i think is just a repeat of a test checking if the caller of
rte_thread_join is interested in the result?
i.e. value_ptr != NULL -> pres != NULL

both pres and *pres are dereferenced so it seems to track that prior to
those dereferences they have to be validated as being non-NULL.
i don't see how we could avoid dereferencing **pres to satisfy the
calling contract when the result is requested.

now if value_ptr was unsigned long ** i guess i'd understand. i could
always be reading the code wrong. but thinking about further there is
another problem with this in that we really don't know what is being
aliased in *pres when using the pthread implementation, since pthread
could be returning a pointer to something narrow or with unknown layout
where later dereferencing it as something wider or in this case
specifically as unsigned long * would have horrible consequences.

i think this ends up semi-related to your other comment about what the
result type from rte_thread_func is, we can discuss offline and post
details back to the list.

> 
> > > > +int
> > > > +rte_thread_create(rte_thread_t *thread_id,
> > > > +		  const rte_thread_attr_t *thread_attr,
> > > > +		  rte_thread_func thread_func, void *args)
> > > > +{
> > > > +	int ret = 0;
> > > > +	DWORD tid;
> > > > +	HANDLE thread_handle = NULL;
> > > > +	GROUP_AFFINITY thread_affinity;
> > > > +	struct thread_routine_ctx *ctx = NULL;
> > > > +
> > > > +	ctx = calloc(1, sizeof(*ctx));
> > > > +	if (ctx == NULL) {
> > > > +		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
> > > > +		ret = ENOMEM;
> > > > +		goto cleanup;
> > > > +	}
> > > > +	ctx->routine_args = args;
> > > > +	ctx->thread_func = thread_func;
> > > > +
> > > > +	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
> > > > +		CREATE_SUSPENDED, &tid);
> > > > +	if (thread_handle == NULL) {
> > > > +		ret = thread_log_last_error("CreateThread()");
> > > > +		free(ctx);
> > > > +		goto cleanup;  
> > > 
> > > Missing `free(ctx)` from other error paths below.  
> > 
> > beyond this point free(ctx) will happen in thread_func_wrapper. i will
> > add a comment to make it clear.
> 
> Not if you exit before ResumeThread()
> and thread_func_wrapper() will never execute to call free().

yes, you are right i forgot that this thread is created suspended.

Re: [PATCH v2 2/6] eal: add thread lifetime management

[Dmitry Kozlyuk]

2022-06-21 14:28 (UTC-0700), Tyler Retzlaff:
> On Tue, Jun 21, 2022 at 10:44:21PM +0300, Dmitry Kozlyuk wrote:
> > 2022-06-21 11:51 (UTC-0700), Tyler Retzlaff:  
> > > > > +int
> > > > > +rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
> > > > > +{
> > > > > +	int ret = 0;
> > > > > +	void *res = NULL;
> > > > > +	void **pres = NULL;
> > > > > +
> > > > > +	if (value_ptr != NULL)
> > > > > +		pres = &res;
> > > > > +
> > > > > +	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
> > > > > +	if (ret != 0) {
> > > > > +		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	if (value_ptr != NULL && *pres != NULL)
> > > > > +		*value_ptr = *(unsigned long *)(*pres);
> > > > > +
> > > > > +	return 0;
> > > > > +}    
> > > > 
> > > > What makes *pres == NULL special?    
> > > 
> > > it's not clear what you mean, can you explain? maybe there is some
> > > context i am missing from the original patch series?  
> > 
> > There's no previous context.
> > After ptread_join(), *pres holds the return value of the thread routine.
> > You only assign *value_ptr if value_ptr is not NULL (obviously correct)
> > and if *pres != NULL, that is, if the thread returned a non-NULL value.
> > But this value is opaque, why do you filter NULL?  
> 
> i don't think it is opaque here? unsigned long * value_ptr says we have
> to store an integer. which leads to a discussion of what should get
> stored at the value_ptr location if pthread_join() itself returns no
> result but the caller of rte_thread_join() requests the result.

There is no question. If `pthread_join()` fails, the function exits early
and `*value_ptr` remains unmodified. If `pthread_join()` succeeds
with a non-NULL second argument (`pres`), `*pres` aka `res` is always filled.
NULL can be placed there too if that's what the thread routine has returned.

> 
> > Perhaps you meant if (pres != NULL), no dereference?  
> 
> that i think is just a repeat of a test checking if the caller of
> rte_thread_join is interested in the result?
> i.e. value_ptr != NULL -> pres != NULL
> 
> both pres and *pres are dereferenced so it seems to track that prior to
> those dereferences they have to be validated as being non-NULL.
> i don't see how we could avoid dereferencing **pres to satisfy the
> calling contract when the result is requested.
> 
> now if value_ptr was unsigned long ** i guess i'd understand. i could
> always be reading the code wrong. but thinking about further there is
> another problem with this in that we really don't know what is being
> aliased in *pres when using the pthread implementation, since pthread
> could be returning a pointer to something narrow or with unknown layout
> where later dereferencing it as something wider or in this case
> specifically as unsigned long * would have horrible consequences.

Sorry, no, it's just all wrong.
We get a pointer from `pthread_join()` and store it in `res`,
which can also be accessed as `*pres`, because `pres == &res`.
Then we store the lower bits of `res` in an `*value_ptr`.
We don't care what `res` points to because it is never dereferenced.

> 
> i think this ends up semi-related to your other comment about what the
> result type from rte_thread_func is, we can discuss offline and post
> details back to the list.

Re: [PATCH v2 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

On Wed, Jun 22, 2022 at 01:24:14AM +0300, Dmitry Kozlyuk wrote:
> 2022-06-21 14:28 (UTC-0700), Tyler Retzlaff:
> > On Tue, Jun 21, 2022 at 10:44:21PM +0300, Dmitry Kozlyuk wrote:
> > > 2022-06-21 11:51 (UTC-0700), Tyler Retzlaff:  
> > > > > > +int
> > > > > > +rte_thread_join(rte_thread_t thread_id, unsigned long *value_ptr)
> > > > > > +{
> > > > > > +	int ret = 0;
> > > > > > +	void *res = NULL;
> > > > > > +	void **pres = NULL;
> > > > > > +
> > > > > > +	if (value_ptr != NULL)
> > > > > > +		pres = &res;
> > > > > > +
> > > > > > +	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
> > > > > > +	if (ret != 0) {
> > > > > > +		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
> > > > > > +		return ret;
> > > > > > +	}
> > > > > > +
> > > > > > +	if (value_ptr != NULL && *pres != NULL)
> > > > > > +		*value_ptr = *(unsigned long *)(*pres);
> > > > > > +
> > > > > > +	return 0;
> > > > > > +}    
> > > > > 
> > > > > What makes *pres == NULL special?    
> > > > 
> > > > it's not clear what you mean, can you explain? maybe there is some
> > > > context i am missing from the original patch series?  
> > > 
> > > There's no previous context.
> > > After ptread_join(), *pres holds the return value of the thread routine.
> > > You only assign *value_ptr if value_ptr is not NULL (obviously correct)
> > > and if *pres != NULL, that is, if the thread returned a non-NULL value.
> > > But this value is opaque, why do you filter NULL?  
> > 
> > i don't think it is opaque here? unsigned long * value_ptr says we have
> > to store an integer. which leads to a discussion of what should get
> > stored at the value_ptr location if pthread_join() itself returns no
> > result but the caller of rte_thread_join() requests the result.
> 
> There is no question. If `pthread_join()` fails, the function exits early
> and `*value_ptr` remains unmodified. If `pthread_join()` succeeds
> with a non-NULL second argument (`pres`), `*pres` aka `res` is always filled.
> NULL can be placed there too if that's what the thread routine has returned.

okay, discussed offline it was just my misunderstanding of impact on the
conditional block in the presence of the NULL check.

as discussed we'll rewrite the check as.

if (value_ptr != NULL)
    *value_ptr = (uint32_t)(uintptr_t)res;

this doesn't double dereference pres as the original code was and
therefore the *pres NULL check is unnecessary.

thanks!

[PATCH v2 3/6] eal: add basic rte thread ID equal API

[Tyler Retzlaff]

Add rte_thread_equal() that tests if two rte_thread_id are equal.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
 lib/eal/unix/rte_thread.c    |  6 ++++++
 lib/eal/version.map          |  1 +
 lib/eal/windows/rte_thread.c |  6 ++++++
 4 files changed, 32 insertions(+)

diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index c27e580..de0486d 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -134,6 +134,25 @@ int rte_thread_create(rte_thread_t *thread_id,
 __rte_experimental
 rte_thread_t rte_thread_self(void);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Check if 2 thread ids are equal.
+ *
+ * @param t1
+ *   First thread id.
+ *
+ * @param t2
+ *   Second thread id.
+ *
+ * @return
+ *   If the ids are equal, return nonzero.
+ *   Otherwise, return 0.
+ */
+__rte_experimental
+int rte_thread_equal(rte_thread_t t1, rte_thread_t t2);
+
 #ifdef RTE_HAS_CPUSET
 
 /**
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index 19c7b80..0304d53 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -183,6 +183,12 @@ struct eal_tls_key {
 	return pthread_detach((pthread_t)thread_id.opaque_id);
 }
 
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return pthread_equal((pthread_t)t1.opaque_id, (pthread_t)t2.opaque_id);
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 22e5c85..4a52484 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -428,6 +428,7 @@ EXPERIMENTAL {
 	rte_thread_attr_set_priority;
 	rte_thread_create;
 	rte_thread_detach;
+	rte_thread_equal;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
 	rte_thread_join;
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index b5f2b04..1352513 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -291,6 +291,12 @@ struct thread_routine_ctx {
 	return 0;
 }
 
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return t1.opaque_id == t2.opaque_id;
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
-- 
1.8.3.1

Re: [PATCH v2 3/6] eal: add basic rte thread ID equal API

[Konstantin Ananyev]

> Add rte_thread_equal() that tests if two rte_thread_id are equal.
> 
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>   lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
>   lib/eal/unix/rte_thread.c    |  6 ++++++
>   lib/eal/version.map          |  1 +
>   lib/eal/windows/rte_thread.c |  6 ++++++
>   4 files changed, 32 insertions(+)
> 
> diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> index c27e580..de0486d 100644
> --- a/lib/eal/include/rte_thread.h
> +++ b/lib/eal/include/rte_thread.h
> @@ -134,6 +134,25 @@ int rte_thread_create(rte_thread_t *thread_id,
>   __rte_experimental
>   rte_thread_t rte_thread_self(void);
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Check if 2 thread ids are equal.
> + *
> + * @param t1
> + *   First thread id.
> + *
> + * @param t2
> + *   Second thread id.
> + *
> + * @return
> + *   If the ids are equal, return nonzero.
> + *   Otherwise, return 0.
> + */
> +__rte_experimental
> +int rte_thread_equal(rte_thread_t t1, rte_thread_t t2);
> +
>   #ifdef RTE_HAS_CPUSET
>   
>   /**
> diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
> index 19c7b80..0304d53 100644
> --- a/lib/eal/unix/rte_thread.c
> +++ b/lib/eal/unix/rte_thread.c
> @@ -183,6 +183,12 @@ struct eal_tls_key {
>   	return pthread_detach((pthread_t)thread_id.opaque_id);
>   }
>   
> +int
> +rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
> +{
> +	return pthread_equal((pthread_t)t1.opaque_id, (pthread_t)t2.opaque_id);
> +}
> +
>   rte_thread_t
>   rte_thread_self(void)
>   {
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index 22e5c85..4a52484 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -428,6 +428,7 @@ EXPERIMENTAL {
>   	rte_thread_attr_set_priority;
>   	rte_thread_create;
>   	rte_thread_detach;
> +	rte_thread_equal;
>   	rte_thread_get_affinity_by_id;
>   	rte_thread_get_priority;
>   	rte_thread_join;
> diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
> index b5f2b04..1352513 100644
> --- a/lib/eal/windows/rte_thread.c
> +++ b/lib/eal/windows/rte_thread.c
> @@ -291,6 +291,12 @@ struct thread_routine_ctx {
>   	return 0;
>   }
>   
> +int
> +rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
> +{
> +	return t1.opaque_id == t2.opaque_id;
> +}
> +
>   rte_thread_t
>   rte_thread_self(void)
>   {

Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>

[PATCH v2 4/6] test/threads: add tests for thread lifetime API

[Tyler Retzlaff]

test basic functionality and demonstrate use of following thread
lifetime api.

    * rte_thread_create
    * rte_thread_detach
    * rte_thread_join

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index b9d8b4e..9a30af5 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -27,6 +27,54 @@
 }
 
 static int
+test_thread_create_join(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, &thread_main_id) == 0,
+		"Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_join(thread_id, NULL) == 0,
+		"Failed to join thread.");
+
+	return 0;
+}
+
+static int
+test_thread_create_detach(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main,
+		&thread_main_id) == 0, "Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_detach(thread_id) == 0,
+		"Failed to detach thread.");
+
+	return 0;
+}
+
+static int
 test_thread_priority(void)
 {
 	pthread_t id;
@@ -123,6 +171,8 @@
 	.setup = NULL,
 	.teardown = NULL,
 	.unit_test_cases = {
+		TEST_CASE(test_thread_create_join),
+		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
 		TEST_CASES_END()
-- 
1.8.3.1

[PATCH v2 5/6] test/threads: add tests for thread attributes API

[Tyler Retzlaff]

test basic functionality and demonstrate use of following thread
attributes api. additionally, test attributes are processed when
supplied to rte_thread_create().

    * rte_thread_attr_init
    * rte_thread_attr_set_affinity
    * rte_thread_attr_get_affinity
    * rte_thread_attr_set_priority

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 9a30af5..92a149a 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -17,7 +17,9 @@
 static void *
 thread_main(void *arg)
 {
-	*(rte_thread_t *)arg = rte_thread_self();
+	if (arg != NULL)
+		*(rte_thread_t *)arg = rte_thread_self();
+
 	__atomic_store_n(&thread_id_ready, 1, __ATOMIC_RELEASE);
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 1)
@@ -166,6 +168,73 @@
 	return 0;
 }
 
+static int
+test_thread_attributes_affinity(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	rte_cpuset_t cpuset0;
+	rte_cpuset_t cpuset1;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+
+	CPU_ZERO(&cpuset0);
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(rte_thread_self(), &cpuset0) == 0,
+		"Failed to get thread affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_set_affinity(&attr, &cpuset0) == 0,
+		"Failed to set thread attributes affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_get_affinity(&attr, &cpuset1) == 0,
+		"Failed to get thread attributes affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Affinity should be stable");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes affinity thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
+		"Failed to get attributes thread affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Failed to apply affinity attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
+static int
+test_thread_attributes_priority(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	enum rte_thread_priority priority;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+	RTE_TEST_ASSERT(rte_thread_attr_set_priority(&attr, RTE_THREAD_PRIORITY_NORMAL) == 0,
+		"Failed to set thread attributes priority");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes priority thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_priority(thread_id, &priority) == 0,
+		"Failed to get thread priority");
+	RTE_TEST_ASSERT(priority == RTE_THREAD_PRIORITY_NORMAL,
+		"Failed to apply priority attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
 static struct unit_test_suite threads_test_suite = {
 	.suite_name = "threads autotest",
 	.setup = NULL,
@@ -175,6 +244,8 @@
 		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
+		TEST_CASE(test_thread_attributes_affinity),
+		TEST_CASE(test_thread_attributes_priority),
 		TEST_CASES_END()
 	}
 };
-- 
1.8.3.1

[PATCH v2 6/6] test/threads: remove unit test use of pthread

[Tyler Retzlaff]

now that rte_thread provides thread lifetime functions stop using
pthread in unit tests.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 92a149a..034f82d 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -3,7 +3,6 @@
  */
 
 #include <string.h>
-#include <pthread.h>
 
 #include <rte_thread.h>
 #include <rte_debug.h>
@@ -79,12 +78,11 @@
 static int
 test_thread_priority(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	enum rte_thread_priority priority;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
@@ -131,13 +129,12 @@
 static int
 test_thread_affinity(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	rte_cpuset_t cpuset0;
 	rte_cpuset_t cpuset1;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
-- 
1.8.3.1

[PATCH v3 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

add rte thread lifetime and attributes api. with these api additions
there is now sufficient platform abstracted thread api to remove the
use of pthread in the unit tests.

v3:
  * change rte_thread_func return type to uint32_t for exit value.
  * change rte_thread_join retval to be uint32_t (matched with the
    return value from rte_thread_func.
  * introduce a wrapper for rte_thread_func on posix platforms to
    adapt differences between rte_thread_func and pthread
    start_routine.
  * remove interpretation / dereference of result from pthread_join
    in posix implementation of rte_thread_join.
  * fix leak of dynamically allocated thread_routine_ctx on windows
    in error paths.
  * don't cast and truncate NULL to integer value for rte_thread_join
    when pthread_join returns no result.

v2:
  * split implementation of rte_thread_equal for windows / posix
    and use pthread_equal for posix platforms.
  * remove parameter validation assertions and instead return
    EINVAL for mandatory pointers to type that are NULL.
  * correct doxygen comment parameter name args -> arg

Tyler Retzlaff (6):
  eal: add thread attributes
  eal: add thread lifetime management
  eal: add basic rte thread ID equal API
  test/threads: add tests for thread lifetime API
  test/threads: add tests for thread attributes API
  test/threads: remove unit test use of pthread

 app/test/test_threads.c         | 134 ++++++++++++++++++++++--
 lib/eal/common/meson.build      |   1 +
 lib/eal/common/rte_thread.c     |  60 +++++++++++
 lib/eal/include/rte_thread.h    | 187 ++++++++++++++++++++++++++++++++++
 lib/eal/unix/rte_thread.c       | 141 ++++++++++++++++++++++++++
 lib/eal/version.map             |   8 ++
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 219 ++++++++++++++++++++++++++++++++--------
 8 files changed, 703 insertions(+), 49 deletions(-)
 create mode 100644 lib/eal/common/rte_thread.c

-- 
1.8.3.1

[PATCH v3 1/6] eal: add thread attributes

[Tyler Retzlaff]

Implement thread attributes for:

    * thread affinity
    * thread priority

Implement functions for managing thread attributes.

  Priority is represented through an enum that allows for two levels:

    * RTE_THREAD_PRIORITY_NORMAL
    * RTE_THREAD_PRIORITY_REALTIME_CRITICAL

  Affinity is described by the rte_cpuset_t type.

An rte_thread_attr_t object can be set to the default values
by calling rte_thread_attr_init().

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/common/meson.build   |  1 +
 lib/eal/common/rte_thread.c  | 60 ++++++++++++++++++++++++++++
 lib/eal/include/rte_thread.h | 93 ++++++++++++++++++++++++++++++++++++++++++++
 lib/eal/version.map          |  4 ++
 4 files changed, 158 insertions(+)
 create mode 100644 lib/eal/common/rte_thread.c

diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
index 917758c..1e77dba 100644
--- a/lib/eal/common/meson.build
+++ b/lib/eal/common/meson.build
@@ -36,6 +36,7 @@ sources += files(
         'rte_random.c',
         'rte_reciprocal.c',
         'rte_service.c',
+        'rte_thread.c',
         'rte_version.c',
 )
 if is_linux or is_windows
diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
new file mode 100644
index 0000000..d204cc5
--- /dev/null
+++ b/lib/eal/common/rte_thread.c
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2022 Microsoft Corporation
+ */
+
+#include <rte_debug.h>
+#include <rte_thread.h>
+
+int
+rte_thread_attr_init(rte_thread_attr_t *attr)
+{
+	if (attr == NULL)
+		return EINVAL;
+
+	CPU_ZERO(&attr->cpuset);
+	attr->priority = RTE_THREAD_PRIORITY_NORMAL;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	if (cpuset == NULL)
+		return EINVAL;
+
+	thread_attr->cpuset = *cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	if (cpuset == NULL)
+		return EINVAL;
+
+	*cpuset = thread_attr->cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	thread_attr->priority = priority;
+
+	return 0;
+}
diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 9e261bf..062308d 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -40,6 +40,18 @@ enum rte_thread_priority {
 	/**< highest thread priority allowed */
 };
 
+#ifdef RTE_HAS_CPUSET
+
+/**
+ * Representation for thread attributes.
+ */
+typedef struct {
+	enum rte_thread_priority priority; /**< thread priority */
+	rte_cpuset_t cpuset; /**< thread affinity */
+} rte_thread_attr_t;
+
+#endif /* RTE_HAS_CPUSET */
+
 /**
  * TLS key type, an opaque pointer.
  */
@@ -63,6 +75,87 @@ enum rte_thread_priority {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
+ * Initialize the attributes of a thread.
+ * These attributes can be passed to the rte_thread_create() function
+ * that will create a new thread and set its attributes according to attr.
+ *
+ * @param attr
+ *   Thread attributes to initialize.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_init(rte_thread_attr_t *attr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the CPU affinity value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which affinity will be updated.
+ *
+ * @param cpuset
+ *   Points to the value of the affinity to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get the value of CPU affinity that is set in the thread attributes pointed
+ * to by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes from which affinity will be retrieved.
+ *
+ * @param cpuset
+ *   Pointer to the memory that will store the affinity.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the thread priority value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which priority will be updated.
+ *
+ * @param priority
+ *   Points to the value of the priority to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Set the affinity of thread 'thread_id' to the cpu set
  * specified by 'cpuset'.
  *
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 5e6d851..ba03528 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -423,6 +423,10 @@ EXPERIMENTAL {
 
 	# added in 22.07
 	rte_drand;
+	rte_thread_attr_get_affinity;
+	rte_thread_attr_init;
+	rte_thread_attr_set_affinity;
+	rte_thread_attr_set_priority;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
 	rte_thread_self;
-- 
1.8.3.1

[PATCH v3 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

The *rte_thread_create()* function can optionally receive an
rte_thread_attr_t object that will cause the thread to be created with
the affinity and priority described by the attributes object. If
no rte_thread_attr_t is passed (parameter is NULL), the default
affinity and priority are used.

On Windows, the function executed by a thread when the thread starts is
represeneted by a function pointer of type DWORD (*func) (void*).
On other platforms, the function pointer is a void* (*func) (void*).

Performing a cast between these two types of function pointers to
uniformize the API on all platforms may result in undefined behavior.
TO fix this issue, a wrapper that respects the signature required by
CreateThread() has been created on Windows.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/include/rte_thread.h    |  75 ++++++++++++++
 lib/eal/unix/rte_thread.c       | 135 +++++++++++++++++++++++++
 lib/eal/version.map             |   3 +
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 213 ++++++++++++++++++++++++++++++++--------
 5 files changed, 387 insertions(+), 41 deletions(-)

diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 062308d..9e80270 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -31,6 +31,18 @@
 } rte_thread_t;
 
 /**
+ * Thread function
+ *
+ * Function pointer to thread start routine.
+ *
+ * @param arg
+ *   Argument passed to rte_thread_create().
+ * @return
+ *   Thread function exit value.
+ */
+typedef uint32_t (*rte_thread_func) (void *);
+
+/**
  * Thread priority values.
  */
 enum rte_thread_priority {
@@ -61,6 +73,69 @@ enum rte_thread_priority {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
+ * Create a new thread that will invoke the 'thread_func' routine.
+ *
+ * @param thread_id
+ *    A pointer that will store the id of the newly created thread.
+ *
+ * @param thread_attr
+ *    Attributes that are used at the creation of the new thread.
+ *
+ * @param thread_func
+ *    The routine that the new thread will invoke when starting execution.
+ *
+ * @param arg
+ *    Argument to be passed to the 'thread_func' routine.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *arg);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Waits for the thread identified by 'thread_id' to terminate
+ *
+ * @param thread_id
+ *    The identifier of the thread.
+ *
+ * @param value_ptr
+ *    Stores the exit status of the thread.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_join(rte_thread_t thread_id, uint32_t *value_ptr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Indicate that the return value of the thread is not needed and
+ * all thread resources should be release when the thread terminates.
+ *
+ * @param thread_id
+ *    The id of the thread to be detached.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_detach(rte_thread_t thread_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Get the id of the calling thread.
  *
  * @return
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index 9126595..d4c1a7f 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -16,6 +16,11 @@ struct eal_tls_key {
 	pthread_key_t thread_index;
 };
 
+struct thread_routine_ctx {
+	rte_thread_func thread_func;
+	void *routine_args;
+};
+
 static int
 thread_map_priority_to_os_value(enum rte_thread_priority eal_pri, int *os_pri,
 	int *pol)
@@ -75,6 +80,136 @@ struct eal_tls_key {
 	return 0;
 }
 
+static void *
+thread_func_wrapper(void *arg)
+{
+	struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg;
+
+	free(arg);
+
+	return (void *)(uintptr_t)ctx.thread_func(ctx.routine_args);
+}
+
+int
+rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	pthread_attr_t attr;
+	pthread_attr_t *attrp = NULL;
+	struct thread_routine_ctx *ctx;
+	struct sched_param param = {
+		.sched_priority = 0,
+	};
+	int policy = SCHED_OTHER;
+
+	ctx = calloc(1, sizeof(*ctx));
+	if (ctx == NULL) {
+		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
+		ret = ENOMEM;
+		goto cleanup;
+	}
+	ctx->routine_args = args;
+	ctx->thread_func = thread_func;
+
+	if (thread_attr != NULL) {
+		ret = pthread_attr_init(&attr);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_init failed\n");
+			goto cleanup;
+		}
+
+		attrp = &attr;
+
+		/*
+		 * Set the inherit scheduler parameter to explicit,
+		 * otherwise the priority attribute is ignored.
+		 */
+		ret = pthread_attr_setinheritsched(attrp,
+				PTHREAD_EXPLICIT_SCHED);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setinheritsched failed\n");
+			goto cleanup;
+		}
+
+
+		if (thread_attr->priority ==
+				RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+		ret = thread_map_priority_to_os_value(thread_attr->priority,
+				&param.sched_priority, &policy);
+		if (ret != 0)
+			goto cleanup;
+
+		ret = pthread_attr_setschedpolicy(attrp, policy);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedpolicy failed\n");
+			goto cleanup;
+		}
+
+		ret = pthread_attr_setschedparam(attrp, &param);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedparam failed\n");
+			goto cleanup;
+		}
+	}
+
+	ret = pthread_create((pthread_t *)&thread_id->opaque_id, attrp,
+		thread_func_wrapper, ctx);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_create failed\n");
+		goto cleanup;
+	}
+
+	if (thread_attr != NULL && CPU_COUNT(&thread_attr->cpuset) > 0) {
+		ret = rte_thread_set_affinity_by_id(*thread_id,
+			&thread_attr->cpuset);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "rte_thread_set_affinity_by_id failed\n");
+			goto cleanup;
+		}
+	}
+
+	ctx = NULL;
+cleanup:
+	free(ctx);
+	if (attrp != NULL)
+		pthread_attr_destroy(&attr);
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, uint32_t *value_ptr)
+{
+	int ret = 0;
+	void *res = (void *)(uintptr_t)0;
+	void **pres = NULL;
+
+	if (value_ptr != NULL)
+		pres = &res;
+
+	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
+		return ret;
+	}
+
+	if (value_ptr != NULL)
+		*value_ptr = (uint32_t)(uintptr_t)res;
+
+	return 0;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	return pthread_detach((pthread_t)thread_id.opaque_id);
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
diff --git a/lib/eal/version.map b/lib/eal/version.map
index ba03528..0b26652 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -427,8 +427,11 @@ EXPERIMENTAL {
 	rte_thread_attr_init;
 	rte_thread_attr_set_affinity;
 	rte_thread_attr_set_priority;
+	rte_thread_create;
+	rte_thread_detach;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
+	rte_thread_join;
 	rte_thread_self;
 	rte_thread_set_affinity_by_id;
 	rte_thread_set_priority;
diff --git a/lib/eal/windows/include/sched.h b/lib/eal/windows/include/sched.h
index bc31cc8..912fed1 100644
--- a/lib/eal/windows/include/sched.h
+++ b/lib/eal/windows/include/sched.h
@@ -44,7 +44,7 @@
 	(1LL << _WHICH_BIT(b))) != 0LL)
 
 static inline int
-count_cpu(rte_cpuset_t *s)
+count_cpu(const rte_cpuset_t *s)
 {
 	unsigned int _i;
 	int count = 0;
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index 0771525..ad71be4 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -13,6 +13,11 @@ struct eal_tls_key {
 	DWORD thread_index;
 };
 
+struct thread_routine_ctx {
+	rte_thread_func thread_func;
+	void *routine_args;
+};
+
 /* Translates the most common error codes related to threads */
 static int
 thread_translate_win32_error(DWORD error)
@@ -114,6 +119,174 @@ struct eal_tls_key {
 	return 0;
 }
 
+static int
+convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
+		PGROUP_AFFINITY affinity)
+{
+	int ret = 0;
+	PGROUP_AFFINITY cpu_affinity = NULL;
+	unsigned int cpu_idx;
+
+	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 (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) {
+			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+
+		affinity->Mask |= cpu_affinity->Mask;
+	}
+
+	if (affinity->Mask == 0) {
+		ret = EINVAL;
+		goto cleanup;
+	}
+
+cleanup:
+	return ret;
+}
+
+static DWORD
+thread_func_wrapper(void *arg)
+{
+	struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg;
+
+	free(arg);
+
+	return (DWORD)ctx.thread_func(ctx.routine_args);
+}
+
+int
+rte_thread_create(rte_thread_t *thread_id,
+		  const rte_thread_attr_t *thread_attr,
+		  rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	DWORD tid;
+	HANDLE thread_handle = NULL;
+	GROUP_AFFINITY thread_affinity;
+	struct thread_routine_ctx *ctx;
+
+	ctx = calloc(1, sizeof(*ctx));
+	if (ctx == NULL) {
+		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
+		ret = ENOMEM;
+		goto cleanup;
+	}
+	ctx->routine_args = args;
+	ctx->thread_func = thread_func;
+
+	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
+		CREATE_SUSPENDED, &tid);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("CreateThread()");
+		goto cleanup;
+	}
+	thread_id->opaque_id = tid;
+
+	if (thread_attr != NULL) {
+		if (CPU_COUNT(&thread_attr->cpuset) > 0) {
+			ret = convert_cpuset_to_affinity(
+							&thread_attr->cpuset,
+							&thread_affinity
+							);
+			if (ret != 0) {
+				RTE_LOG(DEBUG, EAL, "Unable to convert cpuset to thread affinity\n");
+				goto cleanup;
+			}
+
+			if (!SetThreadGroupAffinity(thread_handle,
+						    &thread_affinity, NULL)) {
+				ret = thread_log_last_error("SetThreadGroupAffinity()");
+				goto cleanup;
+			}
+		}
+		ret = rte_thread_set_priority(*thread_id,
+				thread_attr->priority);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "Unable to set thread priority\n");
+			goto cleanup;
+		}
+	}
+
+	if (ResumeThread(thread_handle) == (DWORD)-1) {
+		ret = thread_log_last_error("ResumeThread()");
+		goto cleanup;
+	}
+
+	ctx = NULL;
+cleanup:
+	free(ctx);
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, uint32_t *value_ptr)
+{
+	HANDLE thread_handle;
+	DWORD result;
+	DWORD exit_code = 0;
+	BOOL err;
+	int ret = 0;
+
+	thread_handle = OpenThread(SYNCHRONIZE | THREAD_QUERY_INFORMATION,
+				   FALSE, thread_id.opaque_id);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("OpenThread()");
+		goto cleanup;
+	}
+
+	result = WaitForSingleObject(thread_handle, INFINITE);
+	if (result != WAIT_OBJECT_0) {
+		ret = thread_log_last_error("WaitForSingleObject()");
+		goto cleanup;
+	}
+
+	if (value_ptr != NULL) {
+		err = GetExitCodeThread(thread_handle, &exit_code);
+		if (err == 0) {
+			ret = thread_log_last_error("GetExitCodeThread()");
+			goto cleanup;
+		}
+		*value_ptr = exit_code;
+	}
+
+cleanup:
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	/* No resources that need to be released. */
+	RTE_SET_USED(thread_id);
+
+	return 0;
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
@@ -278,46 +451,6 @@ struct eal_tls_key {
 	return output;
 }
 
-static int
-convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
-		PGROUP_AFFINITY affinity)
-{
-	int ret = 0;
-	PGROUP_AFFINITY cpu_affinity = NULL;
-	unsigned int cpu_idx;
-
-	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 (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) {
-			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
-			ret = ENOTSUP;
-			goto cleanup;
-		}
-
-		affinity->Mask |= cpu_affinity->Mask;
-	}
-
-	if (affinity->Mask == 0) {
-		ret = EINVAL;
-		goto cleanup;
-	}
-
-cleanup:
-	return ret;
-}
-
 int
 rte_thread_set_affinity_by_id(rte_thread_t thread_id,
 		const rte_cpuset_t *cpuset)
-- 
1.8.3.1

[PATCH v3 3/6] eal: add basic rte thread ID equal API

[Tyler Retzlaff]

Add rte_thread_equal() that tests if two rte_thread_id are equal.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
---
 lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
 lib/eal/unix/rte_thread.c    |  6 ++++++
 lib/eal/version.map          |  1 +
 lib/eal/windows/rte_thread.c |  6 ++++++
 4 files changed, 32 insertions(+)

diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 9e80270..ca6d9a2 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -144,6 +144,25 @@ int rte_thread_create(rte_thread_t *thread_id,
 __rte_experimental
 rte_thread_t rte_thread_self(void);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Check if 2 thread ids are equal.
+ *
+ * @param t1
+ *   First thread id.
+ *
+ * @param t2
+ *   Second thread id.
+ *
+ * @return
+ *   If the ids are equal, return nonzero.
+ *   Otherwise, return 0.
+ */
+__rte_experimental
+int rte_thread_equal(rte_thread_t t1, rte_thread_t t2);
+
 #ifdef RTE_HAS_CPUSET
 
 /**
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index d4c1a7f..37ebfcf 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -210,6 +210,12 @@ struct thread_routine_ctx {
 	return pthread_detach((pthread_t)thread_id.opaque_id);
 }
 
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return pthread_equal((pthread_t)t1.opaque_id, (pthread_t)t2.opaque_id);
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 0b26652..9096075 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -429,6 +429,7 @@ EXPERIMENTAL {
 	rte_thread_attr_set_priority;
 	rte_thread_create;
 	rte_thread_detach;
+	rte_thread_equal;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
 	rte_thread_join;
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index ad71be4..1bc648e 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -287,6 +287,12 @@ struct thread_routine_ctx {
 	return 0;
 }
 
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return t1.opaque_id == t2.opaque_id;
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
-- 
1.8.3.1

[PATCH v3 4/6] test/threads: add tests for thread lifetime API

[Tyler Retzlaff]

test basic functionality and demonstrate use of following thread
lifetime api.

    * rte_thread_create
    * rte_thread_detach
    * rte_thread_join

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index b9d8b4e..1077373 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -14,7 +14,7 @@
 
 static uint32_t thread_id_ready;
 
-static void *
+static uint32_t
 thread_main(void *arg)
 {
 	*(rte_thread_t *)arg = rte_thread_self();
@@ -23,7 +23,55 @@
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 1)
 		;
 
-	return NULL;
+	return 0;
+}
+
+static int
+test_thread_create_join(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, &thread_main_id) == 0,
+		"Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_join(thread_id, NULL) == 0,
+		"Failed to join thread.");
+
+	return 0;
+}
+
+static int
+test_thread_create_detach(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main,
+		&thread_main_id) == 0, "Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_detach(thread_id) == 0,
+		"Failed to detach thread.");
+
+	return 0;
 }
 
 static int
@@ -123,6 +171,8 @@
 	.setup = NULL,
 	.teardown = NULL,
 	.unit_test_cases = {
+		TEST_CASE(test_thread_create_join),
+		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
 		TEST_CASES_END()
-- 
1.8.3.1

[PATCH v3 5/6] test/threads: add tests for thread attributes API

[Tyler Retzlaff]

test basic functionality and demonstrate use of following thread
attributes api. additionally, test attributes are processed when
supplied to rte_thread_create().

    * rte_thread_attr_init
    * rte_thread_attr_set_affinity
    * rte_thread_attr_get_affinity
    * rte_thread_attr_set_priority

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 1077373..3c22cec 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -17,7 +17,9 @@
 static uint32_t
 thread_main(void *arg)
 {
-	*(rte_thread_t *)arg = rte_thread_self();
+	if (arg != NULL)
+		*(rte_thread_t *)arg = rte_thread_self();
+
 	__atomic_store_n(&thread_id_ready, 1, __ATOMIC_RELEASE);
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 1)
@@ -166,6 +168,73 @@
 	return 0;
 }
 
+static int
+test_thread_attributes_affinity(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	rte_cpuset_t cpuset0;
+	rte_cpuset_t cpuset1;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+
+	CPU_ZERO(&cpuset0);
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(rte_thread_self(), &cpuset0) == 0,
+		"Failed to get thread affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_set_affinity(&attr, &cpuset0) == 0,
+		"Failed to set thread attributes affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_get_affinity(&attr, &cpuset1) == 0,
+		"Failed to get thread attributes affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Affinity should be stable");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes affinity thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
+		"Failed to get attributes thread affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Failed to apply affinity attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
+static int
+test_thread_attributes_priority(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	enum rte_thread_priority priority;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+	RTE_TEST_ASSERT(rte_thread_attr_set_priority(&attr, RTE_THREAD_PRIORITY_NORMAL) == 0,
+		"Failed to set thread attributes priority");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes priority thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_priority(thread_id, &priority) == 0,
+		"Failed to get thread priority");
+	RTE_TEST_ASSERT(priority == RTE_THREAD_PRIORITY_NORMAL,
+		"Failed to apply priority attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
 static struct unit_test_suite threads_test_suite = {
 	.suite_name = "threads autotest",
 	.setup = NULL,
@@ -175,6 +244,8 @@
 		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
+		TEST_CASE(test_thread_attributes_affinity),
+		TEST_CASE(test_thread_attributes_priority),
 		TEST_CASES_END()
 	}
 };
-- 
1.8.3.1

[PATCH v3 6/6] test/threads: remove unit test use of pthread

[Tyler Retzlaff]

now that rte_thread provides thread lifetime functions stop using
pthread in unit tests.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 3c22cec..e0f18e4 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -3,7 +3,6 @@
  */
 
 #include <string.h>
-#include <pthread.h>
 
 #include <rte_thread.h>
 #include <rte_debug.h>
@@ -79,12 +78,11 @@
 static int
 test_thread_priority(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	enum rte_thread_priority priority;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
@@ -131,13 +129,12 @@
 static int
 test_thread_affinity(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	rte_cpuset_t cpuset0;
 	rte_cpuset_t cpuset1;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
-- 
1.8.3.1

[PATCH v4 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

add rte thread lifetime and attributes api. with these api additions
there is now sufficient platform abstracted thread api to remove the
use of pthread in the unit tests.

v4:
  * update version.map to show api from series added in 22.11 instead
    of 22.07.
  * fix missing parameter name in rte_thread_func declaration causing
    doxygen ci failure.

v3:
  * change rte_thread_func return type to uint32_t for exit value.
  * change rte_thread_join retval to be uint32_t (matched with the
    return value from rte_thread_func).
  * introduce a wrapper for rte_thread_func on posix platforms to
    adapt differences between rte_thread_func and pthread
    start_routine.
  * remove interpretation / dereference of result from pthread_join
    in posix implementation of rte_thread_join.
  * fix leak of dynamically allocated thread_routine_ctx on windows
    in error paths.
  * don't cast and truncate NULL to integer value for rte_thread_join
    when pthread_join returns no result.

v2:
  * split implementation of rte_thread_equal for windows / posix
    and use pthread_equal for posix platforms.
  * remove parameter validation assertions and instead return
    EINVAL for mandatory pointers to type that are NULL.
  * correct doxygen comment parameter name args -> arg

Tyler Retzlaff (6):
  eal: add thread attributes
  eal: add thread lifetime management
  eal: add basic rte thread ID equal API
  test/threads: add tests for thread lifetime API
  test/threads: add tests for thread attributes API
  test/threads: remove unit test use of pthread

 app/test/test_threads.c         | 134 ++++++++++++++++++++++--
 lib/eal/common/meson.build      |   1 +
 lib/eal/common/rte_thread.c     |  60 +++++++++++
 lib/eal/include/rte_thread.h    | 187 ++++++++++++++++++++++++++++++++++
 lib/eal/unix/rte_thread.c       | 141 ++++++++++++++++++++++++++
 lib/eal/version.map             |  10 ++
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 219 ++++++++++++++++++++++++++++++++--------
 8 files changed, 705 insertions(+), 49 deletions(-)
 create mode 100644 lib/eal/common/rte_thread.c

-- 
1.8.3.1

[PATCH v4 1/6] eal: add thread attributes

[Tyler Retzlaff]

Implement thread attributes for:

    * thread affinity
    * thread priority

Implement functions for managing thread attributes.

  Priority is represented through an enum that allows for two levels:

    * RTE_THREAD_PRIORITY_NORMAL
    * RTE_THREAD_PRIORITY_REALTIME_CRITICAL

  Affinity is described by the rte_cpuset_t type.

An rte_thread_attr_t object can be set to the default values
by calling rte_thread_attr_init().

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/common/meson.build   |  1 +
 lib/eal/common/rte_thread.c  | 60 ++++++++++++++++++++++++++++
 lib/eal/include/rte_thread.h | 93 ++++++++++++++++++++++++++++++++++++++++++++
 lib/eal/version.map          |  4 ++
 4 files changed, 158 insertions(+)
 create mode 100644 lib/eal/common/rte_thread.c

diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
index 917758c..1e77dba 100644
--- a/lib/eal/common/meson.build
+++ b/lib/eal/common/meson.build
@@ -36,6 +36,7 @@ sources += files(
         'rte_random.c',
         'rte_reciprocal.c',
         'rte_service.c',
+        'rte_thread.c',
         'rte_version.c',
 )
 if is_linux or is_windows
diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
new file mode 100644
index 0000000..d204cc5
--- /dev/null
+++ b/lib/eal/common/rte_thread.c
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2022 Microsoft Corporation
+ */
+
+#include <rte_debug.h>
+#include <rte_thread.h>
+
+int
+rte_thread_attr_init(rte_thread_attr_t *attr)
+{
+	if (attr == NULL)
+		return EINVAL;
+
+	CPU_ZERO(&attr->cpuset);
+	attr->priority = RTE_THREAD_PRIORITY_NORMAL;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	if (cpuset == NULL)
+		return EINVAL;
+
+	thread_attr->cpuset = *cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	if (cpuset == NULL)
+		return EINVAL;
+
+	*cpuset = thread_attr->cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	thread_attr->priority = priority;
+
+	return 0;
+}
diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 9e261bf..062308d 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -40,6 +40,18 @@ enum rte_thread_priority {
 	/**< highest thread priority allowed */
 };
 
+#ifdef RTE_HAS_CPUSET
+
+/**
+ * Representation for thread attributes.
+ */
+typedef struct {
+	enum rte_thread_priority priority; /**< thread priority */
+	rte_cpuset_t cpuset; /**< thread affinity */
+} rte_thread_attr_t;
+
+#endif /* RTE_HAS_CPUSET */
+
 /**
  * TLS key type, an opaque pointer.
  */
@@ -63,6 +75,87 @@ enum rte_thread_priority {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
+ * Initialize the attributes of a thread.
+ * These attributes can be passed to the rte_thread_create() function
+ * that will create a new thread and set its attributes according to attr.
+ *
+ * @param attr
+ *   Thread attributes to initialize.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_init(rte_thread_attr_t *attr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the CPU affinity value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which affinity will be updated.
+ *
+ * @param cpuset
+ *   Points to the value of the affinity to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get the value of CPU affinity that is set in the thread attributes pointed
+ * to by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes from which affinity will be retrieved.
+ *
+ * @param cpuset
+ *   Pointer to the memory that will store the affinity.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the thread priority value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which priority will be updated.
+ *
+ * @param priority
+ *   Points to the value of the priority to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Set the affinity of thread 'thread_id' to the cpu set
  * specified by 'cpuset'.
  *
diff --git a/lib/eal/version.map b/lib/eal/version.map
index c2a2ceb..6d8f89a 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -419,6 +419,10 @@ EXPERIMENTAL {
 
 	# added in 22.07
 	rte_drand;
+	rte_thread_attr_get_affinity;
+	rte_thread_attr_init;
+	rte_thread_attr_set_affinity;
+	rte_thread_attr_set_priority;
 	rte_thread_get_affinity_by_id;
 	rte_thread_get_priority;
 	rte_thread_self;
-- 
1.8.3.1

[PATCH v4 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

The *rte_thread_create()* function can optionally receive an
rte_thread_attr_t object that will cause the thread to be created with
the affinity and priority described by the attributes object. If
no rte_thread_attr_t is passed (parameter is NULL), the default
affinity and priority are used.

On Windows, the function executed by a thread when the thread starts is
represeneted by a function pointer of type DWORD (*func) (void*).
On other platforms, the function pointer is a void* (*func) (void*).

Performing a cast between these two types of function pointers to
uniformize the API on all platforms may result in undefined behavior.
TO fix this issue, a wrapper that respects the signature required by
CreateThread() has been created on Windows.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/include/rte_thread.h    |  75 ++++++++++++++
 lib/eal/unix/rte_thread.c       | 135 +++++++++++++++++++++++++
 lib/eal/version.map             |   5 +
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 213 ++++++++++++++++++++++++++++++++--------
 5 files changed, 389 insertions(+), 41 deletions(-)

diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 062308d..3c8ff50 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -31,6 +31,18 @@
 } rte_thread_t;
 
 /**
+ * Thread function
+ *
+ * Function pointer to thread start routine.
+ *
+ * @param arg
+ *   Argument passed to rte_thread_create().
+ * @return
+ *   Thread function exit value.
+ */
+typedef uint32_t (*rte_thread_func) (void *arg);
+
+/**
  * Thread priority values.
  */
 enum rte_thread_priority {
@@ -61,6 +73,69 @@ enum rte_thread_priority {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
+ * Create a new thread that will invoke the 'thread_func' routine.
+ *
+ * @param thread_id
+ *    A pointer that will store the id of the newly created thread.
+ *
+ * @param thread_attr
+ *    Attributes that are used at the creation of the new thread.
+ *
+ * @param thread_func
+ *    The routine that the new thread will invoke when starting execution.
+ *
+ * @param arg
+ *    Argument to be passed to the 'thread_func' routine.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *arg);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Waits for the thread identified by 'thread_id' to terminate
+ *
+ * @param thread_id
+ *    The identifier of the thread.
+ *
+ * @param value_ptr
+ *    Stores the exit status of the thread.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_join(rte_thread_t thread_id, uint32_t *value_ptr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Indicate that the return value of the thread is not needed and
+ * all thread resources should be release when the thread terminates.
+ *
+ * @param thread_id
+ *    The id of the thread to be detached.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_detach(rte_thread_t thread_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Get the id of the calling thread.
  *
  * @return
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index 9126595..d4c1a7f 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -16,6 +16,11 @@ struct eal_tls_key {
 	pthread_key_t thread_index;
 };
 
+struct thread_routine_ctx {
+	rte_thread_func thread_func;
+	void *routine_args;
+};
+
 static int
 thread_map_priority_to_os_value(enum rte_thread_priority eal_pri, int *os_pri,
 	int *pol)
@@ -75,6 +80,136 @@ struct eal_tls_key {
 	return 0;
 }
 
+static void *
+thread_func_wrapper(void *arg)
+{
+	struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg;
+
+	free(arg);
+
+	return (void *)(uintptr_t)ctx.thread_func(ctx.routine_args);
+}
+
+int
+rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	pthread_attr_t attr;
+	pthread_attr_t *attrp = NULL;
+	struct thread_routine_ctx *ctx;
+	struct sched_param param = {
+		.sched_priority = 0,
+	};
+	int policy = SCHED_OTHER;
+
+	ctx = calloc(1, sizeof(*ctx));
+	if (ctx == NULL) {
+		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
+		ret = ENOMEM;
+		goto cleanup;
+	}
+	ctx->routine_args = args;
+	ctx->thread_func = thread_func;
+
+	if (thread_attr != NULL) {
+		ret = pthread_attr_init(&attr);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_init failed\n");
+			goto cleanup;
+		}
+
+		attrp = &attr;
+
+		/*
+		 * Set the inherit scheduler parameter to explicit,
+		 * otherwise the priority attribute is ignored.
+		 */
+		ret = pthread_attr_setinheritsched(attrp,
+				PTHREAD_EXPLICIT_SCHED);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setinheritsched failed\n");
+			goto cleanup;
+		}
+
+
+		if (thread_attr->priority ==
+				RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+		ret = thread_map_priority_to_os_value(thread_attr->priority,
+				&param.sched_priority, &policy);
+		if (ret != 0)
+			goto cleanup;
+
+		ret = pthread_attr_setschedpolicy(attrp, policy);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedpolicy failed\n");
+			goto cleanup;
+		}
+
+		ret = pthread_attr_setschedparam(attrp, &param);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedparam failed\n");
+			goto cleanup;
+		}
+	}
+
+	ret = pthread_create((pthread_t *)&thread_id->opaque_id, attrp,
+		thread_func_wrapper, ctx);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_create failed\n");
+		goto cleanup;
+	}
+
+	if (thread_attr != NULL && CPU_COUNT(&thread_attr->cpuset) > 0) {
+		ret = rte_thread_set_affinity_by_id(*thread_id,
+			&thread_attr->cpuset);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "rte_thread_set_affinity_by_id failed\n");
+			goto cleanup;
+		}
+	}
+
+	ctx = NULL;
+cleanup:
+	free(ctx);
+	if (attrp != NULL)
+		pthread_attr_destroy(&attr);
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, uint32_t *value_ptr)
+{
+	int ret = 0;
+	void *res = (void *)(uintptr_t)0;
+	void **pres = NULL;
+
+	if (value_ptr != NULL)
+		pres = &res;
+
+	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
+		return ret;
+	}
+
+	if (value_ptr != NULL)
+		*value_ptr = (uint32_t)(uintptr_t)res;
+
+	return 0;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	return pthread_detach((pthread_t)thread_id.opaque_id);
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 6d8f89a..b404343 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -428,6 +428,11 @@ EXPERIMENTAL {
 	rte_thread_self;
 	rte_thread_set_affinity_by_id;
 	rte_thread_set_priority;
+
+	# added in 22.11
+	rte_thread_create;
+	rte_thread_detach;
+	rte_thread_join;
 };
 
 INTERNAL {
diff --git a/lib/eal/windows/include/sched.h b/lib/eal/windows/include/sched.h
index bc31cc8..912fed1 100644
--- a/lib/eal/windows/include/sched.h
+++ b/lib/eal/windows/include/sched.h
@@ -44,7 +44,7 @@
 	(1LL << _WHICH_BIT(b))) != 0LL)
 
 static inline int
-count_cpu(rte_cpuset_t *s)
+count_cpu(const rte_cpuset_t *s)
 {
 	unsigned int _i;
 	int count = 0;
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index 0771525..ad71be4 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -13,6 +13,11 @@ struct eal_tls_key {
 	DWORD thread_index;
 };
 
+struct thread_routine_ctx {
+	rte_thread_func thread_func;
+	void *routine_args;
+};
+
 /* Translates the most common error codes related to threads */
 static int
 thread_translate_win32_error(DWORD error)
@@ -114,6 +119,174 @@ struct eal_tls_key {
 	return 0;
 }
 
+static int
+convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
+		PGROUP_AFFINITY affinity)
+{
+	int ret = 0;
+	PGROUP_AFFINITY cpu_affinity = NULL;
+	unsigned int cpu_idx;
+
+	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 (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) {
+			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+
+		affinity->Mask |= cpu_affinity->Mask;
+	}
+
+	if (affinity->Mask == 0) {
+		ret = EINVAL;
+		goto cleanup;
+	}
+
+cleanup:
+	return ret;
+}
+
+static DWORD
+thread_func_wrapper(void *arg)
+{
+	struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg;
+
+	free(arg);
+
+	return (DWORD)ctx.thread_func(ctx.routine_args);
+}
+
+int
+rte_thread_create(rte_thread_t *thread_id,
+		  const rte_thread_attr_t *thread_attr,
+		  rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	DWORD tid;
+	HANDLE thread_handle = NULL;
+	GROUP_AFFINITY thread_affinity;
+	struct thread_routine_ctx *ctx;
+
+	ctx = calloc(1, sizeof(*ctx));
+	if (ctx == NULL) {
+		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
+		ret = ENOMEM;
+		goto cleanup;
+	}
+	ctx->routine_args = args;
+	ctx->thread_func = thread_func;
+
+	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
+		CREATE_SUSPENDED, &tid);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("CreateThread()");
+		goto cleanup;
+	}
+	thread_id->opaque_id = tid;
+
+	if (thread_attr != NULL) {
+		if (CPU_COUNT(&thread_attr->cpuset) > 0) {
+			ret = convert_cpuset_to_affinity(
+							&thread_attr->cpuset,
+							&thread_affinity
+							);
+			if (ret != 0) {
+				RTE_LOG(DEBUG, EAL, "Unable to convert cpuset to thread affinity\n");
+				goto cleanup;
+			}
+
+			if (!SetThreadGroupAffinity(thread_handle,
+						    &thread_affinity, NULL)) {
+				ret = thread_log_last_error("SetThreadGroupAffinity()");
+				goto cleanup;
+			}
+		}
+		ret = rte_thread_set_priority(*thread_id,
+				thread_attr->priority);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "Unable to set thread priority\n");
+			goto cleanup;
+		}
+	}
+
+	if (ResumeThread(thread_handle) == (DWORD)-1) {
+		ret = thread_log_last_error("ResumeThread()");
+		goto cleanup;
+	}
+
+	ctx = NULL;
+cleanup:
+	free(ctx);
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, uint32_t *value_ptr)
+{
+	HANDLE thread_handle;
+	DWORD result;
+	DWORD exit_code = 0;
+	BOOL err;
+	int ret = 0;
+
+	thread_handle = OpenThread(SYNCHRONIZE | THREAD_QUERY_INFORMATION,
+				   FALSE, thread_id.opaque_id);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("OpenThread()");
+		goto cleanup;
+	}
+
+	result = WaitForSingleObject(thread_handle, INFINITE);
+	if (result != WAIT_OBJECT_0) {
+		ret = thread_log_last_error("WaitForSingleObject()");
+		goto cleanup;
+	}
+
+	if (value_ptr != NULL) {
+		err = GetExitCodeThread(thread_handle, &exit_code);
+		if (err == 0) {
+			ret = thread_log_last_error("GetExitCodeThread()");
+			goto cleanup;
+		}
+		*value_ptr = exit_code;
+	}
+
+cleanup:
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	/* No resources that need to be released. */
+	RTE_SET_USED(thread_id);
+
+	return 0;
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
@@ -278,46 +451,6 @@ struct eal_tls_key {
 	return output;
 }
 
-static int
-convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
-		PGROUP_AFFINITY affinity)
-{
-	int ret = 0;
-	PGROUP_AFFINITY cpu_affinity = NULL;
-	unsigned int cpu_idx;
-
-	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 (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) {
-			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
-			ret = ENOTSUP;
-			goto cleanup;
-		}
-
-		affinity->Mask |= cpu_affinity->Mask;
-	}
-
-	if (affinity->Mask == 0) {
-		ret = EINVAL;
-		goto cleanup;
-	}
-
-cleanup:
-	return ret;
-}
-
 int
 rte_thread_set_affinity_by_id(rte_thread_t thread_id,
 		const rte_cpuset_t *cpuset)
-- 
1.8.3.1

[PATCH v4 3/6] eal: add basic rte thread ID equal API

[Tyler Retzlaff]

Add rte_thread_equal() that tests if two rte_thread_id are equal.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/eal/include/rte_thread.h | 19 +++++++++++++++++++
 lib/eal/unix/rte_thread.c    |  6 ++++++
 lib/eal/version.map          |  1 +
 lib/eal/windows/rte_thread.c |  6 ++++++
 4 files changed, 32 insertions(+)

diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 3c8ff50..8359c1c 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -144,6 +144,25 @@ int rte_thread_create(rte_thread_t *thread_id,
 __rte_experimental
 rte_thread_t rte_thread_self(void);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Check if 2 thread ids are equal.
+ *
+ * @param t1
+ *   First thread id.
+ *
+ * @param t2
+ *   Second thread id.
+ *
+ * @return
+ *   If the ids are equal, return nonzero.
+ *   Otherwise, return 0.
+ */
+__rte_experimental
+int rte_thread_equal(rte_thread_t t1, rte_thread_t t2);
+
 #ifdef RTE_HAS_CPUSET
 
 /**
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index d4c1a7f..37ebfcf 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -210,6 +210,12 @@ struct thread_routine_ctx {
 	return pthread_detach((pthread_t)thread_id.opaque_id);
 }
 
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return pthread_equal((pthread_t)t1.opaque_id, (pthread_t)t2.opaque_id);
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
diff --git a/lib/eal/version.map b/lib/eal/version.map
index b404343..b5dc7f1 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -432,6 +432,7 @@ EXPERIMENTAL {
 	# added in 22.11
 	rte_thread_create;
 	rte_thread_detach;
+	rte_thread_equal;
 	rte_thread_join;
 };
 
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index ad71be4..1bc648e 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -287,6 +287,12 @@ struct thread_routine_ctx {
 	return 0;
 }
 
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return t1.opaque_id == t2.opaque_id;
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
-- 
1.8.3.1

[PATCH v4 4/6] test/threads: add tests for thread lifetime API

[Tyler Retzlaff]

test basic functionality and demonstrate use of following thread
lifetime api.

    * rte_thread_create
    * rte_thread_detach
    * rte_thread_join

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index b9d8b4e..1077373 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -14,7 +14,7 @@
 
 static uint32_t thread_id_ready;
 
-static void *
+static uint32_t
 thread_main(void *arg)
 {
 	*(rte_thread_t *)arg = rte_thread_self();
@@ -23,7 +23,55 @@
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 1)
 		;
 
-	return NULL;
+	return 0;
+}
+
+static int
+test_thread_create_join(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, &thread_main_id) == 0,
+		"Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_join(thread_id, NULL) == 0,
+		"Failed to join thread.");
+
+	return 0;
+}
+
+static int
+test_thread_create_detach(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main,
+		&thread_main_id) == 0, "Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_detach(thread_id) == 0,
+		"Failed to detach thread.");
+
+	return 0;
 }
 
 static int
@@ -123,6 +171,8 @@
 	.setup = NULL,
 	.teardown = NULL,
 	.unit_test_cases = {
+		TEST_CASE(test_thread_create_join),
+		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
 		TEST_CASES_END()
-- 
1.8.3.1

[PATCH v4 5/6] test/threads: add tests for thread attributes API

[Tyler Retzlaff]

test basic functionality and demonstrate use of following thread
attributes api. additionally, test attributes are processed when
supplied to rte_thread_create().

    * rte_thread_attr_init
    * rte_thread_attr_set_affinity
    * rte_thread_attr_get_affinity
    * rte_thread_attr_set_priority

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 1077373..3c22cec 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -17,7 +17,9 @@
 static uint32_t
 thread_main(void *arg)
 {
-	*(rte_thread_t *)arg = rte_thread_self();
+	if (arg != NULL)
+		*(rte_thread_t *)arg = rte_thread_self();
+
 	__atomic_store_n(&thread_id_ready, 1, __ATOMIC_RELEASE);
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 1)
@@ -166,6 +168,73 @@
 	return 0;
 }
 
+static int
+test_thread_attributes_affinity(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	rte_cpuset_t cpuset0;
+	rte_cpuset_t cpuset1;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+
+	CPU_ZERO(&cpuset0);
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(rte_thread_self(), &cpuset0) == 0,
+		"Failed to get thread affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_set_affinity(&attr, &cpuset0) == 0,
+		"Failed to set thread attributes affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_get_affinity(&attr, &cpuset1) == 0,
+		"Failed to get thread attributes affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Affinity should be stable");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes affinity thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
+		"Failed to get attributes thread affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Failed to apply affinity attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
+static int
+test_thread_attributes_priority(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	enum rte_thread_priority priority;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+	RTE_TEST_ASSERT(rte_thread_attr_set_priority(&attr, RTE_THREAD_PRIORITY_NORMAL) == 0,
+		"Failed to set thread attributes priority");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes priority thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_priority(thread_id, &priority) == 0,
+		"Failed to get thread priority");
+	RTE_TEST_ASSERT(priority == RTE_THREAD_PRIORITY_NORMAL,
+		"Failed to apply priority attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
 static struct unit_test_suite threads_test_suite = {
 	.suite_name = "threads autotest",
 	.setup = NULL,
@@ -175,6 +244,8 @@
 		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
+		TEST_CASE(test_thread_attributes_affinity),
+		TEST_CASE(test_thread_attributes_priority),
 		TEST_CASES_END()
 	}
 };
-- 
1.8.3.1

[PATCH v4 6/6] test/threads: remove unit test use of pthread

[Tyler Retzlaff]

now that rte_thread provides thread lifetime functions stop using
pthread in unit tests.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 3c22cec..e0f18e4 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -3,7 +3,6 @@
  */
 
 #include <string.h>
-#include <pthread.h>
 
 #include <rte_thread.h>
 #include <rte_debug.h>
@@ -79,12 +78,11 @@
 static int
 test_thread_priority(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	enum rte_thread_priority priority;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
@@ -131,13 +129,12 @@
 static int
 test_thread_affinity(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	rte_cpuset_t cpuset0;
 	rte_cpuset_t cpuset1;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
-- 
1.8.3.1

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[Dmitry Kozlyuk]

2022-06-27 09:56 (UTC-0700), Tyler Retzlaff:
> add rte thread lifetime and attributes api. with these api additions
> there is now sufficient platform abstracted thread api to remove the
> use of pthread in the unit tests.

For series,
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Sorry it took so long to review.

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[David Marchand]

On Mon, Jun 27, 2022 at 6:56 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> add rte thread lifetime and attributes api. with these api additions
> there is now sufficient platform abstracted thread api to remove the
> use of pthread in the unit tests.
>
> v4:
>   * update version.map to show api from series added in 22.11 instead
>     of 22.07.

Patch 1 still adds symbols in 22.07 section of the experimental block.

Compilation is broken when stopping at patch 4.

Newly added code can go to eal_common_thread.c rather than introduce a
new common/rte_thread.c file (or is there a rationale for this?).

There are guards for the new structs against RTE_HAS_CPUSET being defined.
Can you elaborate why those guards are needed?

Commitlogs / comments sentences must start with a capital letter.

Can you prepare a new revision for -rc1?

Thanks.


>   * fix missing parameter name in rte_thread_func declaration causing
>     doxygen ci failure.


-- 
David Marchand

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[David Marchand]

On Wed, Sep 21, 2022 at 10:15 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Mon, Jun 27, 2022 at 6:56 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > add rte thread lifetime and attributes api. with these api additions
> > there is now sufficient platform abstracted thread api to remove the
> > use of pthread in the unit tests.
> >
> > v4:
> >   * update version.map to show api from series added in 22.11 instead
> >     of 22.07.
>
> Patch 1 still adds symbols in 22.07 section of the experimental block.
>
> Compilation is broken when stopping at patch 4.
>
> Newly added code can go to eal_common_thread.c rather than introduce a
> new common/rte_thread.c file (or is there a rationale for this?).
>
> There are guards for the new structs against RTE_HAS_CPUSET being defined.
> Can you elaborate why those guards are needed?
>
> Commitlogs / comments sentences must start with a capital letter.
>
> Can you prepare a new revision for -rc1?

Any update?


-- 
David Marchand

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

hi David,

On Wed, Sep 21, 2022 at 10:15:36AM +0200, David Marchand wrote:
> On Mon, Jun 27, 2022 at 6:56 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > add rte thread lifetime and attributes api. with these api additions
> > there is now sufficient platform abstracted thread api to remove the
> > use of pthread in the unit tests.
> >
> > v4:
> >   * update version.map to show api from series added in 22.11 instead
> >     of 22.07.
> 
> Patch 1 still adds symbols in 22.07 section of the experimental block.

this looks like a goofup on my part, i will fix in the next revision.

> 
> Compilation is broken when stopping at patch 4.

this was because commit 72b452c5f2599f970f47fd17d3e8e5d60bfebe7a removed
#include of <errno.h> from rte_common.h

i will fix this in the next revision.

> 
> Newly added code can go to eal_common_thread.c rather than introduce a
> new common/rte_thread.c file (or is there a rationale for this?).

i will make this change in the next revision. if anyone does object i
hope they will do so quickly.

> 
> There are guards for the new structs against RTE_HAS_CPUSET being defined.
> Can you elaborate why those guards are needed?

eal uses this guard in a couple of places so the best explanation i can
provide is an existing pattern was being followed. that being said it's
clear that this code won't work without RTE_HAS_CPUSET.

i will remove the guard and test if the build breaks for the platforms
i have toolchains available. if it builds cleanly i'll remove the guard
in the next revision.

> 
> Commitlogs / comments sentences must start with a capital letter.

i will fix in next revision, but ew caps.

> 
> Can you prepare a new revision for -rc1?

assuming the above actions to address feedback are acceptable the new
revision should be up today (PST).

> 
> Thanks.

thank you!

ty

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

On Wed, Oct 05, 2022 at 09:11:26AM -0700, Tyler Retzlaff wrote:
> hi David,
> 
> 
> > 
> > Newly added code can go to eal_common_thread.c rather than introduce a
> > new common/rte_thread.c file (or is there a rationale for this?).
> 
> i will make this change in the next revision. if anyone does object i
> hope they will do so quickly.

looking at this more closely i'm going to back away from making the
adjustment here. if Thomas and/or Dmitry could comment it would be
appreciated.

it appears that functions placed in eal_common_xxx files are consumed
internally by the eal where rte_xxx files are functions that are exposed
through public api.

since these additions are public api it seems they should remain in
rte_thread.c
 
i won't change this in the next revision, but please do correct me if
i'm still not on track.

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[David Marchand]

On Wed, Oct 5, 2022 at 6:34 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> On Wed, Oct 05, 2022 at 09:11:26AM -0700, Tyler Retzlaff wrote:
> > hi David,
> >
> >
> > >
> > > Newly added code can go to eal_common_thread.c rather than introduce a
> > > new common/rte_thread.c file (or is there a rationale for this?).
> >
> > i will make this change in the next revision. if anyone does object i
> > hope they will do so quickly.
>
> looking at this more closely i'm going to back away from making the
> adjustment here. if Thomas and/or Dmitry could comment it would be
> appreciated.
>
> it appears that functions placed in eal_common_xxx files are consumed
> internally by the eal where rte_xxx files are functions that are exposed
> through public api.
>
> since these additions are public api it seems they should remain in
> rte_thread.c
>
> i won't change this in the next revision, but please do correct me if
> i'm still not on track.

This seems a fair argument.


I'll reply on the v5 series: compilation is still broken at patch 4.


-- 
David Marchand

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

On Thu, Oct 06, 2022 at 08:52:02AM +0200, David Marchand wrote:
> On Wed, Oct 5, 2022 at 6:34 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > On Wed, Oct 05, 2022 at 09:11:26AM -0700, Tyler Retzlaff wrote:
> > > hi David,
> > >
> > >
> > > >
> > > > Newly added code can go to eal_common_thread.c rather than introduce a
> > > > new common/rte_thread.c file (or is there a rationale for this?).
> > >
> > > i will make this change in the next revision. if anyone does object i
> > > hope they will do so quickly.
> >
> > looking at this more closely i'm going to back away from making the
> > adjustment here. if Thomas and/or Dmitry could comment it would be
> > appreciated.
> >
> > it appears that functions placed in eal_common_xxx files are consumed
> > internally by the eal where rte_xxx files are functions that are exposed
> > through public api.
> >
> > since these additions are public api it seems they should remain in
> > rte_thread.c
> >
> > i won't change this in the next revision, but please do correct me if
> > i'm still not on track.
> 
> This seems a fair argument.
> 
> 
> I'll reply on the v5 series: compilation is still broken at patch 4.

can you share what the break is? and which platform/target/toolchain?

the only thing I found was that the removal of errno.h broke compilation
but that is fixed in v5. the CI run for the series doesn't indicate any
compilation failures. assuming i'm looking at the correct results.

https://lab.dpdk.org/results/dashboard/patchsets/23789/

it does show a unit test failure but it looks like it is related to a
service test that you mailed about within the past few days.

thanks

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[Thomas Monjalon]

05/10/2022 18:34, Tyler Retzlaff:
> On Wed, Oct 05, 2022 at 09:11:26AM -0700, Tyler Retzlaff wrote:
> > > Newly added code can go to eal_common_thread.c rather than introduce a
> > > new common/rte_thread.c file (or is there a rationale for this?).
> > 
> > i will make this change in the next revision. if anyone does object i
> > hope they will do so quickly.
> 
> looking at this more closely i'm going to back away from making the
> adjustment here. if Thomas and/or Dmitry could comment it would be
> appreciated.
> 
> it appears that functions placed in eal_common_xxx files are consumed
> internally by the eal where rte_xxx files are functions that are exposed
> through public api.

It is not so clear.
There is already eal_common_thread.c which implements the same kind of functions,
so I think you should move your new functions here.

> since these additions are public api it seems they should remain in
> rte_thread.c

Let's not have 2 .c files for the same purpose in the same directory.

> i won't change this in the next revision, but please do correct me if
> i'm still not on track.

Please I would prefer to keep all in one file: eal_common_thread.c
Thank you

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

On Thu, Oct 06, 2022 at 03:36:12PM +0200, Thomas Monjalon wrote:
> 05/10/2022 18:34, Tyler Retzlaff:
> > On Wed, Oct 05, 2022 at 09:11:26AM -0700, Tyler Retzlaff wrote:
> > > > Newly added code can go to eal_common_thread.c rather than introduce a
> > > > new common/rte_thread.c file (or is there a rationale for this?).
> > > 
> > > i will make this change in the next revision. if anyone does object i
> > > hope they will do so quickly.
> > 
> > looking at this more closely i'm going to back away from making the
> > adjustment here. if Thomas and/or Dmitry could comment it would be
> > appreciated.
> > 
> > it appears that functions placed in eal_common_xxx files are consumed
> > internally by the eal where rte_xxx files are functions that are exposed
> > through public api.
> 
> It is not so clear.
> There is already eal_common_thread.c which implements the same kind of functions,
> so I think you should move your new functions here.
> 
> > since these additions are public api it seems they should remain in
> > rte_thread.c
> 
> Let's not have 2 .c files for the same purpose in the same directory.

just as another point there seem to be several other rte_xxx.c files
here can we clarify why they were not subject to the same requirement?
as a follow on does it mean that the code in those files should also be
moved to eal_common_xxx files?

please let me know if the justification is not the same i'll move the
functions to the eal_common file as requested. i just want to make sure
it is being done for the consistent/correct reason.

thanks.

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[Thomas Monjalon]

06/10/2022 17:10, Tyler Retzlaff:
> On Thu, Oct 06, 2022 at 03:36:12PM +0200, Thomas Monjalon wrote:
> > 05/10/2022 18:34, Tyler Retzlaff:
> > > On Wed, Oct 05, 2022 at 09:11:26AM -0700, Tyler Retzlaff wrote:
> > > > > Newly added code can go to eal_common_thread.c rather than introduce a
> > > > > new common/rte_thread.c file (or is there a rationale for this?).
> > > > 
> > > > i will make this change in the next revision. if anyone does object i
> > > > hope they will do so quickly.
> > > 
> > > looking at this more closely i'm going to back away from making the
> > > adjustment here. if Thomas and/or Dmitry could comment it would be
> > > appreciated.
> > > 
> > > it appears that functions placed in eal_common_xxx files are consumed
> > > internally by the eal where rte_xxx files are functions that are exposed
> > > through public api.
> > 
> > It is not so clear.
> > There is already eal_common_thread.c which implements the same kind of functions,
> > so I think you should move your new functions here.
> > 
> > > since these additions are public api it seems they should remain in
> > > rte_thread.c
> > 
> > Let's not have 2 .c files for the same purpose in the same directory.
> 
> just as another point there seem to be several other rte_xxx.c files
> here can we clarify why they were not subject to the same requirement?
> as a follow on does it mean that the code in those files should also be
> moved to eal_common_xxx files?

That's just history.

> please let me know if the justification is not the same i'll move the
> functions to the eal_common file as requested. i just want to make sure
> it is being done for the consistent/correct reason.

Some file names are not correct, we could rename them.

I think David is already doing the last minor changes on this series
while merging, so no need to do anything on your side.

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

On Thu, Oct 06, 2022 at 05:14:55PM +0200, Thomas Monjalon wrote:
> 06/10/2022 17:10, Tyler Retzlaff:
> > On Thu, Oct 06, 2022 at 03:36:12PM +0200, Thomas Monjalon wrote:
> > > 05/10/2022 18:34, Tyler Retzlaff:
> > > > On Wed, Oct 05, 2022 at 09:11:26AM -0700, Tyler Retzlaff wrote:
> > > > > > Newly added code can go to eal_common_thread.c rather than introduce a
> > > > > > new common/rte_thread.c file (or is there a rationale for this?).
> > > > > 
> > > > > i will make this change in the next revision. if anyone does object i
> > > > > hope they will do so quickly.
> > > > 
> > > > looking at this more closely i'm going to back away from making the
> > > > adjustment here. if Thomas and/or Dmitry could comment it would be
> > > > appreciated.
> > > > 
> > > > it appears that functions placed in eal_common_xxx files are consumed
> > > > internally by the eal where rte_xxx files are functions that are exposed
> > > > through public api.
> > > 
> > > It is not so clear.
> > > There is already eal_common_thread.c which implements the same kind of functions,
> > > so I think you should move your new functions here.
> > > 
> > > > since these additions are public api it seems they should remain in
> > > > rte_thread.c
> > > 
> > > Let's not have 2 .c files for the same purpose in the same directory.
> > 
> > just as another point there seem to be several other rte_xxx.c files
> > here can we clarify why they were not subject to the same requirement?
> > as a follow on does it mean that the code in those files should also be
> > moved to eal_common_xxx files?
> 
> That's just history.
> 
> > please let me know if the justification is not the same i'll move the
> > functions to the eal_common file as requested. i just want to make sure
> > it is being done for the consistent/correct reason.
> 
> Some file names are not correct, we could rename them.
> 
> I think David is already doing the last minor changes on this series
> while merging, so no need to do anything on your side.
> 

Thomas, David with this just a final confirmation no need for a v6?
you're tweaking the series as is for final comments?

thanks

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[David Marchand]

On Thu, Oct 6, 2022 at 5:20 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> On Thu, Oct 06, 2022 at 05:14:55PM +0200, Thomas Monjalon wrote:
> > 06/10/2022 17:10, Tyler Retzlaff:
> > > On Thu, Oct 06, 2022 at 03:36:12PM +0200, Thomas Monjalon wrote:
> > > > 05/10/2022 18:34, Tyler Retzlaff:
> > > > > On Wed, Oct 05, 2022 at 09:11:26AM -0700, Tyler Retzlaff wrote:
> > > > > > > Newly added code can go to eal_common_thread.c rather than introduce a
> > > > > > > new common/rte_thread.c file (or is there a rationale for this?).
> > > > > >
> > > > > > i will make this change in the next revision. if anyone does object i
> > > > > > hope they will do so quickly.
> > > > >
> > > > > looking at this more closely i'm going to back away from making the
> > > > > adjustment here. if Thomas and/or Dmitry could comment it would be
> > > > > appreciated.
> > > > >
> > > > > it appears that functions placed in eal_common_xxx files are consumed
> > > > > internally by the eal where rte_xxx files are functions that are exposed
> > > > > through public api.
> > > >
> > > > It is not so clear.
> > > > There is already eal_common_thread.c which implements the same kind of functions,
> > > > so I think you should move your new functions here.
> > > >
> > > > > since these additions are public api it seems they should remain in
> > > > > rte_thread.c
> > > >
> > > > Let's not have 2 .c files for the same purpose in the same directory.
> > >
> > > just as another point there seem to be several other rte_xxx.c files
> > > here can we clarify why they were not subject to the same requirement?
> > > as a follow on does it mean that the code in those files should also be
> > > moved to eal_common_xxx files?
> >
> > That's just history.
> >
> > > please let me know if the justification is not the same i'll move the
> > > functions to the eal_common file as requested. i just want to make sure
> > > it is being done for the consistent/correct reason.
> >
> > Some file names are not correct, we could rename them.
> >
> > I think David is already doing the last minor changes on this series
> > while merging, so no need to do anything on your side.
> >
>
> Thomas, David with this just a final confirmation no need for a v6?
> you're tweaking the series as is for final comments?

No need for a v6, the code move is trivial, and for the rest, I'm finished.
I'll restart the per patch build all over again to be sure, and then
merge the series probably tonight (CET+2).


-- 
David Marchand

Re: [PATCH v4 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

On Thu, Oct 06, 2022 at 05:26:10PM +0200, David Marchand wrote:
> On Thu, Oct 6, 2022 at 5:20 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > On Thu, Oct 06, 2022 at 05:14:55PM +0200, Thomas Monjalon wrote:
> > > 06/10/2022 17:10, Tyler Retzlaff:
> > > > On Thu, Oct 06, 2022 at 03:36:12PM +0200, Thomas Monjalon wrote:
> > > > > 05/10/2022 18:34, Tyler Retzlaff:
> > > > > > On Wed, Oct 05, 2022 at 09:11:26AM -0700, Tyler Retzlaff wrote:
> > > > > > > > Newly added code can go to eal_common_thread.c rather than introduce a
> > > > > > > > new common/rte_thread.c file (or is there a rationale for this?).
> > > > > > >
> > > > > > > i will make this change in the next revision. if anyone does object i
> > > > > > > hope they will do so quickly.
> > > > > >
> > > > > > looking at this more closely i'm going to back away from making the
> > > > > > adjustment here. if Thomas and/or Dmitry could comment it would be
> > > > > > appreciated.
> > > > > >
> > > > > > it appears that functions placed in eal_common_xxx files are consumed
> > > > > > internally by the eal where rte_xxx files are functions that are exposed
> > > > > > through public api.
> > > > >
> > > > > It is not so clear.
> > > > > There is already eal_common_thread.c which implements the same kind of functions,
> > > > > so I think you should move your new functions here.
> > > > >
> > > > > > since these additions are public api it seems they should remain in
> > > > > > rte_thread.c
> > > > >
> > > > > Let's not have 2 .c files for the same purpose in the same directory.
> > > >
> > > > just as another point there seem to be several other rte_xxx.c files
> > > > here can we clarify why they were not subject to the same requirement?
> > > > as a follow on does it mean that the code in those files should also be
> > > > moved to eal_common_xxx files?
> > >
> > > That's just history.
> > >
> > > > please let me know if the justification is not the same i'll move the
> > > > functions to the eal_common file as requested. i just want to make sure
> > > > it is being done for the consistent/correct reason.
> > >
> > > Some file names are not correct, we could rename them.
> > >
> > > I think David is already doing the last minor changes on this series
> > > while merging, so no need to do anything on your side.
> > >
> >
> > Thomas, David with this just a final confirmation no need for a v6?
> > you're tweaking the series as is for final comments?
> 
> No need for a v6, the code move is trivial, and for the rest, I'm finished.
> I'll restart the per patch build all over again to be sure, and then
> merge the series probably tonight (CET+2).

acknowledged.

thanks very much!

> 
> 
> -- 
> David Marchand

[PATCH v5 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

add rte thread lifetime and attributes api. with these api additions
there is now sufficient platform abstracted thread api to remove the
use of pthread in the unit tests.

v5:
  * include errno.h in rte_thread.c since errno.h is no longer included
    in rte_common.h
  * move rte_thread_attr symbols from 22.07 to 22.11 section of
    version.map.
  * remove RTE_HAS_CPUSET guards from rte_thread.h
  * change case of characters in commit message for patches {6,5,4} of
    the series.

v4:
  * update version.map to show api from series added in 22.11 instead
    of 22.07.
  * fix missing parameter name in rte_thread_func declaration causing
    doxygen ci failure.

v3:
  * change rte_thread_func return type to uint32_t for exit value.
  * change rte_thread_join retval to be uint32_t (matched with the
    return value from rte_thread_func).
  * introduce a wrapper for rte_thread_func on posix platforms to
    adapt differences between rte_thread_func and pthread
    start_routine.
  * remove interpretation / dereference of result from pthread_join
    in posix implementation of rte_thread_join.
  * fix leak of dynamically allocated thread_routine_ctx on windows
    in error paths.
  * don't cast and truncate NULL to integer value for rte_thread_join
    when pthread_join returns no result.

v2:
  * split implementation of rte_thread_equal for windows / posix
    and use pthread_equal for posix platforms.
  * remove parameter validation assertions and instead return
    EINVAL for mandatory pointers to type that are NULL.
  * correct doxygen comment parameter name args -> arg

Tyler Retzlaff (6):
  eal: add thread attributes
  eal: add thread lifetime management
  eal: add basic rte thread ID equal API
  test/threads: add tests for thread lifetime API
  test/threads: add tests for thread attributes API
  test/threads: remove unit test use of pthread

 app/test/test_threads.c         | 134 ++++++++++++++++++++++--
 lib/eal/common/meson.build      |   1 +
 lib/eal/common/rte_thread.c     |  62 ++++++++++++
 lib/eal/include/rte_thread.h    | 185 ++++++++++++++++++++++++++++++++-
 lib/eal/unix/rte_thread.c       | 141 ++++++++++++++++++++++++++
 lib/eal/version.map             |  10 ++
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 219 ++++++++++++++++++++++++++++++++--------
 8 files changed, 702 insertions(+), 52 deletions(-)
 create mode 100644 lib/eal/common/rte_thread.c

-- 
1.8.3.1

[PATCH v5 1/6] eal: add thread attributes

[Tyler Retzlaff]

Implement thread attributes for:

    * thread affinity
    * thread priority

Implement functions for managing thread attributes.

  Priority is represented through an enum that allows for two levels:

    * RTE_THREAD_PRIORITY_NORMAL
    * RTE_THREAD_PRIORITY_REALTIME_CRITICAL

  Affinity is described by the rte_cpuset_t type.

An rte_thread_attr_t object can be set to the default values
by calling rte_thread_attr_init().

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/common/meson.build   |   1 +
 lib/eal/common/rte_thread.c  |  62 ++++++++++++++++++++++++
 lib/eal/include/rte_thread.h | 110 +++++++++++++++++++++++++++++++++++++++++--
 lib/eal/version.map          |   6 +++
 4 files changed, 176 insertions(+), 3 deletions(-)
 create mode 100644 lib/eal/common/rte_thread.c

diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
index 917758c..1e77dba 100644
--- a/lib/eal/common/meson.build
+++ b/lib/eal/common/meson.build
@@ -36,6 +36,7 @@ sources += files(
         'rte_random.c',
         'rte_reciprocal.c',
         'rte_service.c',
+        'rte_thread.c',
         'rte_version.c',
 )
 if is_linux or is_windows
diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
new file mode 100644
index 0000000..760ad62
--- /dev/null
+++ b/lib/eal/common/rte_thread.c
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2022 Microsoft Corporation
+ */
+
+#include <errno.h>
+
+#include <rte_debug.h>
+#include <rte_thread.h>
+
+int
+rte_thread_attr_init(rte_thread_attr_t *attr)
+{
+	if (attr == NULL)
+		return EINVAL;
+
+	CPU_ZERO(&attr->cpuset);
+	attr->priority = RTE_THREAD_PRIORITY_NORMAL;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	if (cpuset == NULL)
+		return EINVAL;
+
+	thread_attr->cpuset = *cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	if (cpuset == NULL)
+		return EINVAL;
+
+	*cpuset = thread_attr->cpuset;
+
+	return 0;
+}
+
+int
+rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority)
+{
+	if (thread_attr == NULL)
+		return EINVAL;
+
+	thread_attr->priority = priority;
+
+	return 0;
+}
diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 9e261bf..a3802de 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -41,6 +41,14 @@ enum rte_thread_priority {
 };
 
 /**
+ * Representation for thread attributes.
+ */
+typedef struct {
+	enum rte_thread_priority priority; /**< thread priority */
+	rte_cpuset_t cpuset; /**< thread affinity */
+} rte_thread_attr_t;
+
+/**
  * TLS key type, an opaque pointer.
  */
 typedef struct eal_tls_key *rte_thread_key;
@@ -57,7 +65,105 @@ enum rte_thread_priority {
 __rte_experimental
 rte_thread_t rte_thread_self(void);
 
-#ifdef RTE_HAS_CPUSET
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Check if 2 thread ids are equal.
+ *
+ * @param t1
+ *   First thread id.
+ *
+ * @param t2
+ *   Second thread id.
+ *
+ * @return
+ *   If the ids are equal, return nonzero.
+ *   Otherwise, return 0.
+ */
+__rte_experimental
+int rte_thread_equal(rte_thread_t t1, rte_thread_t t2);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Initialize the attributes of a thread.
+ * These attributes can be passed to the rte_thread_create() function
+ * that will create a new thread and set its attributes according to attr.
+ *
+ * @param attr
+ *   Thread attributes to initialize.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_init(rte_thread_attr_t *attr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the CPU affinity value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which affinity will be updated.
+ *
+ * @param cpuset
+ *   Points to the value of the affinity to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get the value of CPU affinity that is set in the thread attributes pointed
+ * to by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes from which affinity will be retrieved.
+ *
+ * @param cpuset
+ *   Pointer to the memory that will store the affinity.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_get_affinity(rte_thread_attr_t *thread_attr,
+		rte_cpuset_t *cpuset);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Set the thread priority value in the thread attributes pointed to
+ * by 'thread_attr'.
+ *
+ * @param thread_attr
+ *   Points to the thread attributes in which priority will be updated.
+ *
+ * @param priority
+ *   Points to the value of the priority to be set.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_attr_set_priority(rte_thread_attr_t *thread_attr,
+		enum rte_thread_priority priority);
 
 /**
  * @warning
@@ -122,8 +228,6 @@ int rte_thread_get_affinity_by_id(rte_thread_t thread_id,
  */
 void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
 
-#endif /* RTE_HAS_CPUSET */
-
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
diff --git a/lib/eal/version.map b/lib/eal/version.map
index e617ba7..e9df18d 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -430,6 +430,12 @@ EXPERIMENTAL {
 	rte_thread_self;
 	rte_thread_set_affinity_by_id;
 	rte_thread_set_priority;
+
+	# added in 22.11
+	rte_thread_attr_get_affinity;
+	rte_thread_attr_init;
+	rte_thread_attr_set_affinity;
+	rte_thread_attr_set_priority;
 };
 
 INTERNAL {
-- 
1.8.3.1

Re: [PATCH v5 1/6] eal: add thread attributes

[David Marchand]

On Wed, Oct 5, 2022 at 7:07 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
> @@ -57,7 +65,105 @@ enum rte_thread_priority {
>  __rte_experimental
>  rte_thread_t rte_thread_self(void);
>
> -#ifdef RTE_HAS_CPUSET
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Check if 2 thread ids are equal.
> + *
> + * @param t1
> + *   First thread id.
> + *
> + * @param t2
> + *   Second thread id.
> + *
> + * @return
> + *   If the ids are equal, return nonzero.
> + *   Otherwise, return 0.
> + */
> +__rte_experimental
> +int rte_thread_equal(rte_thread_t t1, rte_thread_t t2);

Rebase damage, I'll fix when applying.


-- 
David Marchand

[PATCH v5 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

The *rte_thread_create()* function can optionally receive an
rte_thread_attr_t object that will cause the thread to be created with
the affinity and priority described by the attributes object. If
no rte_thread_attr_t is passed (parameter is NULL), the default
affinity and priority are used.

On Windows, the function executed by a thread when the thread starts is
represeneted by a function pointer of type DWORD (*func) (void*).
On other platforms, the function pointer is a void* (*func) (void*).

Performing a cast between these two types of function pointers to
uniformize the API on all platforms may result in undefined behavior.
TO fix this issue, a wrapper that respects the signature required by
CreateThread() has been created on Windows.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/include/rte_thread.h    |  75 ++++++++++++++
 lib/eal/unix/rte_thread.c       | 135 +++++++++++++++++++++++++
 lib/eal/version.map             |   3 +
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 213 ++++++++++++++++++++++++++++++++--------
 5 files changed, 387 insertions(+), 41 deletions(-)

diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index a3802de..65cdcfa 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -31,6 +31,18 @@
 } rte_thread_t;
 
 /**
+ * Thread function
+ *
+ * Function pointer to thread start routine.
+ *
+ * @param arg
+ *   Argument passed to rte_thread_create().
+ * @return
+ *   Thread function exit value.
+ */
+typedef uint32_t (*rte_thread_func) (void *arg);
+
+/**
  * Thread priority values.
  */
 enum rte_thread_priority {
@@ -57,6 +69,69 @@ enum rte_thread_priority {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
+ * Create a new thread that will invoke the 'thread_func' routine.
+ *
+ * @param thread_id
+ *    A pointer that will store the id of the newly created thread.
+ *
+ * @param thread_attr
+ *    Attributes that are used at the creation of the new thread.
+ *
+ * @param thread_func
+ *    The routine that the new thread will invoke when starting execution.
+ *
+ * @param arg
+ *    Argument to be passed to the 'thread_func' routine.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *arg);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Waits for the thread identified by 'thread_id' to terminate
+ *
+ * @param thread_id
+ *    The identifier of the thread.
+ *
+ * @param value_ptr
+ *    Stores the exit status of the thread.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_join(rte_thread_t thread_id, uint32_t *value_ptr);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Indicate that the return value of the thread is not needed and
+ * all thread resources should be release when the thread terminates.
+ *
+ * @param thread_id
+ *    The id of the thread to be detached.
+ *
+ * @return
+ *   On success, return 0.
+ *   On failure, return a positive errno-style error number.
+ */
+__rte_experimental
+int rte_thread_detach(rte_thread_t thread_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
  * Get the id of the calling thread.
  *
  * @return
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index 9126595..d4c1a7f 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -16,6 +16,11 @@ struct eal_tls_key {
 	pthread_key_t thread_index;
 };
 
+struct thread_routine_ctx {
+	rte_thread_func thread_func;
+	void *routine_args;
+};
+
 static int
 thread_map_priority_to_os_value(enum rte_thread_priority eal_pri, int *os_pri,
 	int *pol)
@@ -75,6 +80,136 @@ struct eal_tls_key {
 	return 0;
 }
 
+static void *
+thread_func_wrapper(void *arg)
+{
+	struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg;
+
+	free(arg);
+
+	return (void *)(uintptr_t)ctx.thread_func(ctx.routine_args);
+}
+
+int
+rte_thread_create(rte_thread_t *thread_id,
+		const rte_thread_attr_t *thread_attr,
+		rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	pthread_attr_t attr;
+	pthread_attr_t *attrp = NULL;
+	struct thread_routine_ctx *ctx;
+	struct sched_param param = {
+		.sched_priority = 0,
+	};
+	int policy = SCHED_OTHER;
+
+	ctx = calloc(1, sizeof(*ctx));
+	if (ctx == NULL) {
+		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
+		ret = ENOMEM;
+		goto cleanup;
+	}
+	ctx->routine_args = args;
+	ctx->thread_func = thread_func;
+
+	if (thread_attr != NULL) {
+		ret = pthread_attr_init(&attr);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_init failed\n");
+			goto cleanup;
+		}
+
+		attrp = &attr;
+
+		/*
+		 * Set the inherit scheduler parameter to explicit,
+		 * otherwise the priority attribute is ignored.
+		 */
+		ret = pthread_attr_setinheritsched(attrp,
+				PTHREAD_EXPLICIT_SCHED);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setinheritsched failed\n");
+			goto cleanup;
+		}
+
+
+		if (thread_attr->priority ==
+				RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+		ret = thread_map_priority_to_os_value(thread_attr->priority,
+				&param.sched_priority, &policy);
+		if (ret != 0)
+			goto cleanup;
+
+		ret = pthread_attr_setschedpolicy(attrp, policy);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedpolicy failed\n");
+			goto cleanup;
+		}
+
+		ret = pthread_attr_setschedparam(attrp, &param);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "pthread_attr_setschedparam failed\n");
+			goto cleanup;
+		}
+	}
+
+	ret = pthread_create((pthread_t *)&thread_id->opaque_id, attrp,
+		thread_func_wrapper, ctx);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_create failed\n");
+		goto cleanup;
+	}
+
+	if (thread_attr != NULL && CPU_COUNT(&thread_attr->cpuset) > 0) {
+		ret = rte_thread_set_affinity_by_id(*thread_id,
+			&thread_attr->cpuset);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "rte_thread_set_affinity_by_id failed\n");
+			goto cleanup;
+		}
+	}
+
+	ctx = NULL;
+cleanup:
+	free(ctx);
+	if (attrp != NULL)
+		pthread_attr_destroy(&attr);
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, uint32_t *value_ptr)
+{
+	int ret = 0;
+	void *res = (void *)(uintptr_t)0;
+	void **pres = NULL;
+
+	if (value_ptr != NULL)
+		pres = &res;
+
+	ret = pthread_join((pthread_t)thread_id.opaque_id, pres);
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "pthread_join failed\n");
+		return ret;
+	}
+
+	if (value_ptr != NULL)
+		*value_ptr = (uint32_t)(uintptr_t)res;
+
+	return 0;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	return pthread_detach((pthread_t)thread_id.opaque_id);
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
diff --git a/lib/eal/version.map b/lib/eal/version.map
index e9df18d..5fd84b6 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -436,6 +436,9 @@ EXPERIMENTAL {
 	rte_thread_attr_init;
 	rte_thread_attr_set_affinity;
 	rte_thread_attr_set_priority;
+	rte_thread_create;
+	rte_thread_detach;
+	rte_thread_join;
 };
 
 INTERNAL {
diff --git a/lib/eal/windows/include/sched.h b/lib/eal/windows/include/sched.h
index bc31cc8..912fed1 100644
--- a/lib/eal/windows/include/sched.h
+++ b/lib/eal/windows/include/sched.h
@@ -44,7 +44,7 @@
 	(1LL << _WHICH_BIT(b))) != 0LL)
 
 static inline int
-count_cpu(rte_cpuset_t *s)
+count_cpu(const rte_cpuset_t *s)
 {
 	unsigned int _i;
 	int count = 0;
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index 63442ec..e153278 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -15,6 +15,11 @@ struct eal_tls_key {
 	DWORD thread_index;
 };
 
+struct thread_routine_ctx {
+	rte_thread_func thread_func;
+	void *routine_args;
+};
+
 /* Translates the most common error codes related to threads */
 static int
 thread_translate_win32_error(DWORD error)
@@ -116,6 +121,174 @@ struct eal_tls_key {
 	return 0;
 }
 
+static int
+convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
+		PGROUP_AFFINITY affinity)
+{
+	int ret = 0;
+	PGROUP_AFFINITY cpu_affinity = NULL;
+	unsigned int cpu_idx;
+
+	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 (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) {
+			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
+			ret = ENOTSUP;
+			goto cleanup;
+		}
+
+		affinity->Mask |= cpu_affinity->Mask;
+	}
+
+	if (affinity->Mask == 0) {
+		ret = EINVAL;
+		goto cleanup;
+	}
+
+cleanup:
+	return ret;
+}
+
+static DWORD
+thread_func_wrapper(void *arg)
+{
+	struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg;
+
+	free(arg);
+
+	return (DWORD)ctx.thread_func(ctx.routine_args);
+}
+
+int
+rte_thread_create(rte_thread_t *thread_id,
+		  const rte_thread_attr_t *thread_attr,
+		  rte_thread_func thread_func, void *args)
+{
+	int ret = 0;
+	DWORD tid;
+	HANDLE thread_handle = NULL;
+	GROUP_AFFINITY thread_affinity;
+	struct thread_routine_ctx *ctx;
+
+	ctx = calloc(1, sizeof(*ctx));
+	if (ctx == NULL) {
+		RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
+		ret = ENOMEM;
+		goto cleanup;
+	}
+	ctx->routine_args = args;
+	ctx->thread_func = thread_func;
+
+	thread_handle = CreateThread(NULL, 0, thread_func_wrapper, ctx,
+		CREATE_SUSPENDED, &tid);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("CreateThread()");
+		goto cleanup;
+	}
+	thread_id->opaque_id = tid;
+
+	if (thread_attr != NULL) {
+		if (CPU_COUNT(&thread_attr->cpuset) > 0) {
+			ret = convert_cpuset_to_affinity(
+							&thread_attr->cpuset,
+							&thread_affinity
+							);
+			if (ret != 0) {
+				RTE_LOG(DEBUG, EAL, "Unable to convert cpuset to thread affinity\n");
+				goto cleanup;
+			}
+
+			if (!SetThreadGroupAffinity(thread_handle,
+						    &thread_affinity, NULL)) {
+				ret = thread_log_last_error("SetThreadGroupAffinity()");
+				goto cleanup;
+			}
+		}
+		ret = rte_thread_set_priority(*thread_id,
+				thread_attr->priority);
+		if (ret != 0) {
+			RTE_LOG(DEBUG, EAL, "Unable to set thread priority\n");
+			goto cleanup;
+		}
+	}
+
+	if (ResumeThread(thread_handle) == (DWORD)-1) {
+		ret = thread_log_last_error("ResumeThread()");
+		goto cleanup;
+	}
+
+	ctx = NULL;
+cleanup:
+	free(ctx);
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_join(rte_thread_t thread_id, uint32_t *value_ptr)
+{
+	HANDLE thread_handle;
+	DWORD result;
+	DWORD exit_code = 0;
+	BOOL err;
+	int ret = 0;
+
+	thread_handle = OpenThread(SYNCHRONIZE | THREAD_QUERY_INFORMATION,
+				   FALSE, thread_id.opaque_id);
+	if (thread_handle == NULL) {
+		ret = thread_log_last_error("OpenThread()");
+		goto cleanup;
+	}
+
+	result = WaitForSingleObject(thread_handle, INFINITE);
+	if (result != WAIT_OBJECT_0) {
+		ret = thread_log_last_error("WaitForSingleObject()");
+		goto cleanup;
+	}
+
+	if (value_ptr != NULL) {
+		err = GetExitCodeThread(thread_handle, &exit_code);
+		if (err == 0) {
+			ret = thread_log_last_error("GetExitCodeThread()");
+			goto cleanup;
+		}
+		*value_ptr = exit_code;
+	}
+
+cleanup:
+	if (thread_handle != NULL) {
+		CloseHandle(thread_handle);
+		thread_handle = NULL;
+	}
+
+	return ret;
+}
+
+int
+rte_thread_detach(rte_thread_t thread_id)
+{
+	/* No resources that need to be released. */
+	RTE_SET_USED(thread_id);
+
+	return 0;
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
@@ -280,46 +453,6 @@ struct eal_tls_key {
 	return output;
 }
 
-static int
-convert_cpuset_to_affinity(const rte_cpuset_t *cpuset,
-		PGROUP_AFFINITY affinity)
-{
-	int ret = 0;
-	PGROUP_AFFINITY cpu_affinity = NULL;
-	unsigned int cpu_idx;
-
-	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 (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) {
-			RTE_LOG(DEBUG, EAL, "All processors must belong to the same processor group\n");
-			ret = ENOTSUP;
-			goto cleanup;
-		}
-
-		affinity->Mask |= cpu_affinity->Mask;
-	}
-
-	if (affinity->Mask == 0) {
-		ret = EINVAL;
-		goto cleanup;
-	}
-
-cleanup:
-	return ret;
-}
-
 int
 rte_thread_set_affinity_by_id(rte_thread_t thread_id,
 		const rte_cpuset_t *cpuset)
-- 
1.8.3.1

Re: [PATCH v5 2/6] eal: add thread lifetime management

[David Marchand]

Hello Tyler,

On Wed, Oct 5, 2022 at 7:07 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
> diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
> index 9126595..d4c1a7f 100644
> --- a/lib/eal/unix/rte_thread.c
> +++ b/lib/eal/unix/rte_thread.c
> @@ -16,6 +16,11 @@ struct eal_tls_key {
>         pthread_key_t thread_index;
>  };
>
> +struct thread_routine_ctx {
> +       rte_thread_func thread_func;
> +       void *routine_args;
> +};
> +
>  static int
>  thread_map_priority_to_os_value(enum rte_thread_priority eal_pri, int *os_pri,
>         int *pol)
> @@ -75,6 +80,136 @@ struct eal_tls_key {
>         return 0;
>  }
>
> +static void *
> +thread_func_wrapper(void *arg)
> +{
> +       struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg;
> +
> +       free(arg);
> +
> +       return (void *)(uintptr_t)ctx.thread_func(ctx.routine_args);
> +}
> +
> +int
> +rte_thread_create(rte_thread_t *thread_id,
> +               const rte_thread_attr_t *thread_attr,
> +               rte_thread_func thread_func, void *args)
> +{
> +       int ret = 0;
> +       pthread_attr_t attr;
> +       pthread_attr_t *attrp = NULL;
> +       struct thread_routine_ctx *ctx;
> +       struct sched_param param = {
> +               .sched_priority = 0,
> +       };
> +       int policy = SCHED_OTHER;
> +
> +       ctx = calloc(1, sizeof(*ctx));
> +       if (ctx == NULL) {
> +               RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
> +               ret = ENOMEM;
> +               goto cleanup;
> +       }
> +       ctx->routine_args = args;
> +       ctx->thread_func = thread_func;
> +
> +       if (thread_attr != NULL) {
> +               ret = pthread_attr_init(&attr);
> +               if (ret != 0) {
> +                       RTE_LOG(DEBUG, EAL, "pthread_attr_init failed\n");
> +                       goto cleanup;
> +               }
> +
> +               attrp = &attr;
> +
> +               /*
> +                * Set the inherit scheduler parameter to explicit,
> +                * otherwise the priority attribute is ignored.
> +                */
> +               ret = pthread_attr_setinheritsched(attrp,
> +                               PTHREAD_EXPLICIT_SCHED);
> +               if (ret != 0) {
> +                       RTE_LOG(DEBUG, EAL, "pthread_attr_setinheritsched failed\n");
> +                       goto cleanup;
> +               }
> +
> +
> +               if (thread_attr->priority ==
> +                               RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
> +                       ret = ENOTSUP;
> +                       goto cleanup;
> +               }
> +               ret = thread_map_priority_to_os_value(thread_attr->priority,
> +                               &param.sched_priority, &policy);
> +               if (ret != 0)
> +                       goto cleanup;
> +
> +               ret = pthread_attr_setschedpolicy(attrp, policy);
> +               if (ret != 0) {
> +                       RTE_LOG(DEBUG, EAL, "pthread_attr_setschedpolicy failed\n");
> +                       goto cleanup;
> +               }
> +
> +               ret = pthread_attr_setschedparam(attrp, &param);
> +               if (ret != 0) {
> +                       RTE_LOG(DEBUG, EAL, "pthread_attr_setschedparam failed\n");
> +                       goto cleanup;
> +               }
> +       }
> +
> +       ret = pthread_create((pthread_t *)&thread_id->opaque_id, attrp,
> +               thread_func_wrapper, ctx);
> +       if (ret != 0) {
> +               RTE_LOG(DEBUG, EAL, "pthread_create failed\n");
> +               goto cleanup;
> +       }
> +
> +       if (thread_attr != NULL && CPU_COUNT(&thread_attr->cpuset) > 0) {
> +               ret = rte_thread_set_affinity_by_id(*thread_id,
> +                       &thread_attr->cpuset);
> +               if (ret != 0) {
> +                       RTE_LOG(DEBUG, EAL, "rte_thread_set_affinity_by_id failed\n");
> +                       goto cleanup;
> +               }
> +       }
> +
> +       ctx = NULL;
> +cleanup:
> +       free(ctx);
> +       if (attrp != NULL)
> +               pthread_attr_destroy(&attr);
> +
> +       return ret;
> +}

I am looking back at potential races in our code while reviewing the
ctrl thread creation fix, and I stopped at this patch.

What happens if rte_thread_set_affinity_by_id() fails?
A new thread got started, but I don't see it being terminated in the
cleanup phase.


In this same situation, we may have a small race for a double free on
ctx since thread_func_wrapper() may have already been called from the
new thread.


I have the same concerns with the windows implementation.


-- 
David Marchand

Re: [PATCH v5 2/6] eal: add thread lifetime management

[Tyler Retzlaff]

On Wed, Mar 01, 2023 at 09:11:43AM +0100, David Marchand wrote:
> Hello Tyler,
> 
> On Wed, Oct 5, 2022 at 7:07 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> > diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
> > index 9126595..d4c1a7f 100644
> > --- a/lib/eal/unix/rte_thread.c
> > +++ b/lib/eal/unix/rte_thread.c
> > @@ -16,6 +16,11 @@ struct eal_tls_key {
> >         pthread_key_t thread_index;
> >  };
> >
> > +struct thread_routine_ctx {
> > +       rte_thread_func thread_func;
> > +       void *routine_args;
> > +};
> > +
> >  static int
> >  thread_map_priority_to_os_value(enum rte_thread_priority eal_pri, int *os_pri,
> >         int *pol)
> > @@ -75,6 +80,136 @@ struct eal_tls_key {
> >         return 0;
> >  }
> >
> > +static void *
> > +thread_func_wrapper(void *arg)
> > +{
> > +       struct thread_routine_ctx ctx = *(struct thread_routine_ctx *)arg;
> > +
> > +       free(arg);
> > +
> > +       return (void *)(uintptr_t)ctx.thread_func(ctx.routine_args);
> > +}
> > +
> > +int
> > +rte_thread_create(rte_thread_t *thread_id,
> > +               const rte_thread_attr_t *thread_attr,
> > +               rte_thread_func thread_func, void *args)
> > +{
> > +       int ret = 0;
> > +       pthread_attr_t attr;
> > +       pthread_attr_t *attrp = NULL;
> > +       struct thread_routine_ctx *ctx;
> > +       struct sched_param param = {
> > +               .sched_priority = 0,
> > +       };
> > +       int policy = SCHED_OTHER;
> > +
> > +       ctx = calloc(1, sizeof(*ctx));
> > +       if (ctx == NULL) {
> > +               RTE_LOG(DEBUG, EAL, "Insufficient memory for thread context allocations\n");
> > +               ret = ENOMEM;
> > +               goto cleanup;
> > +       }
> > +       ctx->routine_args = args;
> > +       ctx->thread_func = thread_func;
> > +
> > +       if (thread_attr != NULL) {
> > +               ret = pthread_attr_init(&attr);
> > +               if (ret != 0) {
> > +                       RTE_LOG(DEBUG, EAL, "pthread_attr_init failed\n");
> > +                       goto cleanup;
> > +               }
> > +
> > +               attrp = &attr;
> > +
> > +               /*
> > +                * Set the inherit scheduler parameter to explicit,
> > +                * otherwise the priority attribute is ignored.
> > +                */
> > +               ret = pthread_attr_setinheritsched(attrp,
> > +                               PTHREAD_EXPLICIT_SCHED);
> > +               if (ret != 0) {
> > +                       RTE_LOG(DEBUG, EAL, "pthread_attr_setinheritsched failed\n");
> > +                       goto cleanup;
> > +               }
> > +
> > +
> > +               if (thread_attr->priority ==
> > +                               RTE_THREAD_PRIORITY_REALTIME_CRITICAL) {
> > +                       ret = ENOTSUP;
> > +                       goto cleanup;
> > +               }
> > +               ret = thread_map_priority_to_os_value(thread_attr->priority,
> > +                               &param.sched_priority, &policy);
> > +               if (ret != 0)
> > +                       goto cleanup;
> > +
> > +               ret = pthread_attr_setschedpolicy(attrp, policy);
> > +               if (ret != 0) {
> > +                       RTE_LOG(DEBUG, EAL, "pthread_attr_setschedpolicy failed\n");
> > +                       goto cleanup;
> > +               }
> > +
> > +               ret = pthread_attr_setschedparam(attrp, &param);
> > +               if (ret != 0) {
> > +                       RTE_LOG(DEBUG, EAL, "pthread_attr_setschedparam failed\n");
> > +                       goto cleanup;
> > +               }
> > +       }
> > +
> > +       ret = pthread_create((pthread_t *)&thread_id->opaque_id, attrp,
> > +               thread_func_wrapper, ctx);
> > +       if (ret != 0) {
> > +               RTE_LOG(DEBUG, EAL, "pthread_create failed\n");
> > +               goto cleanup;
> > +       }
> > +
> > +       if (thread_attr != NULL && CPU_COUNT(&thread_attr->cpuset) > 0) {
> > +               ret = rte_thread_set_affinity_by_id(*thread_id,
> > +                       &thread_attr->cpuset);
> > +               if (ret != 0) {
> > +                       RTE_LOG(DEBUG, EAL, "rte_thread_set_affinity_by_id failed\n");
> > +                       goto cleanup;
> > +               }
> > +       }
> > +
> > +       ctx = NULL;
> > +cleanup:
> > +       free(ctx);
> > +       if (attrp != NULL)
> > +               pthread_attr_destroy(&attr);
> > +
> > +       return ret;
> > +}
> 
> I am looking back at potential races in our code while reviewing the
> ctrl thread creation fix, and I stopped at this patch.

Thank you for doing this, this series I actually did not start I only
took over so the review is welcome.

> 
> What happens if rte_thread_set_affinity_by_id() fails?
> A new thread got started, but I don't see it being terminated in the
> cleanup phase.
> 
> 
> In this same situation, we may have a small race for a double free on
> ctx since thread_func_wrapper() may have already been called from the
> new thread.

I will review this and get back with fixes if necessary.

> 
> 
> I have the same concerns with the windows implementation.

I will check it too.

> 
> 
> -- 
> David Marchand

[PATCH v5 3/6] eal: add basic rte thread ID equal API

[Tyler Retzlaff]

Add rte_thread_equal() that tests if two rte_thread_id are equal.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/eal/unix/rte_thread.c    | 6 ++++++
 lib/eal/version.map          | 1 +
 lib/eal/windows/rte_thread.c | 6 ++++++
 3 files changed, 13 insertions(+)

diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/unix/rte_thread.c
index d4c1a7f..37ebfcf 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/unix/rte_thread.c
@@ -210,6 +210,12 @@ struct thread_routine_ctx {
 	return pthread_detach((pthread_t)thread_id.opaque_id);
 }
 
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return pthread_equal((pthread_t)t1.opaque_id, (pthread_t)t2.opaque_id);
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 5fd84b6..2d64a25 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -438,6 +438,7 @@ EXPERIMENTAL {
 	rte_thread_attr_set_priority;
 	rte_thread_create;
 	rte_thread_detach;
+	rte_thread_equal;
 	rte_thread_join;
 };
 
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index e153278..1c1e9d0 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -289,6 +289,12 @@ struct thread_routine_ctx {
 	return 0;
 }
 
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return t1.opaque_id == t2.opaque_id;
+}
+
 rte_thread_t
 rte_thread_self(void)
 {
-- 
1.8.3.1

[PATCH v5 4/6] test/threads: add tests for thread lifetime API

[Tyler Retzlaff]

Test basic functionality and demonstrate use of following thread
lifetime api.

    * rte_thread_create
    * rte_thread_detach
    * rte_thread_join

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index b9d8b4e..1077373 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -14,7 +14,7 @@
 
 static uint32_t thread_id_ready;
 
-static void *
+static uint32_t
 thread_main(void *arg)
 {
 	*(rte_thread_t *)arg = rte_thread_self();
@@ -23,7 +23,55 @@
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 1)
 		;
 
-	return NULL;
+	return 0;
+}
+
+static int
+test_thread_create_join(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, &thread_main_id) == 0,
+		"Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_join(thread_id, NULL) == 0,
+		"Failed to join thread.");
+
+	return 0;
+}
+
+static int
+test_thread_create_detach(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_t thread_main_id;
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main,
+		&thread_main_id) == 0, "Failed to create thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_equal(thread_id, thread_main_id) != 0,
+		"Unexpected thread id.");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	RTE_TEST_ASSERT(rte_thread_detach(thread_id) == 0,
+		"Failed to detach thread.");
+
+	return 0;
 }
 
 static int
@@ -123,6 +171,8 @@
 	.setup = NULL,
 	.teardown = NULL,
 	.unit_test_cases = {
+		TEST_CASE(test_thread_create_join),
+		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
 		TEST_CASES_END()
-- 
1.8.3.1

Re: [PATCH v5 4/6] test/threads: add tests for thread lifetime API

[David Marchand]

On Wed, Oct 5, 2022 at 7:07 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> Test basic functionality and demonstrate use of following thread
> lifetime api.
>
>     * rte_thread_create
>     * rte_thread_detach

And, to some extent, rte_thread_equal.

>     * rte_thread_join
>
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  app/test/test_threads.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/app/test/test_threads.c b/app/test/test_threads.c
> index b9d8b4e..1077373 100644
> --- a/app/test/test_threads.c
> +++ b/app/test/test_threads.c
> @@ -14,7 +14,7 @@
>
>  static uint32_t thread_id_ready;
>
> -static void *
> +static uint32_t
>  thread_main(void *arg)

Stopping at this patch, I still see a build failure.
This prototype change makes it uncompatible with remaining calls to
pthread_create.

This is fixed in patch 6 which I intend to squash here (and adding the
check on arg != NULL that is in patch 5).
Deal?


-- 
David Marchand

Re: [PATCH v5 4/6] test/threads: add tests for thread lifetime API

[Tyler Retzlaff]

On Thu, Oct 06, 2022 at 10:32:56AM +0200, David Marchand wrote:
> On Wed, Oct 5, 2022 at 7:07 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > Test basic functionality and demonstrate use of following thread
> > lifetime api.
> >
> >     * rte_thread_create
> >     * rte_thread_detach
> 
> And, to some extent, rte_thread_equal.
> 
> >     * rte_thread_join
> >
> > Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> >  app/test/test_threads.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 52 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test/test_threads.c b/app/test/test_threads.c
> > index b9d8b4e..1077373 100644
> > --- a/app/test/test_threads.c
> > +++ b/app/test/test_threads.c
> > @@ -14,7 +14,7 @@
> >
> >  static uint32_t thread_id_ready;
> >
> > -static void *
> > +static uint32_t
> >  thread_main(void *arg)
> 
> Stopping at this patch, I still see a build failure.
> This prototype change makes it uncompatible with remaining calls to
> pthread_create.

hmm, i must have made a mistake somewhere in the rebase. now i see what
you mean about build failure @ patch 4 i was considering it only with
the entire series applied i had not been testing individual patches in
the series. sorry about that.

> 
> This is fixed in patch 6 which I intend to squash here (and adding the
> check on arg != NULL that is in patch 5).
> Deal?

i have no problem with that.

now that i've read this can you confirm the only outstanding issue
is whether to move code to eal_common_thread? i posted a follow up to
thomas and i'm just waiting on his reply.

if the reply is still a positive to move the code i can fix the above in
a v6.

Thomas waiting on your follow up regarding code move.

thanks

[PATCH v5 5/6] test/threads: add tests for thread attributes API

[Tyler Retzlaff]

Test basic functionality and demonstrate use of following thread
attributes api. Additionally, test attributes are processed when
supplied to rte_thread_create().

    * rte_thread_attr_init
    * rte_thread_attr_set_affinity
    * rte_thread_attr_get_affinity
    * rte_thread_attr_set_priority

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 1077373..3c22cec 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -17,7 +17,9 @@
 static uint32_t
 thread_main(void *arg)
 {
-	*(rte_thread_t *)arg = rte_thread_self();
+	if (arg != NULL)
+		*(rte_thread_t *)arg = rte_thread_self();
+
 	__atomic_store_n(&thread_id_ready, 1, __ATOMIC_RELEASE);
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 1)
@@ -166,6 +168,73 @@
 	return 0;
 }
 
+static int
+test_thread_attributes_affinity(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	rte_cpuset_t cpuset0;
+	rte_cpuset_t cpuset1;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+
+	CPU_ZERO(&cpuset0);
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(rte_thread_self(), &cpuset0) == 0,
+		"Failed to get thread affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_set_affinity(&attr, &cpuset0) == 0,
+		"Failed to set thread attributes affinity");
+	RTE_TEST_ASSERT(rte_thread_attr_get_affinity(&attr, &cpuset1) == 0,
+		"Failed to get thread attributes affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Affinity should be stable");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes affinity thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_affinity_by_id(thread_id, &cpuset1) == 0,
+		"Failed to get attributes thread affinity");
+	RTE_TEST_ASSERT(memcmp(&cpuset0, &cpuset1, sizeof(rte_cpuset_t)) == 0,
+		"Failed to apply affinity attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
+static int
+test_thread_attributes_priority(void)
+{
+	rte_thread_t thread_id;
+	rte_thread_attr_t attr;
+	enum rte_thread_priority priority;
+
+	RTE_TEST_ASSERT(rte_thread_attr_init(&attr) == 0,
+		"Failed to initialize thread attributes");
+	RTE_TEST_ASSERT(rte_thread_attr_set_priority(&attr, RTE_THREAD_PRIORITY_NORMAL) == 0,
+		"Failed to set thread attributes priority");
+
+	thread_id_ready = 0;
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, &attr, thread_main, NULL) == 0,
+		"Failed to create attributes priority thread.");
+
+	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
+		;
+
+	RTE_TEST_ASSERT(rte_thread_get_priority(thread_id, &priority) == 0,
+		"Failed to get thread priority");
+	RTE_TEST_ASSERT(priority == RTE_THREAD_PRIORITY_NORMAL,
+		"Failed to apply priority attributes");
+
+	__atomic_store_n(&thread_id_ready, 2, __ATOMIC_RELEASE);
+
+	return 0;
+}
+
 static struct unit_test_suite threads_test_suite = {
 	.suite_name = "threads autotest",
 	.setup = NULL,
@@ -175,6 +244,8 @@
 		TEST_CASE(test_thread_create_detach),
 		TEST_CASE(test_thread_affinity),
 		TEST_CASE(test_thread_priority),
+		TEST_CASE(test_thread_attributes_affinity),
+		TEST_CASE(test_thread_attributes_priority),
 		TEST_CASES_END()
 	}
 };
-- 
1.8.3.1

[PATCH v5 6/6] test/threads: remove unit test use of pthread

[Tyler Retzlaff]

Now that rte_thread provides thread lifetime functions stop using
pthread in unit tests.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/test/test_threads.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/app/test/test_threads.c b/app/test/test_threads.c
index 3c22cec..e0f18e4 100644
--- a/app/test/test_threads.c
+++ b/app/test/test_threads.c
@@ -3,7 +3,6 @@
  */
 
 #include <string.h>
-#include <pthread.h>
 
 #include <rte_thread.h>
 #include <rte_debug.h>
@@ -79,12 +78,11 @@
 static int
 test_thread_priority(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	enum rte_thread_priority priority;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
@@ -131,13 +129,12 @@
 static int
 test_thread_affinity(void)
 {
-	pthread_t id;
 	rte_thread_t thread_id;
 	rte_cpuset_t cpuset0;
 	rte_cpuset_t cpuset1;
 
 	thread_id_ready = 0;
-	RTE_TEST_ASSERT(pthread_create(&id, NULL, thread_main, &thread_id) == 0,
+	RTE_TEST_ASSERT(rte_thread_create(&thread_id, NULL, thread_main, NULL) == 0,
 		"Failed to create thread");
 
 	while (__atomic_load_n(&thread_id_ready, __ATOMIC_ACQUIRE) == 0)
-- 
1.8.3.1

Re: [PATCH v5 0/6] add thread lifetime and attributes API

[David Marchand]

On Wed, Oct 5, 2022 at 7:07 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> add rte thread lifetime and attributes api. with these api additions
> there is now sufficient platform abstracted thread api to remove the
> use of pthread in the unit tests.
>
> v5:
>   * include errno.h in rte_thread.c since errno.h is no longer included
>     in rte_common.h
>   * move rte_thread_attr symbols from 22.07 to 22.11 section of
>     version.map.
>   * remove RTE_HAS_CPUSET guards from rte_thread.h

Here is a summary of what I did:
- I had to rollback part of RTE_HAS_CPUSET guards.
I had forgotten about the reason for those guards: the "culprit" is
Alpine Linux / musl where cpuset is not exposed unless an application
is built with _GNU_SOURCE.
See: https://git.dpdk.org/dpdk/commit/lib/librte_eal/include/rte_thread.h?id=e0473c6d5b18560dd11fd4d7ebc81dea6774f33e
- I squashed patch 6 in patch 4 to fix the compilation.
- I moved rte_thread.c content to eal_common_thread.c.
- I still added Dmitry acks from v4, as the functional parts/API did not change.


Series applied, thanks.


-- 
David Marchand

Re: [PATCH v5 0/6] add thread lifetime and attributes API

[Tyler Retzlaff]

thanks David,

appreciate it. it's nice to finally get this series cleared.

On Thu, Oct 06, 2022 at 09:25:12PM +0200, David Marchand wrote:
> On Wed, Oct 5, 2022 at 7:07 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > add rte thread lifetime and attributes api. with these api additions
> > there is now sufficient platform abstracted thread api to remove the
> > use of pthread in the unit tests.
> >
> > v5:
> >   * include errno.h in rte_thread.c since errno.h is no longer included
> >     in rte_common.h
> >   * move rte_thread_attr symbols from 22.07 to 22.11 section of
> >     version.map.
> >   * remove RTE_HAS_CPUSET guards from rte_thread.h
> 
> Here is a summary of what I did:
> - I had to rollback part of RTE_HAS_CPUSET guards.
> I had forgotten about the reason for those guards: the "culprit" is
> Alpine Linux / musl where cpuset is not exposed unless an application
> is built with _GNU_SOURCE.
> See: https://git.dpdk.org/dpdk/commit/lib/librte_eal/include/rte_thread.h?id=e0473c6d5b18560dd11fd4d7ebc81dea6774f33e
> - I squashed patch 6 in patch 4 to fix the compilation.
> - I moved rte_thread.c content to eal_common_thread.c.
> - I still added Dmitry acks from v4, as the functional parts/API did not change.
> 
> 
> Series applied, thanks.
> 
> 
> -- 
> David Marchand