Re: [PATCH v18 1/8] eal: add basic threading functions

Re: [PATCH v18 1/8] eal: add basic threading functions

[Ananyev, Konstantin]

> Use a portable, type-safe representation for the thread identifier.
> Add functions for comparing thread ids and obtaining the thread id
> for the current thread.
> 
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> ---
>  lib/eal/common/meson.build            |  1 +
>  lib/eal/{unix => common}/rte_thread.c | 57 ++++++++++++++++----------
>  lib/eal/include/rte_thread.h          | 53 ++++++++++++++++++------
>  lib/eal/unix/meson.build              |  1 -
>  lib/eal/version.map                   |  3 ++
>  lib/eal/windows/rte_thread.c          | 58 +++++++++++++++++----------
>  6 files changed, 117 insertions(+), 56 deletions(-)
>  rename lib/eal/{unix => common}/rte_thread.c (66%)
> 
> diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
> index 917758cc65..6bdc9cd854 100644
> --- a/lib/eal/common/meson.build
> +++ b/lib/eal/common/meson.build
> @@ -52,5 +52,6 @@ if not is_windows
>              'hotplug_mp.c',
>              'malloc_mp.c',
>              'rte_keepalive.c',
> +            'rte_thread.c'
>      )
>  endif
> diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/common/rte_thread.c
> similarity index 66%
> rename from lib/eal/unix/rte_thread.c
> rename to lib/eal/common/rte_thread.c
> index c72d619ec1..92a7451b0a 100644
> --- a/lib/eal/unix/rte_thread.c
> +++ b/lib/eal/common/rte_thread.c
> @@ -1,5 +1,6 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
>   * Copyright 2021 Mellanox Technologies, Ltd
> + * Copyright(c) 2021 Microsoft Corporation
>   */
> 
>  #include <errno.h>
> @@ -16,25 +17,41 @@ struct eal_tls_key {
>  	pthread_key_t thread_index;
>  };
> 
> +rte_thread_t
> +rte_thread_self(void)
> +{
> +	rte_thread_t thread_id;
> +
> +	thread_id.opaque_id = (uintptr_t)pthread_self();
> +
> +	return thread_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);
> +}
> +
>  int
>  rte_thread_key_create(rte_thread_key *key, void (*destructor)(void *))
>  {
>  	int err;
> +	rte_thread_key k;

I know that 'rte_thread_key' wasn't introduced in this patchset,
but wonder - why not 'rte_thread_key_t' as we do for other DPDK typedefs?
 
> -	*key = malloc(sizeof(**key));
> -	if ((*key) == NULL) {
> +	k = malloc(sizeof(*k));
> +	if (k == NULL) {
>  		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.\n");
> -		rte_errno = ENOMEM;
> -		return -1;
> +		return EINVAL;
>  	}
> -	err = pthread_key_create(&((*key)->thread_index), destructor);
> -	if (err) {
> +	err = pthread_key_create(&(k->thread_index), destructor);
> +	if (err != 0) {
>  		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
>  			 strerror(err));
> -		free(*key);
> -		rte_errno = ENOEXEC;
> -		return -1;
> +		free(k);
> +		return err;
>  	}
> +	*key = k;
>  	return 0;
>  }
> 
> @@ -43,18 +60,16 @@ rte_thread_key_delete(rte_thread_key key)
>  {
>  	int err;
> 
> -	if (!key) {
> +	if (key == NULL) {
>  		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
> -		rte_errno = EINVAL;
> -		return -1;
> +		return EINVAL;
>  	}
>  	err = pthread_key_delete(key->thread_index);
> -	if (err) {
> +	if (err != 0) {
>  		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
>  			 strerror(err));
>  		free(key);
> -		rte_errno = ENOEXEC;
> -		return -1;
> +		return err;
>  	}
>  	free(key);
>  	return 0;
> @@ -65,17 +80,15 @@ rte_thread_value_set(rte_thread_key key, const void *value)
>  {
>  	int err;
> 
> -	if (!key) {
> +	if (key == NULL) {
>  		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
> -		rte_errno = EINVAL;
> -		return -1;
> +		return EINVAL;
>  	}
>  	err = pthread_setspecific(key->thread_index, value);
> -	if (err) {
> +	if (err != 0) {
>  		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
>  			strerror(err));
> -		rte_errno = ENOEXEC;
> -		return -1;
> +		return err;
>  	}
>  	return 0;
>  }
> @@ -83,7 +96,7 @@ rte_thread_value_set(rte_thread_key key, const void *value)
>  void *
>  rte_thread_value_get(rte_thread_key key)
>  {
> -	if (!key) {
> +	if (key == NULL) {
>  		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
>  		rte_errno = EINVAL;
>  		return NULL;
> diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> index 8be8ed8f36..c9cdeb07aa 100644
> --- a/lib/eal/include/rte_thread.h
> +++ b/lib/eal/include/rte_thread.h
> @@ -1,6 +1,8 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
>   * Copyright(c) 2021 Mellanox Technologies, Ltd
> + * Copyright(c) 2021 Microsoft Corporation
>   */
> +#include <stdint.h>

Do we really need it in common .h?
I don't see anything sched.h that we start to use with that patch.

>  #include <rte_os.h>
>  #include <rte_compat.h>
> @@ -20,11 +22,45 @@
>  extern "C" {
>  #endif
> 
> +#include <sched.h>
> +
> +/**
> + * Thread id descriptor.
> + */
> +typedef struct rte_thread_tag {
> +	uintptr_t opaque_id; /**< thread identifier */
> +} rte_thread_t;
> +
>  /**
>   * TLS key type, an opaque pointer.
>   */
>  typedef struct eal_tls_key *rte_thread_key;
> 
> +/**
> + * Get the id of the calling thread.
> + *
> + * @return
> + *   Return the thread id of the calling thread.
> + */
> +__rte_experimental
> +rte_thread_t rte_thread_self(void);
> +
> +/**
> + * 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
> 
>  /**
> @@ -63,9 +99,7 @@ void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
>   *
>   * @return
>   *   On success, zero.
> - *   On failure, a negative number and an error number is set in rte_errno.
> - *   rte_errno can be: ENOMEM  - Memory allocation error.
> - *                     ENOEXEC - Specific OS error.
> + *   On failure, return a positive errno-style error number.
>   */
> 
>  __rte_experimental
> @@ -80,9 +114,7 @@ int rte_thread_key_create(rte_thread_key *key,
>   *
>   * @return
>   *   On success, zero.
> - *   On failure, a negative number and an error number is set in rte_errno.
> - *   rte_errno can be: EINVAL  - Invalid parameter passed.
> - *                     ENOEXEC - Specific OS error.
> + *   On failure, return a positive errno-style error number.
>   */
>  __rte_experimental
>  int rte_thread_key_delete(rte_thread_key key);
> @@ -97,9 +129,7 @@ int rte_thread_key_delete(rte_thread_key key);
>   *
>   * @return
>   *   On success, zero.
> - *   On failure, a negative number and an error number is set in rte_errno.
> - *   rte_errno can be: EINVAL  - Invalid parameter passed.
> - *                     ENOEXEC - Specific OS error.
> + *   On failure, return a positive errno-style error number.
>   */
>  __rte_experimental
>  int rte_thread_value_set(rte_thread_key key, const void *value);
> @@ -112,9 +142,8 @@ int rte_thread_value_set(rte_thread_key key, const void *value);
>   *
>   * @return
>   *   On success, value data pointer (can also be NULL).
> - *   On failure, NULL and an error number is set in rte_errno.
> - *   rte_errno can be: EINVAL  - Invalid parameter passed.
> - *                     ENOEXEC - Specific OS error.
> + *   On failure, NULL and a positive error number is set in rte_errno.
> + *
>   */
>  __rte_experimental
>  void *rte_thread_value_get(rte_thread_key key);
> diff --git a/lib/eal/unix/meson.build b/lib/eal/unix/meson.build
> index e3ecd3e956..cb6d233721 100644
> --- a/lib/eal/unix/meson.build
> +++ b/lib/eal/unix/meson.build
> @@ -6,5 +6,4 @@ sources += files(
>          'eal_unix_memory.c',
>          'eal_unix_timer.c',
>          'eal_firmware.c',
> -        'rte_thread.c',
>  )

[dpdk-dev] [PATCH v17 00/13] eal: Add EAL API for threading

[Narcisa Ana Maria Vasile]

From: Narcisa Vasile <navasile@microsoft.com>

EAL thread API

**Problem Statement**
DPDK currently uses the pthread interface to create and manage threads.
Windows does not support the POSIX thread programming model,
so it currently
relies on a header file that hides the Windows calls under
pthread matched interfaces. Given that EAL should isolate the environment
specifics from the applications and libraries and mediate
all the communication with the operating systems, a new EAL interface
is needed for thread management.

**Goals**
* Introduce a generic EAL API for threading support that will remove
  the current Windows pthread.h shim.
* Replace references to pthread_* across the DPDK codebase with the new
  RTE_THREAD_* API.
* Allow users to choose between using the RTE_THREAD_* API or a
  3rd party thread library through a configuration option.

**Design plan**
New API main files:
* rte_thread.h (librte_eal/include)
* rte_thread.c (librte_eal/windows)
* rte_thread.c (librte_eal/common)

**A schematic example of the design**
--------------------------------------------------
lib/librte_eal/include/rte_thread.h
int rte_thread_create();

lib/librte_eal/common/rte_thread.c
int rte_thread_create() 
{
	return pthread_create();
}

lib/librte_eal/windows/rte_thread.c
int rte_thread_create() 
{
	return CreateThread();
}
-----------------------------------------------------

**Thread attributes**

When or after a thread is created, specific characteristics of the thread
can be adjusted. Currently in DPDK most threads operate at the OS-default
priority level but there are cases when increasing the priority is useful.
For example, high-performance applications require elevated priority to
avoid being preempted by other threads on the system.
The following structure that represents thread attributes has been
defined:

typedef struct
{
	enum rte_thread_priority priority;
	rte_cpuset_t cpuset;
} rte_thread_attr_t;

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.
An rte_thread_attr_t object can also be set to the default values
by calling *rte_thread_attr_init()*.

*Priority* is represented through an enum that currently advertises
two values for priority:
	- RTE_THREAD_PRIORITY_NORMAL
	- RTE_THREAD_PRIORITY_REALTIME_CRITICAL
The enum can be extended to allow for multiple priority levels.
rte_thread_set_priority      - sets the priority of a thread
rte_thread_get_priority      - retrieves the priority of a thread
                               from the OS
rte_thread_attr_set_priority - updates an rte_thread_attr_t object
                               with a new value for priority

*Affinity* is described by the already known “rte_cpuset_t” type.
rte_thread_attr_set/get_affinity - sets/gets the affinity field in a
                                   rte_thread_attr_t object
rte_thread_set/get_affinity      – sets/gets the affinity of a thread

**Errors**
As different platforms have different error codes, the approach here
is to translate the Windows error to POSIX-style ones to have
uniformity over the values returned. 

**Future work**
The long term plan is for EAL to provide full threading support:
* Add support for conditional variables
* Additional functionality offered by pthread_*
  (such as pthread_setname_np, etc.)

v17:
 - Move unrelated changes to the correct patch.
 - Rename RTE_STATIC_MUTEX to avoid confusion, since
   the mutex is still dynamically initialized behind the scenes.
 - Break down the unit tests into smaller patches and reorder them.
 - Remove duplicated code in header.
 - Improve commit messages and cover letter.

v16:
- Fix warning on freebsd by adding cast
- Change affinity unit test to consider ases when the requested CPU
  are not available on the system.
- Fix priority unit test to avoid termination of thread before the
  priority is checked.

v15:
- Add try_lock mutex functionality. If the mutex is already owned by a
  different thread, the function returns immediately. Otherwise, 
  the mutex will be acquired.
- Add function for getting the priority of a thread.
  An auxiliary function that translates the OS priority to the
  EAL accepted ones is added.
- Fix unit tests logging, add descriptive asserts that mark test failures.
  Verify mutex locking, verify barrier return values. Add test for
  statically initialized mutexes.
- Fix Alpine build by removing the use of pthread_attr_set_affinity() and
  using pthread_set_affinity() after the thread is created.

v14:
- Remove patch "eal: add EAL argument for setting thread priority"
  This will be added later when enabling the new threading API.
- Remove priority enum value "_UNDEFINED". NORMAL is used
  as the default.
- Fix issue with thread return value.

v13:
 - Fix syntax error in unit tests

v12:
 - Fix freebsd warning about initializer in unit tests

v11:
 - Add unit tests for thread API
 - Rebase

v10:
 - Remove patch no. 10. It will be broken down in subpatches 
   and sent as a different patchset that depends on this one.
   This is done due to the ABI breaks that would be caused by patch 10.
 - Replace unix/rte_thread.c with common/rte_thread.c
 - Remove initializations that may prevent compiler from issuing useful
   warnings.
 - Remove rte_thread_types.h and rte_windows_thread_types.h
 - Remove unneeded priority macros (EAL_THREAD_PRIORITY*)
 - Remove functions that retrieves thread handle from process handle
 - Remove rte_thread_cancel() until same behavior is obtained on
   all platforms.
 - Fix rte_thread_detach() function description,
   return value and remove empty line.
 - Reimplement mutex functions. Add compatible representation for mutex
   identifier. Add macro to replace static mutex initialization instances.
 - Fix commit messages (lines too long, remove unicode symbols)

v9:
- Sign patches

v8:
- Rebase
- Add rte_thread_detach() API
- Set default priority, when user did not specify a value

v7:
Based on DmitryK's review:
- Change thread id representation
- Change mutex id representation
- Implement static mutex inititalizer for Windows
- Change barrier identifier representation
- Improve commit messages
- Add missing doxygen comments
- Split error translation function
- Improve name for affinity function
- Remove cpuset_size parameter
- Fix eal_create_cpu_map function
- Map EAL priority values to OS specific values
- Add thread wrapper for start routine
- Do not export rte_thread_cancel() on Windows
- Cleanup, fix comments, fix typos.

v6:
- improve error-translation function
- call the error translation function in rte_thread_value_get()

v5:
- update cover letter with more details on the priority argument

v4:
- fix function description
- rebase

v3:
- rebase

v2:
- revert changes that break ABI 
- break up changes into smaller patches
- fix coding style issues
- fix issues with errors
- fix parameter type in examples/kni.c

Narcisa Vasile (13):
  eal: add basic threading functions
  eal: add thread attributes
  eal/windows: translate Windows errors to errno-style errors
  eal: implement functions for thread affinity management
  eal: implement thread priority management functions
  eal: add thread lifetime management
  app/test: add unit tests for rte_thread_self
  app/test: add unit tests for thread attributes
  app/test: add unit tests for thread lifetime management
  eal: implement functions for thread barrier management
  app/test: add unit tests for barrier
  eal: implement functions for mutex management
  app/test: add unit tests for mutex

 app/test/meson.build            |   2 +
 app/test/test_threads.c         | 372 ++++++++++++++++++
 lib/eal/common/meson.build      |   1 +
 lib/eal/common/rte_thread.c     | 497 ++++++++++++++++++++++++
 lib/eal/include/rte_thread.h    | 412 +++++++++++++++++++-
 lib/eal/unix/meson.build        |   1 -
 lib/eal/unix/rte_thread.c       |  92 -----
 lib/eal/version.map             |  22 ++
 lib/eal/windows/eal_lcore.c     | 176 ++++++---
 lib/eal/windows/eal_windows.h   |  10 +
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 656 ++++++++++++++++++++++++++++++--
 12 files changed, 2070 insertions(+), 173 deletions(-)
 create mode 100644 app/test/test_threads.c
 create mode 100644 lib/eal/common/rte_thread.c
 delete mode 100644 lib/eal/unix/rte_thread.c

-- 
2.31.0.vfs.0.1

[PATCH v18 0/8] eal: Add EAL API for threading

[Narcisa Ana Maria Vasile]

From: Narcisa Vasile <navasile@microsoft.com>

EAL thread API

**Problem Statement**
DPDK currently uses the pthread interface to create and manage threads.
Windows does not support the POSIX thread programming model,
so it currently
relies on a header file that hides the Windows calls under
pthread matched interfaces. Given that EAL should isolate the environment
specifics from the applications and libraries and mediate
all the communication with the operating systems, a new EAL interface
is needed for thread management.

**Goals**
* Introduce a generic EAL API for threading support that will remove
  the current Windows pthread.h shim.
* Replace references to pthread_* across the DPDK codebase with the new
  RTE_THREAD_* API.
* Allow users to choose between using the RTE_THREAD_* API or a
  3rd party thread library through a configuration option.

**Design plan**
New API main files:
* rte_thread.h (librte_eal/include)
* rte_thread.c (librte_eal/windows)
* rte_thread.c (librte_eal/common)

**A schematic example of the design**
--------------------------------------------------
lib/librte_eal/include/rte_thread.h
int rte_thread_create();

lib/librte_eal/common/rte_thread.c
int rte_thread_create() 
{
	return pthread_create();
}

lib/librte_eal/windows/rte_thread.c
int rte_thread_create() 
{
	return CreateThread();
}
-----------------------------------------------------

**Thread attributes**

When or after a thread is created, specific characteristics of the thread
can be adjusted. Currently in DPDK most threads operate at the OS-default
priority level but there are cases when increasing the priority is useful.
For example, high-performance applications require elevated priority to
avoid being preempted by other threads on the system.
The following structure that represents thread attributes has been
defined:

typedef struct
{
	enum rte_thread_priority priority;
	rte_cpuset_t cpuset;
} rte_thread_attr_t;

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.
An rte_thread_attr_t object can also be set to the default values
by calling *rte_thread_attr_init()*.

*Priority* is represented through an enum that currently advertises
two values for priority:
	- RTE_THREAD_PRIORITY_NORMAL
	- RTE_THREAD_PRIORITY_REALTIME_CRITICAL
The enum can be extended to allow for multiple priority levels.
rte_thread_set_priority      - sets the priority of a thread
rte_thread_get_priority      - retrieves the priority of a thread
                               from the OS
rte_thread_attr_set_priority - updates an rte_thread_attr_t object
                               with a new value for priority

*Affinity* is described by the already known “rte_cpuset_t” type.
rte_thread_attr_set/get_affinity - sets/gets the affinity field in a
                                   rte_thread_attr_t object
rte_thread_set/get_affinity      – sets/gets the affinity of a thread

**Errors**
As different platforms have different error codes, the approach here
is to translate the Windows error to POSIX-style ones to have
uniformity over the values returned. 

**Future work**
The long term plan is for EAL to provide full threading support:
* Add support for conditional variables
* Additional functionality offered by pthread_*
  (such as pthread_setname_np, etc.)

v18:
 - Squash unit tests in corresponding patches.
 - Prevent priority to be set to realtime on non-Windows systems.
 - Use already existing affinity function in rte_thread_create()

v17:
 - Move unrelated changes to the correct patch.
 - Rename RTE_STATIC_MUTEX to avoid confusion, since
   the mutex is still dynamically initialized behind the scenes.
 - Break down the unit tests into smaller patches and reorder them.
 - Remove duplicated code in header
 - Improve commit messages and cover letter.

v16:
- Fix warning on freebsd by adding cast
- Change affinity unit test to consider ases when the requested CPU
  are not available on the system.
- Fix priority unit test to avoid termination of thread before the
  priority is checked.

v15:
- Add try_lock mutex functionality. If the mutex is already owned by a
  different thread, the function returns immediately. Otherwise, 
  the mutex will be acquired.
- Add function for getting the priority of a thread.
  An auxiliary function that translates the OS priority to the
  EAL accepted ones is added.
- Fix unit tests logging, add descriptive asserts that mark test failures.
  Verify mutex locking, verify barrier return values. Add test for
  statically initialized mutexes.
- Fix Alpine build by removing the use of pthread_attr_set_affinity() and
  using pthread_set_affinity() after the thread is created.

v14:
- Remove patch "eal: add EAL argument for setting thread priority"
  This will be added later when enabling the new threading API.
- Remove priority enum value "_UNDEFINED". NORMAL is used
  as the default.
- Fix issue with thread return value.

v13:
 - Fix syntax error in unit tests

v12:
 - Fix freebsd warning about initializer in unit tests

v11:
 - Add unit tests for thread API
 - Rebase

v10:
 - Remove patch no. 10. It will be broken down in subpatches 
   and sent as a different patchset that depends on this one.
   This is done due to the ABI breaks that would be caused by patch 10.
 - Replace unix/rte_thread.c with common/rte_thread.c
 - Remove initializations that may prevent compiler from issuing useful
   warnings.
 - Remove rte_thread_types.h and rte_windows_thread_types.h
 - Remove unneeded priority macros (EAL_THREAD_PRIORITY*)
 - Remove functions that retrieves thread handle from process handle
 - Remove rte_thread_cancel() until same behavior is obtained on
   all platforms.
 - Fix rte_thread_detach() function description,
   return value and remove empty line.
 - Reimplement mutex functions. Add compatible representation for mutex
   identifier. Add macro to replace static mutex initialization instances.
 - Fix commit messages (lines too long, remove unicode symbols)

v9:
- Sign patches

v8:
- Rebase
- Add rte_thread_detach() API
- Set default priority, when user did not specify a value

v7:
Based on DmitryK's review:
- Change thread id representation
- Change mutex id representation
- Implement static mutex inititalizer for Windows
- Change barrier identifier representation
- Improve commit messages
- Add missing doxygen comments
- Split error translation function
- Improve name for affinity function
- Remove cpuset_size parameter
- Fix eal_create_cpu_map function
- Map EAL priority values to OS specific values
- Add thread wrapper for start routine
- Do not export rte_thread_cancel() on Windows
- Cleanup, fix comments, fix typos.

v6:
- improve error-translation function
- call the error translation function in rte_thread_value_get()

v5:
- update cover letter with more details on the priority argument

v4:
- fix function description
- rebase

v3:
- rebase

v2:
- revert changes that break ABI 
- break up changes into smaller patches
- fix coding style issues
- fix issues with errors
- fix parameter type in examples/kni.c

Narcisa Vasile (8):
  eal: add basic threading functions
  eal: add thread attributes
  eal/windows: translate Windows errors to errno-style errors
  eal: implement functions for thread affinity management
  eal: implement thread priority management functions
  eal: add thread lifetime management
  eal: implement functions for thread barrier management
  eal: implement functions for mutex management

 app/test/meson.build            |   2 +
 app/test/test_threads.c         | 372 ++++++++++++++++++
 lib/eal/common/meson.build      |   1 +
 lib/eal/common/rte_thread.c     | 511 +++++++++++++++++++++++++
 lib/eal/include/rte_thread.h    | 412 +++++++++++++++++++-
 lib/eal/unix/meson.build        |   1 -
 lib/eal/unix/rte_thread.c       |  92 -----
 lib/eal/version.map             |  22 ++
 lib/eal/windows/eal_lcore.c     | 176 ++++++---
 lib/eal/windows/eal_windows.h   |  10 +
 lib/eal/windows/include/sched.h |   2 +-
 lib/eal/windows/rte_thread.c    | 656 ++++++++++++++++++++++++++++++--
 12 files changed, 2084 insertions(+), 173 deletions(-)
 create mode 100644 app/test/test_threads.c
 create mode 100644 lib/eal/common/rte_thread.c
 delete mode 100644 lib/eal/unix/rte_thread.c

-- 
2.31.0.vfs.0.1

[PATCH v18 1/8] eal: add basic threading functions

[Narcisa Ana Maria Vasile]

From: Narcisa Vasile <navasile@microsoft.com>

Use a portable, type-safe representation for the thread identifier.
Add functions for comparing thread ids and obtaining the thread id
for the current thread.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
---
 lib/eal/common/meson.build            |  1 +
 lib/eal/{unix => common}/rte_thread.c | 57 ++++++++++++++++----------
 lib/eal/include/rte_thread.h          | 53 ++++++++++++++++++------
 lib/eal/unix/meson.build              |  1 -
 lib/eal/version.map                   |  3 ++
 lib/eal/windows/rte_thread.c          | 58 +++++++++++++++++----------
 6 files changed, 117 insertions(+), 56 deletions(-)
 rename lib/eal/{unix => common}/rte_thread.c (66%)

diff --git a/lib/eal/common/meson.build b/lib/eal/common/meson.build
index 917758cc65..6bdc9cd854 100644
--- a/lib/eal/common/meson.build
+++ b/lib/eal/common/meson.build
@@ -52,5 +52,6 @@ if not is_windows
             'hotplug_mp.c',
             'malloc_mp.c',
             'rte_keepalive.c',
+            'rte_thread.c'
     )
 endif
diff --git a/lib/eal/unix/rte_thread.c b/lib/eal/common/rte_thread.c
similarity index 66%
rename from lib/eal/unix/rte_thread.c
rename to lib/eal/common/rte_thread.c
index c72d619ec1..92a7451b0a 100644
--- a/lib/eal/unix/rte_thread.c
+++ b/lib/eal/common/rte_thread.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright 2021 Mellanox Technologies, Ltd
+ * Copyright(c) 2021 Microsoft Corporation
  */
 
 #include <errno.h>
@@ -16,25 +17,41 @@ struct eal_tls_key {
 	pthread_key_t thread_index;
 };
 
+rte_thread_t
+rte_thread_self(void)
+{
+	rte_thread_t thread_id;
+
+	thread_id.opaque_id = (uintptr_t)pthread_self();
+
+	return thread_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);
+}
+
 int
 rte_thread_key_create(rte_thread_key *key, void (*destructor)(void *))
 {
 	int err;
+	rte_thread_key k;
 
-	*key = malloc(sizeof(**key));
-	if ((*key) == NULL) {
+	k = malloc(sizeof(*k));
+	if (k == NULL) {
 		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.\n");
-		rte_errno = ENOMEM;
-		return -1;
+		return EINVAL;
 	}
-	err = pthread_key_create(&((*key)->thread_index), destructor);
-	if (err) {
+	err = pthread_key_create(&(k->thread_index), destructor);
+	if (err != 0) {
 		RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
 			 strerror(err));
-		free(*key);
-		rte_errno = ENOEXEC;
-		return -1;
+		free(k);
+		return err;
 	}
+	*key = k;
 	return 0;
 }
 
@@ -43,18 +60,16 @@ rte_thread_key_delete(rte_thread_key key)
 {
 	int err;
 
-	if (!key) {
+	if (key == NULL) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
-		rte_errno = EINVAL;
-		return -1;
+		return EINVAL;
 	}
 	err = pthread_key_delete(key->thread_index);
-	if (err) {
+	if (err != 0) {
 		RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
 			 strerror(err));
 		free(key);
-		rte_errno = ENOEXEC;
-		return -1;
+		return err;
 	}
 	free(key);
 	return 0;
@@ -65,17 +80,15 @@ rte_thread_value_set(rte_thread_key key, const void *value)
 {
 	int err;
 
-	if (!key) {
+	if (key == NULL) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
-		rte_errno = EINVAL;
-		return -1;
+		return EINVAL;
 	}
 	err = pthread_setspecific(key->thread_index, value);
-	if (err) {
+	if (err != 0) {
 		RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
 			strerror(err));
-		rte_errno = ENOEXEC;
-		return -1;
+		return err;
 	}
 	return 0;
 }
@@ -83,7 +96,7 @@ rte_thread_value_set(rte_thread_key key, const void *value)
 void *
 rte_thread_value_get(rte_thread_key key)
 {
-	if (!key) {
+	if (key == NULL) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
 		rte_errno = EINVAL;
 		return NULL;
diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
index 8be8ed8f36..c9cdeb07aa 100644
--- a/lib/eal/include/rte_thread.h
+++ b/lib/eal/include/rte_thread.h
@@ -1,6 +1,8 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(c) 2021 Mellanox Technologies, Ltd
+ * Copyright(c) 2021 Microsoft Corporation
  */
+#include <stdint.h>
 
 #include <rte_os.h>
 #include <rte_compat.h>
@@ -20,11 +22,45 @@
 extern "C" {
 #endif
 
+#include <sched.h>
+
+/**
+ * Thread id descriptor.
+ */
+typedef struct rte_thread_tag {
+	uintptr_t opaque_id; /**< thread identifier */
+} rte_thread_t;
+
 /**
  * TLS key type, an opaque pointer.
  */
 typedef struct eal_tls_key *rte_thread_key;
 
+/**
+ * Get the id of the calling thread.
+ *
+ * @return
+ *   Return the thread id of the calling thread.
+ */
+__rte_experimental
+rte_thread_t rte_thread_self(void);
+
+/**
+ * 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
 
 /**
@@ -63,9 +99,7 @@ void rte_thread_get_affinity(rte_cpuset_t *cpusetp);
  *
  * @return
  *   On success, zero.
- *   On failure, a negative number and an error number is set in rte_errno.
- *   rte_errno can be: ENOMEM  - Memory allocation error.
- *                     ENOEXEC - Specific OS error.
+ *   On failure, return a positive errno-style error number.
  */
 
 __rte_experimental
@@ -80,9 +114,7 @@ int rte_thread_key_create(rte_thread_key *key,
  *
  * @return
  *   On success, zero.
- *   On failure, a negative number and an error number is set in rte_errno.
- *   rte_errno can be: EINVAL  - Invalid parameter passed.
- *                     ENOEXEC - Specific OS error.
+ *   On failure, return a positive errno-style error number.
  */
 __rte_experimental
 int rte_thread_key_delete(rte_thread_key key);
@@ -97,9 +129,7 @@ int rte_thread_key_delete(rte_thread_key key);
  *
  * @return
  *   On success, zero.
- *   On failure, a negative number and an error number is set in rte_errno.
- *   rte_errno can be: EINVAL  - Invalid parameter passed.
- *                     ENOEXEC - Specific OS error.
+ *   On failure, return a positive errno-style error number.
  */
 __rte_experimental
 int rte_thread_value_set(rte_thread_key key, const void *value);
@@ -112,9 +142,8 @@ int rte_thread_value_set(rte_thread_key key, const void *value);
  *
  * @return
  *   On success, value data pointer (can also be NULL).
- *   On failure, NULL and an error number is set in rte_errno.
- *   rte_errno can be: EINVAL  - Invalid parameter passed.
- *                     ENOEXEC - Specific OS error.
+ *   On failure, NULL and a positive error number is set in rte_errno.
+ *
  */
 __rte_experimental
 void *rte_thread_value_get(rte_thread_key key);
diff --git a/lib/eal/unix/meson.build b/lib/eal/unix/meson.build
index e3ecd3e956..cb6d233721 100644
--- a/lib/eal/unix/meson.build
+++ b/lib/eal/unix/meson.build
@@ -6,5 +6,4 @@ sources += files(
         'eal_unix_memory.c',
         'eal_unix_timer.c',
         'eal_firmware.c',
-        'rte_thread.c',
 )
diff --git a/lib/eal/version.map b/lib/eal/version.map
index ab28c22791..24e9747795 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -420,6 +420,9 @@ EXPERIMENTAL {
 	rte_intr_instance_free;
 	rte_intr_type_get;
 	rte_intr_type_set;
+
+	rte_thread_self;
+	rte_thread_equal;
 };
 
 INTERNAL {
diff --git a/lib/eal/windows/rte_thread.c b/lib/eal/windows/rte_thread.c
index 667287c387..26c8dd4ebe 100644
--- a/lib/eal/windows/rte_thread.c
+++ b/lib/eal/windows/rte_thread.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright 2021 Mellanox Technologies, Ltd
+ * Copyright(c) 2021 Microsoft Corporation
  */
 
 #include <rte_common.h>
@@ -11,22 +12,38 @@ struct eal_tls_key {
 	DWORD thread_index;
 };
 
+rte_thread_t
+rte_thread_self(void)
+{
+	rte_thread_t thread_id;
+
+	thread_id.opaque_id = GetCurrentThreadId();
+
+	return thread_id;
+}
+
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+	return t1.opaque_id == t2.opaque_id;
+}
+
 int
 rte_thread_key_create(rte_thread_key *key,
 		__rte_unused void (*destructor)(void *))
 {
+	int ret;
+
 	*key = malloc(sizeof(**key));
 	if ((*key) == NULL) {
 		RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.\n");
-		rte_errno = ENOMEM;
-		return -1;
+		return ENOMEM;
 	}
 	(*key)->thread_index = TlsAlloc();
 	if ((*key)->thread_index == TLS_OUT_OF_INDEXES) {
-		RTE_LOG_WIN32_ERR("TlsAlloc()");
+		ret = thread_log_last_error("TlsAlloc()");
 		free(*key);
-		rte_errno = ENOEXEC;
-		return -1;
+		return ret;
 	}
 	return 0;
 }
@@ -34,16 +51,16 @@ rte_thread_key_create(rte_thread_key *key,
 int
 rte_thread_key_delete(rte_thread_key key)
 {
-	if (!key) {
+	int ret;
+
+	if (key == NULL) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
-		rte_errno = EINVAL;
-		return -1;
+		return EINVAL;
 	}
 	if (!TlsFree(key->thread_index)) {
-		RTE_LOG_WIN32_ERR("TlsFree()");
+		ret = thread_log_last_error("TlsFree()");
 		free(key);
-		rte_errno = ENOEXEC;
-		return -1;
+		return ret;
 	}
 	free(key);
 	return 0;
@@ -54,17 +71,14 @@ rte_thread_value_set(rte_thread_key key, const void *value)
 {
 	char *p;
 
-	if (!key) {
+	if (key == NULL) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
-		rte_errno = EINVAL;
-		return -1;
+		return EINVAL;
 	}
 	/* discard const qualifier */
 	p = (char *) (uintptr_t) value;
 	if (!TlsSetValue(key->thread_index, p)) {
-		RTE_LOG_WIN32_ERR("TlsSetValue()");
-		rte_errno = ENOEXEC;
-		return -1;
+		return thread_log_last_error("TlsSetValue()");
 	}
 	return 0;
 }
@@ -73,16 +87,18 @@ void *
 rte_thread_value_get(rte_thread_key key)
 {
 	void *output;
+	DWORD ret = 0;
 
-	if (!key) {
+	if (key == NULL) {
 		RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
 		rte_errno = EINVAL;
 		return NULL;
 	}
 	output = TlsGetValue(key->thread_index);
-	if (GetLastError() != ERROR_SUCCESS) {
-		RTE_LOG_WIN32_ERR("TlsGetValue()");
-		rte_errno = ENOEXEC;
+	ret = GetLastError();
+	if (ret != 0) {
+		RTE_LOG(DEBUG, EAL, "GetLastError()=%lu: TlsGetValue()\n", ret);
+		rte_errno = thread_translate_win32_error(ret);
 		return NULL;
 	}
 	return output;
-- 
2.31.0.vfs.0.1

Re: [PATCH v18 1/8] eal: add basic threading functions

[Narcisa Ana Maria Vasile]

On Wed, Nov 10, 2021 at 05:33:38PM -0800, Narcisa Ana Maria Vasile wrote:
> From: Narcisa Vasile <navasile@microsoft.com>
> 
> Use a portable, type-safe representation for the thread identifier.
> Add functions for comparing thread ids and obtaining the thread id
> for the current thread.
> 
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> ---

Abandoning this patch series.
It is superseded by the thread patches sent by Tyler.