diff mbox series

[v3,1/2] eal/windows: add pthread mutex lock

Message ID 1602080249-36533-2-git-send-email-suanmingm@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series ethdev: make rte_flow API thread safe |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Suanming Mou Oct. 7, 2020, 2:17 p.m. UTC 
  Add pthread mutex lock as it is needed for the thread safe rte_flow
functions.

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
---

v3:
 - No updates.

v2:
 - Using critical section for windows pthread mutex.

---

 lib/librte_eal/windows/include/pthread.h | 33 ++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

Dmitry Kozlyuk Oct. 7, 2020, 4:53 p.m. UTC | #1 
On Wed,  7 Oct 2020 22:17:28 +0800, Suanming Mou wrote:
> Add pthread mutex lock as it is needed for the thread safe rte_flow
> functions.
> 
> Signed-off-by: Suanming Mou <suanmingm@nvidia.com>

Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Suanming Mou Oct. 8, 2020, 2:46 a.m. UTC | #2 
Hi Dmitry,

Thank you very much.
I also got some messages that they wish the PTHREAD_MUTEX_INITIALIZER macro can also be supported.

Refer to [1], we found that with critical section solution, maybe the macro can be defined as this:
#define PTHREAD_MUTEX_INITIALIZER {(void*)-1,-1,0,0,0,0}

We have tested that works, so I would prefer to add that if the macro is also OK with you.
What do you think about that? 

The explanation from [1]:
" The pthreads API has an initialization macro that has no correspondence to anything in the windows API. By investigating the internal definition of the critical section type, one may work out how to initialize one without calling InitializeCriticalSection(). The trick here is that InitializeCriticalSection() is not allowed to fail. It tries to allocate a critical section debug object, but if no memory is available, it sets the pointer to a specific value. (One would expect that value to be NULL, but it is actually (void *)-1 for some reason.) Thus we can use this special value for that pointer, and the critical section code will work.

The other important part of the critical section type to initialize is the number of waiters. This controls whether or not the mutex is locked. Fortunately, this part of the critical section is unlikely to change. Apparently, many programs already test critical sections to see if they are locked using this value, so Microsoft felt that it was necessary to keep it set at -1 for an unlocked critical section, even when they changed the underlying algorithm to be more scalable. The final parts of the critical section object are unimportant, and can be set to zero for their defaults."

[1] https://locklessinc.com/articles/pthreads_on_windows/

BR,
SuanmingMou

> -----Original Message-----
> From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Sent: Thursday, October 8, 2020 12:54 AM
> To: Suanming Mou <suanmingm@nvidia.com>
> Cc: Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>; Dmitry Malloy
> <dmitrym@microsoft.com>; Pallavi Kadam <pallavi.kadam@intel.com>;
> dev@dpdk.org
> Subject: Re: [PATCH v3 1/2] eal/windows: add pthread mutex lock
> 
> On Wed,  7 Oct 2020 22:17:28 +0800, Suanming Mou wrote:
> > Add pthread mutex lock as it is needed for the thread safe rte_flow
> > functions.
> >
> > Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
> 
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Tal Shnaiderman Oct. 14, 2020, 10:02 a.m. UTC | #3 
Hi DmitryM, Naty,

We would like to know if the macro added below for PTHREAD_MUTEX_INITIALIZER is a safe initialization of a CRITICAL_SECTION.

Can you please review the code and explanation below and check it internally?  

