Re: [dpdk-dev] [PATCH v9 07/10] eal: implement functions for mutex management

On Thu, Jun 10, 2021 at 01:37:17AM +0300, Dmitry Kozlyuk wrote:

2021-06-09 02:04 (UTC+0300), Dmitry Kozlyuk:

quoted

2021-06-04 16:44 (UTC-0700), Narcisa Ana Maria Vasile:
[...]

quoted

diff --git a/lib/eal/include/rte_thread_types.h b/lib/eal/include/rte_thread_types.h
index d67b24a563..7bb0d2948c 100644
--- a/lib/eal/include/rte_thread_types.h
+++ b/lib/eal/include/rte_thread_types.h

@@ -7,4 +7,8 @@
 
 #include <pthread.h>
 
+#define RTE_THREAD_MUTEX_INITIALIZER     PTHREAD_MUTEX_INITIALIZER
+
+typedef pthread_mutex_t                 rte_thread_mutex_t;
+
 #endif /* _RTE_THREAD_TYPES_H_ */

diff --git a/lib/eal/windows/include/rte_windows_thread_types.h b/lib/eal/windows/include/rte_windows_thread_types.h
index 60e6d94553..c6c8502bfb 100644
--- a/lib/eal/windows/include/rte_windows_thread_types.h
+++ b/lib/eal/windows/include/rte_windows_thread_types.h

@@ -7,4 +7,13 @@
 
 #include <rte_windows.h>
 
+#define WINDOWS_MUTEX_INITIALIZER               (void*)-1
+#define RTE_THREAD_MUTEX_INITIALIZER            {WINDOWS_MUTEX_INITIALIZER}
+
+struct thread_mutex_t {
+	void* mutex_id;
+};
+
+typedef struct thread_mutex_t rte_thread_mutex_t;
+
 #endif /* _RTE_THREAD_TYPES_H_ */  

In previous patches rte_thread content was made opaque and of equal size
for pthread (most implementations) and non-pthread variant.
AFAIU, we agree on the requirement of compatible ABI between variants,
that is, a compiled app can work with any threading variant of DPDK.
Above definition of `rte_thread_mutex_t` does not satisfy it.
Or do we only promise API compatibility?
This is the most important question now.

From Windows community call 2021-06-10, for everyone's information.

1. Yes, binary compatibility is a requirement.

2. Static mutex initializer for Windows is tricky (an old topic).
This patch proposes `rte_mutex` to hold a pointer to actual mutex
and use NULL as static initializer, then allocate on first use.
At the same time we want to use the same initializer for pthread variant.
This means it would also need indirection, allocation, and tricky logic.

My opinion:

New threading API can just be without static initilizer.
All it usages in DPDK could be converted to dynamic initialization
either in appropriate function or using `RTE_INIT`.
Maybe create a convenient macro to declare a static mutex and its
initialization code, what do others think?

	RTE_STATIC_MUTEX(private_lock)

Expanding to:

	static RTE_DECLARE_MUTEX(private_lock)
	RTE_DEFINE_MUTEX(private_lock)


Expanding to:

	static rte_mutex private_lock;

	RTE_INIT(__rte_private_lock_init)
	{
		RTE_VERIFY(rte_thread_mutex_init(&private_lock));
	}

As a bonus it removes the need of `rte_*_thread_types.h`.

Thank you Dmitry, I think this is the best and most elegant solution.
I will use a pointer to represent the mutex:
typedef struct rte_thread_mutex_tag {
	void* mutex_id;
} rte_thread_mutex;

..and use the macro for static initializations as you described.