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.
