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.