[v6,01/10] eal: add thread id and simple thread functions
Checks
Commit Message
From: Narcisa Vasile <navasile@microsoft.com>
Add the thread identifier type.
Add functions for comparing thread ids and obtaining the thread id
for the current thread.
Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
---
lib/librte_eal/common/rte_thread.c | 101 ++++++++++++++++++
lib/librte_eal/include/rte_thread.h | 45 ++++++--
lib/librte_eal/include/rte_thread_types.h | 12 +++
.../include/rte_windows_thread_types.h | 12 +++
lib/librte_eal/windows/rte_thread.c | 13 +++
5 files changed, 174 insertions(+), 9 deletions(-)
create mode 100644 lib/librte_eal/common/rte_thread.c
create mode 100644 lib/librte_eal/include/rte_thread_types.h
create mode 100644 lib/librte_eal/windows/include/rte_windows_thread_types.h
Comments
2021-04-02 18:38 (UTC-0700), Narcisa Ana Maria Vasile:
> From: Narcisa Vasile <navasile@microsoft.com>
>
> Add the thread identifier type.
> Add functions for comparing thread ids and obtaining the thread id
> for the current thread.
>
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> ---
(For the whole series.)
Please summarize and distribute relevant parts of the cover letter to commit
messages. Remember that cover letter doesn't get to commit log. This series
has subtle details that a good commit message should describe.
> diff --git a/lib/librte_eal/include/rte_thread_types.h b/lib/librte_eal/include/rte_thread_types.h
> new file mode 100644
> index 000000000..19fb85e38
> --- /dev/null
> +++ b/lib/librte_eal/include/rte_thread_types.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2021 Microsoft Corporation
> + */
> +
> +#ifndef _RTE_THREAD_TYPES_H_
> +#define _RTE_THREAD_TYPES_H_
> +
> +#include <pthread.h>
> +
> +typedef pthread_t rte_thread_t;
> +
> +#endif /* _RTE_THREAD_TYPES_H_ */
> diff --git a/lib/librte_eal/windows/include/rte_windows_thread_types.h b/lib/librte_eal/windows/include/rte_windows_thread_types.h
> new file mode 100644
> index 000000000..ebd3d9e8f
> --- /dev/null
> +++ b/lib/librte_eal/windows/include/rte_windows_thread_types.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2021 Microsoft Corporation
> + */
> +
> +#ifndef _RTE_THREAD_TYPES_H_
> +#define _RTE_THREAD_TYPES_H_
> +
> +#include <rte_windows.h>
> +
> +typedef DWORD rte_thread_t;
> +
> +#endif /* _RTE_THREAD_TYPES_H_ */
pthread_t type in pthreads-win32 and winpthread is not 32 bit.
DPDK will have different ABI depending on a threading backend used.
Apps must know it at build time then. How do they discover it?
This is worth a warning in commit log and docs.
> diff --git a/lib/librte_eal/windows/rte_thread.c b/lib/librte_eal/windows/rte_thread.c
> index 667287c38..940d9c653 100644
> --- a/lib/librte_eal/windows/rte_thread.c
> +++ b/lib/librte_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,6 +12,18 @@ struct eal_tls_key {
> DWORD thread_index;
> };
>
> +rte_thread_t
> +rte_thread_self(void)
> +{
> + return GetCurrentThreadId();
> +}
> +
> +int
> +rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
> +{
> + return t1 == t2 ? 1 : 0;
> +}
> +
"a == b" returns (int)0 or (int)1 in C.
29/04/2021 02:50, Dmitry Kozlyuk:
> 2021-04-02 18:38 (UTC-0700), Narcisa Ana Maria Vasile:
> > --- /dev/null
> > +++ b/lib/librte_eal/windows/include/rte_windows_thread_types.h
> > @@ -0,0 +1,12 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2021 Microsoft Corporation
> > + */
> > +
> > +#ifndef _RTE_THREAD_TYPES_H_
> > +#define _RTE_THREAD_TYPES_H_
> > +
> > +#include <rte_windows.h>
> > +
> > +typedef DWORD rte_thread_t;
> > +
> > +#endif /* _RTE_THREAD_TYPES_H_ */
>
> pthread_t type in pthreads-win32 and winpthread is not 32 bit.
> DPDK will have different ABI depending on a threading backend used.
> Apps must know it at build time then. How do they discover it?
> This is worth a warning in commit log and docs.
Not sure this is an acceptable behaviour.
In my opinion, ABI should not vary.
+Cc Ray
On 29/04/2021 08:44, Thomas Monjalon wrote:
> 29/04/2021 02:50, Dmitry Kozlyuk:
>> 2021-04-02 18:38 (UTC-0700), Narcisa Ana Maria Vasile:
>>> --- /dev/null
>>> +++ b/lib/librte_eal/windows/include/rte_windows_thread_types.h
>>> @@ -0,0 +1,12 @@
>>> +/* SPDX-License-Identifier: BSD-3-Clause
>>> + * Copyright(c) 2021 Microsoft Corporation
>>> + */
>>> +
>>> +#ifndef _RTE_THREAD_TYPES_H_
>>> +#define _RTE_THREAD_TYPES_H_
>>> +
>>> +#include <rte_windows.h>
>>> +
>>> +typedef DWORD rte_thread_t;
>>> +
>>> +#endif /* _RTE_THREAD_TYPES_H_ */
>>
>> pthread_t type in pthreads-win32 and winpthread is not 32 bit.
>> DPDK will have different ABI depending on a threading backend used.
>> Apps must know it at build time then. How do they discover it?
>> This is worth a warning in commit log and docs.
>
> Not sure this is an acceptable behaviour.
> In my opinion, ABI should not vary.
> +Cc Ray
>
So pthread_t on Win32 should just map to the HANDLE datatype.
Which if memory serves is in fact a DWORD on Win32.
So I suspect that pthreads indirection is probably be just providing a circuitous route to end up in the same place, a HANDLE
IMHO
To absolutely guarantee no ABI change, we ought to be passing back void * not rte_thread_t.
#ifdef WIN32
/* The primitives for Windows types */
..
typedef HANDLE apr_os_thread_t; /*becomes pthread_t later*/
..
Ray K
On Thu, Apr 29, 2021 at 01:05:05PM +0100, Kinsella, Ray wrote:
>
>
> On 29/04/2021 08:44, Thomas Monjalon wrote:
> > 29/04/2021 02:50, Dmitry Kozlyuk:
> >> 2021-04-02 18:38 (UTC-0700), Narcisa Ana Maria Vasile:
> >>> --- /dev/null
> >>> +++ b/lib/librte_eal/windows/include/rte_windows_thread_types.h
> >>> @@ -0,0 +1,12 @@
> >>> +/* SPDX-License-Identifier: BSD-3-Clause
> >>> + * Copyright(c) 2021 Microsoft Corporation
> >>> + */
> >>> +
> >>> +#ifndef _RTE_THREAD_TYPES_H_
> >>> +#define _RTE_THREAD_TYPES_H_
> >>> +
> >>> +#include <rte_windows.h>
> >>> +
> >>> +typedef DWORD rte_thread_t;
> >>> +
> >>> +#endif /* _RTE_THREAD_TYPES_H_ */
> >>
> >> pthread_t type in pthreads-win32 and winpthread is not 32 bit.
> >> DPDK will have different ABI depending on a threading backend used.
> >> Apps must know it at build time then. How do they discover it?
> >> This is worth a warning in commit log and docs.
> >
> > Not sure this is an acceptable behaviour.
> > In my opinion, ABI should not vary.
> > +Cc Ray
> >
>
> So pthread_t on Win32 should just map to the HANDLE datatype.
> Which if memory serves is in fact a DWORD on Win32.
> So I suspect that pthreads indirection is probably be just providing a circuitous route to end up in the same place, a HANDLE
>
> IMHO
> To absolutely guarantee no ABI change, we ought to be passing back void * not rte_thread_t.
agreed, the type should be opaque.
but may i suggest uintptr_t instead since void * still leaks implementation detail.
2021-04-29 13:05 (UTC+0100), Kinsella, Ray:
> On 29/04/2021 08:44, Thomas Monjalon wrote:
> > 29/04/2021 02:50, Dmitry Kozlyuk:
> >> 2021-04-02 18:38 (UTC-0700), Narcisa Ana Maria Vasile:
> >>> --- /dev/null
> >>> +++ b/lib/librte_eal/windows/include/rte_windows_thread_types.h
> >>> @@ -0,0 +1,12 @@
> >>> +/* SPDX-License-Identifier: BSD-3-Clause
> >>> + * Copyright(c) 2021 Microsoft Corporation
> >>> + */
> >>> +
> >>> +#ifndef _RTE_THREAD_TYPES_H_
> >>> +#define _RTE_THREAD_TYPES_H_
> >>> +
> >>> +#include <rte_windows.h>
> >>> +
> >>> +typedef DWORD rte_thread_t;
> >>> +
> >>> +#endif /* _RTE_THREAD_TYPES_H_ */
> >>
> >> pthread_t type in pthreads-win32 and winpthread is not 32 bit.
> >> DPDK will have different ABI depending on a threading backend used.
> >> Apps must know it at build time then. How do they discover it?
> >> This is worth a warning in commit log and docs.
> >
> > Not sure this is an acceptable behaviour.
> > In my opinion, ABI should not vary.
> > +Cc Ray
> >
>
> So pthread_t on Win32 should just map to the HANDLE datatype.
> Which if memory serves is in fact a DWORD on Win32.
DWORD = uint32_t, HANDLE = void*, which are of different size on x64.
I suggest an opaque 64-bit value to fit pthread_t from MinGW's winpthread.
Only pthreads-win32 has a bigger pthread_t, but we don't have to support it.
> So I suspect that pthreads indirection is probably be just providing a circuitous route to end up in the same place, a HANDLE
>
> IMHO
> To absolutely guarantee no ABI change, we ought to be passing back void * not rte_thread_t.
Yes. Only I'd use a type-safe version:
typedef struct rte_thread_tag {
void *opaque; /* or uintptr_t per Tyler's suggestion */
} rte_thread_t;
On Thu, Apr 29, 2021 at 07:28:26PM +0300, Dmitry Kozlyuk wrote:
> 2021-04-29 13:05 (UTC+0100), Kinsella, Ray:
> > On 29/04/2021 08:44, Thomas Monjalon wrote:
> > > 29/04/2021 02:50, Dmitry Kozlyuk:
> > >> 2021-04-02 18:38 (UTC-0700), Narcisa Ana Maria Vasile:
> > >>> --- /dev/null
> > >>> +++ b/lib/librte_eal/windows/include/rte_windows_thread_types.h
> > >>> @@ -0,0 +1,12 @@
> > >>> +/* SPDX-License-Identifier: BSD-3-Clause
> > >>> + * Copyright(c) 2021 Microsoft Corporation
> > >>> + */
> > >>> +
> > >>> +#ifndef _RTE_THREAD_TYPES_H_
> > >>> +#define _RTE_THREAD_TYPES_H_
> > >>> +
> > >>> +#include <rte_windows.h>
> > >>> +
> > >>> +typedef DWORD rte_thread_t;
> > >>> +
> > >>> +#endif /* _RTE_THREAD_TYPES_H_ */
> > >>
> > >> pthread_t type in pthreads-win32 and winpthread is not 32 bit.
> > >> DPDK will have different ABI depending on a threading backend used.
> > >> Apps must know it at build time then. How do they discover it?
> > >> This is worth a warning in commit log and docs.
> > >
> > > Not sure this is an acceptable behaviour.
> > > In my opinion, ABI should not vary.
> > > +Cc Ray
> > >
> >
> > So pthread_t on Win32 should just map to the HANDLE datatype.
> > Which if memory serves is in fact a DWORD on Win32.
>
> DWORD = uint32_t, HANDLE = void*, which are of different size on x64.
> I suggest an opaque 64-bit value to fit pthread_t from MinGW's winpthread.
> Only pthreads-win32 has a bigger pthread_t, but we don't have to support it.
>
> > So I suspect that pthreads indirection is probably be just providing a circuitous route to end up in the same place, a HANDLE
> >
> > IMHO
> > To absolutely guarantee no ABI change, we ought to be passing back void * not rte_thread_t.
>
> Yes. Only I'd use a type-safe version:
>
> typedef struct rte_thread_tag {
> void *opaque; /* or uintptr_t per Tyler's suggestion */
> } rte_thread_t;
I agree we need a big enough value to fit different identifiers.
However, just to clarify, on Windows, there are two distinct thread-related identifiers:
thread id (DWORD) and thread HANDLE (void*).
In this API implementation I've used the rte_thread_t as the DWORD thread identifier, as the HANDLE
can be obtain from this DWORD using the OpenThread() function.
I will implement an opaque value that will be assigned the thread id (not the HANDLE) in this API.
new file mode 100644
@@ -0,0 +1,101 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2021 Mellanox Technologies, Ltd
+ * Copyright(c) 2021 Microsoft Corporation
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_errno.h>
+#include <rte_log.h>
+#include <rte_thread.h>
+
+struct eal_tls_key {
+ pthread_key_t thread_index;
+};
+
+rte_thread_t
+rte_thread_self(void)
+{
+ return pthread_self();
+}
+
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+ return pthread_equal(t1, t2);
+}
+
+int
+rte_thread_key_create(rte_thread_key *key, void (*destructor)(void *))
+{
+ int err;
+ rte_thread_key k;
+
+ k = malloc(sizeof(*k));
+ if (k == NULL) {
+ RTE_LOG(DEBUG, EAL, "Cannot allocate TLS key.\n");
+ return EINVAL;
+ }
+ err = pthread_key_create(&(k->thread_index), destructor);
+ if (err != 0) {
+ RTE_LOG(DEBUG, EAL, "pthread_key_create failed: %s\n",
+ strerror(err));
+ free(k);
+ return err;
+ }
+ *key = k;
+ return 0;
+}
+
+int
+rte_thread_key_delete(rte_thread_key key)
+{
+ int err;
+
+ if (key == NULL) {
+ RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+ return EINVAL;
+ }
+ err = pthread_key_delete(key->thread_index);
+ if (err != 0) {
+ RTE_LOG(DEBUG, EAL, "pthread_key_delete failed: %s\n",
+ strerror(err));
+ free(key);
+ return err;
+ }
+ free(key);
+ return 0;
+}
+
+int
+rte_thread_value_set(rte_thread_key key, const void *value)
+{
+ int err;
+
+ if (key == NULL) {
+ RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+ return EINVAL;
+ }
+ err = pthread_setspecific(key->thread_index, value);
+ if (err != 0) {
+ RTE_LOG(DEBUG, EAL, "pthread_setspecific failed: %s\n",
+ strerror(err));
+ return err;
+ }
+ return 0;
+}
+
+void *
+rte_thread_value_get(rte_thread_key key)
+{
+ if (key == NULL) {
+ RTE_LOG(DEBUG, EAL, "Invalid TLS key.\n");
+ rte_errno = EINVAL;
+ return NULL;
+ }
+ return pthread_getspecific(key->thread_index);
+}
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: BSD-3-Clause
* Copyright(c) 2021 Mellanox Technologies, Ltd
+ * Copyright(c) 2021 Microsoft Corporation
*/
#include <rte_os.h>
@@ -20,6 +21,13 @@
extern "C" {
#endif
+#include <sched.h>
+#if defined(RTE_USE_WINDOWS_THREAD_TYPES)
+#include <rte_windows_thread_types.h>
+#else
+#include <rte_thread_types.h>
+#endif
+
/**
* TLS key type, an opaque pointer.
*/
@@ -27,6 +35,31 @@ typedef struct eal_tls_key *rte_thread_key;
#ifdef RTE_HAS_CPUSET
+/**
+ * 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);
+
/**
* Set core affinity of the current thread.
* Support both EAL and non-EAL thread and update TLS.
@@ -63,9 +96,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 +111,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 +126,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);
new file mode 100644
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Microsoft Corporation
+ */
+
+#ifndef _RTE_THREAD_TYPES_H_
+#define _RTE_THREAD_TYPES_H_
+
+#include <pthread.h>
+
+typedef pthread_t rte_thread_t;
+
+#endif /* _RTE_THREAD_TYPES_H_ */
new file mode 100644
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Microsoft Corporation
+ */
+
+#ifndef _RTE_THREAD_TYPES_H_
+#define _RTE_THREAD_TYPES_H_
+
+#include <rte_windows.h>
+
+typedef DWORD rte_thread_t;
+
+#endif /* _RTE_THREAD_TYPES_H_ */
@@ -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,6 +12,18 @@ struct eal_tls_key {
DWORD thread_index;
};
+rte_thread_t
+rte_thread_self(void)
+{
+ return GetCurrentThreadId();
+}
+
+int
+rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
+{
+ return t1 == t2 ? 1 : 0;
+}
+
int
rte_thread_key_create(rte_thread_key *key,
__rte_unused void (*destructor)(void *))