[dpdk-dev] [PATCH v6 01/10] eal: add thread id and simple thread functions

[Narcisa Ana Maria Vasile]

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.