> Subject: Re: [dpdk-dev] [PATCH v3 1/2] eal/windows: add pthread mutex
> lock
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Dmitry,
> 
> Thank you very much.
> I also got some messages that they wish the PTHREAD_MUTEX_INITIALIZER
> macro can also be supported.
> 
> Refer to [1], we found that with critical section solution, maybe the macro
> can be defined as this:
> #define PTHREAD_MUTEX_INITIALIZER {(void*)-1,-1,0,0,0,0}
> 
> We have tested that works, so I would prefer to add that if the macro is also
> OK with you.
> What do you think about that?
> 
> The explanation from [1]:
> " The pthreads API has an initialization macro that has no correspondence to
> anything in the windows API. By investigating the internal definition of the
> critical section type, one may work out how to initialize one without calling
> InitializeCriticalSection(). The trick here is that InitializeCriticalSection() is not
> allowed to fail. It tries to allocate a critical section debug object, but if no
> memory is available, it sets the pointer to a specific value. (One would expect
> that value to be NULL, but it is actually (void *)-1 for some reason.) Thus we
> can use this special value for that pointer, and the critical section code will
> work.
> 
> The other important part of the critical section type to initialize is the number
> of waiters. This controls whether or not the mutex is locked. Fortunately, this
> part of the critical section is unlikely to change. Apparently, many programs
> already test critical sections to see if they are locked using this value, so
> Microsoft felt that it was necessary to keep it set at -1 for an unlocked critical
> section, even when they changed the underlying algorithm to be more
> scalable. The final parts of the critical section object are unimportant, and can
> be set to zero for their defaults."
> 
> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flockl
> essinc.com%2Farticles%2Fpthreads_on_windows%2F&amp;data=02%7C01%
> 7Ctalshn%40nvidia.com%7C4533970f5c964751eeea08d86b3483e3%7C43083d
> 15727340c1b7db39efd9ccc17a%7C0%7C0%7C637377220576713708&amp;sdat
> a=qELFpXSw7%2Fk8m9FMIidiQsnTq%2BeyJRII8C1sDyrHJe4%3D&amp;reserv
> ed=0
> 
> BR,
> SuanmingMou
> 
> > -----Original Message-----
> > From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > Sent: Thursday, October 8, 2020 12:54 AM
> > To: Suanming Mou <suanmingm@nvidia.com>
> > Cc: Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>; Dmitry
> > Malloy <dmitrym@microsoft.com>; Pallavi Kadam
> > <pallavi.kadam@intel.com>; dev@dpdk.org
> > Subject: Re: [PATCH v3 1/2] eal/windows: add pthread mutex lock
> >
> > On Wed,  7 Oct 2020 22:17:28 +0800, Suanming Mou wrote:
> > > Add pthread mutex lock as it is needed for the thread safe rte_flow
> > > functions.
> > >
> > > Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
> >
> > Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
diff mbox series

Patch

diff --git a/lib/librte_eal/windows/include/pthread.h b/lib/librte_eal/windows/include/pthread.h
index 99013dc..644fd49 100644
--- a/lib/librte_eal/windows/include/pthread.h
+++ b/lib/librte_eal/windows/include/pthread.h
@@ -28,6 +28,10 @@ 
 /* defining pthread_attr_t type on Windows since there is no in Microsoft libc*/
 typedef void *pthread_attr_t;
 
+typedef void *pthread_mutexattr_t;
+
+typedef CRITICAL_SECTION pthread_mutex_t;
+
 typedef SYNCHRONIZATION_BARRIER pthread_barrier_t;
 
 #define pthread_barrier_init(barrier, attr, count) \
@@ -139,6 +143,35 @@ 
 	return 0;
 }
 
+static inline int
+pthread_mutex_init(pthread_mutex_t *mutex,
+		   __rte_unused pthread_mutexattr_t *attr)
+{
+	InitializeCriticalSection(mutex);
+	return 0;
+}
+
+static inline int
+pthread_mutex_lock(pthread_mutex_t *mutex)
+{
+	EnterCriticalSection(mutex);
+	return 0;
+}
+
+static inline int
+pthread_mutex_unlock(pthread_mutex_t *mutex)
+{
+	LeaveCriticalSection(mutex);
+	return 0;
+}
+
+static inline int
+pthread_mutex_destroy(pthread_mutex_t *mutex)
+{
+	DeleteCriticalSection(mutex);
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif