[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