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

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Hi Dmitry,

> 2022-02-21 00:56 (UTC+0300), Dmitry Kozlyuk:
> > 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?
> 
> Conclusion from offline call: yes, this is OK to do so.
> 
> However, DmitryM suggested using Slim Reader-Writer lock (SRW):
> https://docs.microsoft.com/en-us/windows/win32/sync/slim-reader-writer--srw--locks
> instead of CRITICAL_SECTION.
> It seems to be a much better option:
> 
> * sizeof(SRWLOCK) == 8 (technically "size of a pointer"),
>   same as sizeof(pthread_mutex_t) on a typical Linux.
>   Layout of data structures containing rte_thread_mutex_t
>   can be the same on Windows and Unix,
>   which simplifies design and promises similar less performance differences.
> 
> * Can be taken by multiple readers and one writer,
>   which is semantically similar to pthread_mutex_t

Not sure I understand you here:
pthread_mutex provides only mutually-exclusive lock semantics.
For RW lock there exists: pthread_rwlock_t.
Off-course you can use rwlock fo exclusive locking too -
if all callers will use only writer locks, but usually that's no point to do that -
mutexes are simpler and faster.
That's for posix-like systems, don't know much about Windows environment :)

>   (CRITICAL_SECTION can only be taken by a single thread).
> 
> Technically it can be a "typedef uintptr_t" or a structure wrapping it.

Again can't say much about Windows, but pthread_mutex_t
can (and is) bigger then then 8 bytes.