[m-rev.] for-review: Apply upstream BoehmGC changes for glibc lock elision bug workaround
Paul Bone
paul at bone.id.au
Thu Sep 18 16:51:12 AEST 2014
For review/testing by others.
Last time I modified something in boehm_gc/ I broke the build for OS X.
This works on 64bit Linux (Debian testing) but I havn't tested it on other
platforms. Julien could you help me out by building this on OS X and
perhaps Windows? I've been testing it with the icfp2000_par_pbone benchmark
in asm_fast.gc.par.stseg. Thanks.
I have a branch with these patches here:
https://github.com/PaulBone/mercury/tree/fix_tsx_bug
---
Apply upstream BoehmGC changes for glibc lock elision bug workaround
This code was written by Ivan Maidanski <ivmai at mail.ru> in response to my
patch to the the Boehm GC to work-around the lock elision bug in glibc.
Fix and code refactoring of lock elision workaround (Linux/x64)
https://github.com/ivmai/bdwgc/commit/3d342554dc03af8d27925706a6513080995e1904
boehm_gc/private/gcconfig.h:
+ (GLIBC_2_19_TSX_BUG): New macro defined for Linux/x86_64 (if Glibc
used) to workaround a bug in Glibc lock elision implementation.
boehm_gc/misc.c:
+ (GC_init): Reformat code.
boehm_gc/pthread_support.c:
+ Move include of gnu/libc-version.h to gcconfig.h (used to check
whether lock elision workaround needed).
+ (mark_mutex): Initialize (to PTHREAD_MUTEX_INITIALIZER) even lock
elision workaround is needed (revert change in previous commit).
+ (parse_version): New static function (defined only if
GLIBC_2_19_TSX_BUG).
+ (GC_setup_mark_lock): Use parse_version to check target Glibc version
properly; do not reinitialize mutex unless workaround needed; call
ABORT (with the appropriate message) in case of a failure in
pthread_mutexattr_init/settype, pthread_mutex_init.
---
boehm_gc/include/private/gcconfig.h | 5 +++
boehm_gc/misc.c | 4 +--
boehm_gc/pthread_support.c | 68 +++++++++++++++++++++++--------------
3 files changed, 50 insertions(+), 27 deletions(-)
diff --git a/boehm_gc/include/private/gcconfig.h b/boehm_gc/include/private/gcconfig.h
index 767859c..1d5559a 100644
--- a/boehm_gc/include/private/gcconfig.h
+++ b/boehm_gc/include/private/gcconfig.h
@@ -2147,6 +2147,11 @@
/* FIXME: This seems to be fixed in GLibc v2.14. */
# define GETCONTEXT_FPU_EXCMASK_BUG
# endif
+# if defined(__GLIBC__)
+ /* Workaround lock elision implementation for some glibc. */
+# define GLIBC_2_19_TSX_BUG
+# include <gnu/libc-version.h> /* for gnu_get_libc_version() */
+# endif
# endif
# ifdef DARWIN
# define OS_TYPE "DARWIN"
diff --git a/boehm_gc/misc.c b/boehm_gc/misc.c
index 1b65243..c25abe0 100644
--- a/boehm_gc/misc.c
+++ b/boehm_gc/misc.c
@@ -744,8 +744,8 @@ GC_API void GC_CALL GC_init(void)
/* else */ InitializeCriticalSection (&GC_allocate_ml);
}
# endif /* GC_WIN32_THREADS */
-# if (defined(GC_PTHREADS) && !defined(GC_WIN32_THREADS))
- GC_setup_mark_lock();
+# if defined(GC_PTHREADS) && !defined(GC_WIN32_THREADS)
+ GC_setup_mark_lock();
# endif /* GC_PTHREADS */
# if (defined(MSWIN32) || defined(MSWINCE)) && defined(THREADS)
InitializeCriticalSection(&GC_write_cs);
diff --git a/boehm_gc/pthread_support.c b/boehm_gc/pthread_support.c
index 72f9346..674ef52 100644
--- a/boehm_gc/pthread_support.c
+++ b/boehm_gc/pthread_support.c
@@ -1854,35 +1854,53 @@ GC_INNER void GC_lock(void)
static pthread_cond_t builder_cv = PTHREAD_COND_INITIALIZER;
-GC_INNER void GC_setup_mark_lock(void)
-{
-#if !defined(GLIBC_2_1_MUTEX_HACK)
- pthread_mutexattr_t attr;
-
- if (0 != pthread_mutexattr_init(&attr)) {
- goto error;
+#ifdef GLIBC_2_19_TSX_BUG
+ /* Parse string like <major>[.<minor>[<tail>]] and return major value. */
+ static int parse_version(int *pminor, const char *pverstr) {
+ char *endp;
+ unsigned long value = strtoul(pverstr, &endp, 10);
+ int major = (int)value;
+
+ if (major < 0 || (char *)pverstr == endp || (unsigned)major != value) {
+ /* Parse error */
+ return -1;
}
-
- /*
- * By explicitly setting a mutex type we disable lock elision and work
- * around a bug in glibc 2.19
- */
- if (0 != pthread_mutexattr_settype(&attr,
- PTHREAD_MUTEX_NORMAL))
- {
- goto error;
+ if (*endp != '.') {
+ /* No minor part. */
+ *pminor = -1;
+ } else {
+ value = strtoul(endp + 1, &endp, 10);
+ *pminor = (int)value;
+ if (*pminor < 0 || (unsigned)(*pminor) != value) {
+ return -1;
+ }
}
+ return major;
+ }
+#endif /* GLIBC_2_19_TSX_BUG */
- if (0 != pthread_mutex_init(&mark_mutex, &attr)) {
- goto error;
+GC_INNER void GC_setup_mark_lock(void)
+{
+# ifdef GLIBC_2_19_TSX_BUG
+ pthread_mutexattr_t mattr;
+ int glibc_minor = -1;
+ int glibc_major = parse_version(&glibc_minor, gnu_get_libc_version());
+
+ if (glibc_major > 2 || (glibc_major == 2 && glibc_minor >= 19)) {
+ /* TODO: disable this workaround for glibc with fixed TSX */
+ /* This disables lock elision to workaround a bug in glibc 2.19+ */
+ if (0 != pthread_mutexattr_init(&mattr)) {
+ ABORT("pthread_mutexattr_init failed");
+ }
+ if (0 != pthread_mutexattr_settype(&mattr, PTHREAD_MUTEX_ERRORCHECK)) {
+ ABORT("pthread_mutexattr_settype failed");
+ }
+ if (0 != pthread_mutex_init(&mark_mutex, &mattr)) {
+ ABORT("pthread_mutex_init failed");
+ }
+ pthread_mutexattr_destroy(&mattr);
}
- pthread_mutexattr_destroy(&attr);
- return;
-
-error:
- perror("Error setting up marker mutex");
- exit(1);
-#endif /* ! GLIBC_2_1_MUTEX_HACK */
+# endif
}
GC_INNER void GC_acquire_mark_lock(void)
--
2.1.0
More information about the reviews
mailing list