Re: [PATCH v18 8/8] eal: implement functions for mutex management

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

2022-02-09 13:57 (UTC+0000), Ananyev, Konstantin:
> > > Actually, please scrap that comment.
> > > Obviously it wouldn't work for static variables,
> > > and doesn't make much sense.
> > > Though few thoughts remain:
> > > for posix we probably don't need an indirection and
> > > rte_thread_mutex can be just typedef of pthread_mutex_t.
> > > also for posix we don't need RTE_INIT constructor for each
> > > static mutex initialization.
> > > Something like:
> > > #define RTE_STATIC_INITIALIZED_MUTEX(mx) \
> > > 	rte_thread_mutex_t mx = PTHREAD_MUTEX_INITIALIZER
> > > should work, I think.
> > > Konstantin  
> > 
> > Thank you for reviewing, Konstantin!
> > Some context for the current representation of mutex
> > can be found in v9, patch 7/10 of this patchset.
> > 
> > Originally we've typedef'ed the pthread_mutex_t on POSIX, just
> > like you are suggesting here.
> > However, on Windows there's no static initializer similar to the pthread
> > one. Still, we want ABI compatibility and same thread behavior between
> > platforms. The most elegant solution we found was the current representation,
> > as suggested by Dmitry K.  
> 
> Yes, I agree it is a problem with Windows for static initializer.
> But why we can't have different structs typedef for mutex 
> for posix and windows platforms?

Yes, I agree that having different mutex types on *nix and Windows
is a great idea. It will avoid ABI change for *nix
and will guarantee no performance impact.

Maybe wrap pthread_mutex_t into a struct to have a distinct type
and to force using only DPDK API with it?

[...]
> Yes, on Windows rte_thread_mutex still wouldn't work for MP,
> but that's the same as with current design.

MP support is not planned for Windows and it is unknown if it ever will be,
so it's not an issue.
Data location is.
The reason rte_thread_mutex_t is not a typedef of CRITICAL_SECTION
(akin to pthread_mutex_t) is to avoid including Windows headers
into DPDK public headers, because Windows headers can break user code
by some macros they define.
Maybe instead of a pointer it could be an opaque array:

	#define RTE_PTHREAD_MUTEX_SIZE 40

	struct rte_pthread_mutex_t {
		uint8_t opaque[RTE_PTHREAD_MUTEX_SIZE];
	};

where RTE_PTHREAD_MUTEX_SIZE is actually sizeof(CRITICAL_SECTION).
Win32 ABI is remarkably stable, I don't think this constant will ever change,
it would break all the Windows user space.
Naty, DmitryM, Tyler, what do you think?