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

[Narcisa Ana Maria Vasile]

On Thu, Jun 10, 2021 at 01:37:17AM +0300, Dmitry Kozlyuk wrote:
> 2021-06-09 02:04 (UTC+0300), Dmitry Kozlyuk:
> > 2021-06-04 16:44 (UTC-0700), Narcisa Ana Maria Vasile:
> > [...]
> > > 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.