This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[review v2] nptl: Add struct_mutex.h
Adhemerval Zanella has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/518
......................................................................
Patch Set 2:
(11 comments)
| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +11,29 @@ C11 on pthreadtypes-arch.h (added by commit 06be6368da16104be5) is
| +not really the best options for newer ports. It requires define some
| +misleading flags that should be always defined as 0
| +(__PTHREAD_COMPAT_PADDING_MID and __PTHREAD_COMPAT_PADDING_END), it
| +exposes options used solely for linuxthreads compat mode
| +(__PTHREAD_MUTEX_USE_UNION and __PTHREAD_MUTEX_NUSERS_AFTER_KIND), and
| +requires newer ports to explicit define them (adding more boilerplate
| +code).
| +
| +This patch adds a new default __pthread_mutex_s definition meant to
| +be used by newer ports. Its layout mimic the current usage on both
PS2, Line 20:
Done
| +32 and 64 bits ports and it allows most ports use the generic
PS2, Line 21:
Done
| +definition. Only ports that use some arch-specific definition (such
| +as hardware lock-elision or linuxthread compat) requires specific
PS2, Line 23:
Done
| +headers.
| +
| +For 32 bits, the generic definitions mimics the other 32 bits ports
PS2, Line 26:
Done
| +of using an union to define the fields uses on adaptive and robust
| +mutexes (thus not allowing both usage at same time) and by using a
| +single linked-list for robust mutexes. Both decisions seemed to
| +follow what recent ports has done and make the resulting
PS2, Line 30:
Done
| +pthread_mutex_t/mtx_t object smaller.
| +
| +Also the static intialization macro for pthread_mutex_t is set to use
| +an arch defined on (__PTHREAD_MUTEX_INITIALIZER) which simplifies its
PS2, Line 34:
I changed to:
Also the static intialization macro for pthread_mutex_t is set to use
a macro __PTHREAD_MUTEX_INITIALIZER where the architecture can
redefine
in its struct_mutex.h if it requires additional fields to be
initialized.
| +implementation.
| +
| +Checked with a build on affected abis.
| +
| +Change-Id: I30a22c3e3497805fd6e52994c5925897cffcfe13
| --- /dev/null
| +++ sysdeps/nptl/bits/struct_mutex.h
| @@ -1,0 +1,10 @@
| +/* Default internal mutex struct definitions. Linux version.
PS2, Line 1:
Ack
| + Copyright (C) 2019 Free Software Foundation, Inc.
| + This file is part of the GNU C Library.
| +
| + The GNU C Library is free software; you can redistribute it and/or
| + modify it under the terms of the GNU Lesser General Public
| + License as published by the Free Software Foundation; either
| + version 2.1 of the License, or (at your option) any later version.
| +
| + The GNU C Library is distributed in the hope that it will be useful,
...
| @@ -1,0 +41,20 @@ #endif
| +
| + After a mutex has been initialized, the __kind of a mutex is usually not
| + changed. BUT it can be set to -1 in pthread_mutex_destroy or elision can
| + be enabled. This is done concurrently in the pthread_mutex_*lock
| + functions by using the macro FORCE_ELISION. This macro is only defined
| + for architectures which supports lock elision.
| +
| + For elision, there are the flags PTHREAD_MUTEX_ELISION_NP and
| + PTHREAD_MUTEX_NO_ELISION_NP which can be set in addition to the already
| + set type of a mutex. Before a mutex is initialized, only
| + PTHREAD_MUTEX_NO_ELISION_NP can be set with pthread_mutexattr_settype.
PS2, Line 51:
These comments were copied from thread-shared-types.h and added
originally by 403b4feb22d. My understanding is the comment state that
PTHREAD_MUTEX_ELISION_NP can not be set internally by
pthread_mutexattr_settype, since it is done by pthread_mutex_*lock.
| +
| + After a mutex has been initialized, the functions pthread_mutex_*lock can
| + enable elision - if the mutex-type and the machine supports it - by
| + setting the flag PTHREAD_MUTEX_ELISION_NP. This is done concurrently.
| + Afterwards the lock / unlock functions are using specific elision
| + code-paths. */
| + int __kind;
| +#if __WORDSIZE != 64
| + unsigned int __nusers;
| --- sysdeps/nptl/bits/thread-shared-types.h
| +++ sysdeps/nptl/bits/thread-shared-types.h
| @@ -160,11 +62,20 @@ #else
| - __PTHREAD_SPINS_DATA;
| - __pthread_slist_t __list;
| - };
| -# define __PTHREAD_MUTEX_HAVE_PREV 0
| -#endif
| - __PTHREAD_COMPAT_PADDING_END
| -};
| +
| +/* Arch-specific mutex definitions. A generic implementation is provided
| + by sysdeps/nptl/bits/struct_mutex.h. If required, an architecture
PS2, Line 64:
I don't think a comment on Makefile would be meaningful, I removed the
path and just refered as 'struct_mutex.h'.
| + can override it by defining:
| +
| + 1. struct __pthread_mutex_s (used on both pthread_mutex_t and mtx_t
| + definition). It should contains at least the internal members
| + defined in the generic version.
PS2, Line 69:
Ack
| +
| + 2. __LOCK_ALIGNMENT for any extra attribute for internal lock used with
| + atomic operations.
PS2, Line 72:
I think we should move it to a separate header of its own since it
used on pthread_cond_t definition as well. I will refactor after this
patchset, along with Joseph suggestion to use _Alignas / alignas where
possible.
| +
| + 3. The macro __PTHREAD_MUTEX_INITIALIZER used for static initialization.
| + It should initialize the mutex internal flag. */
| +
| +#include <bits/struct_mutex.h>
|
|
| /* Common definition of pthread_cond_t. */
|
--
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I30a22c3e3497805fd6e52994c5925897cffcfe13
Gerrit-Change-Number: 518
Gerrit-PatchSet: 2
Gerrit-Owner: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Gerrit-Reviewer: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Gerrit-Reviewer: Florian Weimer <fweimer@redhat.com>
Gerrit-Comment-Date: Wed, 20 Nov 2019 14:49:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Florian Weimer <fweimer@redhat.com>
Gerrit-MessageType: comment