This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH v2] handle sem_t with ILP32 and __HAVE_64B_ATOMICS


This version reflects Torvald's recent comments.

2015-01-25  Chris Metcalf  <cmetcalf@ezchip.com>

        * sysdeps/nptl/internaltypes.h (to_new_sem): Define.  Provides new
        behavior for [__HAVE_64B_ATOMICS && !defined (_LP64)].
        * nptl/sem_getvalue.c (__new_sem_getvalue): Use to_new_sem.
        * nptl/sem_init.c (__new_sem_init): Likewise.
        * nptl/sem_open.c (sem_open): Likewise.
        * nptl/sem_post.c (__new_sem_post): Likewise.
        * nptl/sem_timedwait.c (sem_timedwait): Likewise.
        * nptl/sem_wait.c (__new_sem_wait): Likewise.
        (__new_sem_trywait): Likewise.

diff --git a/nptl/sem_getvalue.c b/nptl/sem_getvalue.c
index 1432cc7..08a0789 100644
--- a/nptl/sem_getvalue.c
+++ b/nptl/sem_getvalue.c
@@ -25,7 +25,7 @@
 int
 __new_sem_getvalue (sem_t *sem, int *sval)
 {
-  struct new_sem *isem = (struct new_sem *) sem;
+  struct new_sem *isem = to_new_sem (sem);

   /* XXX Check for valid SEM parameter.  */
   /* FIXME This uses relaxed MO, even though POSIX specifies that this function
diff --git a/nptl/sem_init.c b/nptl/sem_init.c
index 575b661..aaa6af8 100644
--- a/nptl/sem_init.c
+++ b/nptl/sem_init.c
@@ -50,7 +50,7 @@ __new_sem_init (sem_t *sem, int pshared, unsigned int value)
     }

   /* Map to the internal type.  */
-  struct new_sem *isem = (struct new_sem *) sem;
+  struct new_sem *isem = to_new_sem (sem);

   /* Use the values the caller provided.  */
 #if __HAVE_64B_ATOMICS
diff --git a/nptl/sem_open.c b/nptl/sem_open.c
index bfd2dea..202769f 100644
--- a/nptl/sem_open.c
+++ b/nptl/sem_open.c
@@ -186,25 +186,28 @@ sem_open (const char *name, int oflag, ...)
          return SEM_FAILED;
        }

-      /* Create the initial file content.  */
-      union
-      {
-       sem_t initsem;
-       struct new_sem newsem;
-      } sem;
+      /* Create the initial file content.  We force the alignment of
+         the sem_t to match the alignment of struct new_sem since we
+         will copy this stack structure to a file and then mmap it,
+         so we must ensure it is aligned to zero here so that the
+         behavior of to_new_sem () is the same as when we later mmap
+         it into memory and have it be page-aligned.  */
+      sem_t sem __attribute__ ((aligned (__alignof__ (struct new_sem))));;
+      struct new_sem *newsem = to_new_sem (&sem);
+
+      /* Initialize the unused parts of sem_t to zero.
+         The compiler will notice most of this memset is dead based on
+         the assignments through the struct new_sem pointer.  */
+      memset (&sem, '\0', sizeof (sem_t));

 #if __HAVE_64B_ATOMICS
-      sem.newsem.data = value;
+      newsem->data = value;
 #else
-      sem.newsem.value = value << SEM_VALUE_SHIFT;
-      sem.newsem.nwaiters = 0;
+      newsem->value = value << SEM_VALUE_SHIFT;
+      newsem->nwaiters = 0;
 #endif
       /* This always is a shared semaphore.  */
-      sem.newsem.private = LLL_SHARED;
-
-      /* Initialize the remaining bytes as well.  */
-      memset ((char *) &sem.initsem + sizeof (struct new_sem), '\0',
-             sizeof (sem_t) - sizeof (struct new_sem));
+      newsem->private = LLL_SHARED;

       tmpfname = __alloca (shm_dirlen + sizeof SEM_SHM_PREFIX + 6);
       char *xxxxxx = __mempcpy (tmpfname, shm_dir, shm_dirlen);
@@ -242,7 +245,7 @@ sem_open (const char *name, int oflag, ...)
          break;
        }

-      if (TEMP_FAILURE_RETRY (__libc_write (fd, &sem.initsem, sizeof (sem_t)))
+      if (TEMP_FAILURE_RETRY (__libc_write (fd, &sem, sizeof (sem_t)))
          == sizeof (sem_t)
          /* Map the sem_t structure from the file.  */
          && (result = (sem_t *) mmap (NULL, sizeof (sem_t),
diff --git a/nptl/sem_post.c b/nptl/sem_post.c
index 6e495ed..71818ea 100644
--- a/nptl/sem_post.c
+++ b/nptl/sem_post.c
@@ -56,7 +56,7 @@ futex_wake (unsigned int* futex, int processes_to_wake, int private)
 int
 __new_sem_post (sem_t *sem)
 {
-  struct new_sem *isem = (struct new_sem *) sem;
+  struct new_sem *isem = to_new_sem (sem);
   int private = isem->private;

 #if __HAVE_64B_ATOMICS
diff --git a/nptl/sem_timedwait.c b/nptl/sem_timedwait.c
index 042b0ac..5e650e0 100644
--- a/nptl/sem_timedwait.c
+++ b/nptl/sem_timedwait.c
@@ -30,8 +30,8 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
       return -1;
     }

-  if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0)
+  if (__new_sem_wait_fast (to_new_sem (sem), 0) == 0)
     return 0;
   else
-    return __new_sem_wait_slow((struct new_sem *) sem, abstime);
+    return __new_sem_wait_slow(to_new_sem (sem), abstime);
 }
diff --git a/nptl/sem_wait.c b/nptl/sem_wait.c
index c1fd10c..73759f1 100644
--- a/nptl/sem_wait.c
+++ b/nptl/sem_wait.c
@@ -22,10 +22,10 @@
 int
 __new_sem_wait (sem_t *sem)
 {
-  if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0)
+  if (__new_sem_wait_fast (to_new_sem (sem), 0) == 0)
     return 0;
   else
-    return __new_sem_wait_slow((struct new_sem *) sem, NULL);
+    return __new_sem_wait_slow(to_new_sem (sem), NULL);
 }
 versioned_symbol (libpthread, __new_sem_wait, sem_wait, GLIBC_2_1);

@@ -65,7 +65,7 @@ __new_sem_trywait (sem_t *sem)
 {
   /* We must not fail spuriously, so require a definitive result even if this
      may lead to a long execution time.  */
-  if (__new_sem_wait_fast ((struct new_sem *) sem, 1) == 0)
+  if (__new_sem_wait_fast (to_new_sem (sem), 1) == 0)
     return 0;
   __set_errno (EAGAIN);
   return -1;
diff --git a/sysdeps/nptl/internaltypes.h b/sysdeps/nptl/internaltypes.h
index 8f5cfa4..8dd4018 100644
--- a/sysdeps/nptl/internaltypes.h
+++ b/sysdeps/nptl/internaltypes.h
@@ -168,6 +168,22 @@ struct new_sem
 #endif
 };

+/* The sem_t alignment is typically just 4 bytes for ILP32, whereas
+   new_sem is 8 bytes with __HAVE_64B_ATOMICS.  For better atomic
+   performance on platforms that tolerate unaligned atomics (and to
+   make it work at all on platforms that don't), round up the new_sem
+   pointer within the sem_t object to an 8-byte boundary.
+
+   Note that in this case, the useful part of the new_sem structure
+   (up to the "pad" variable) is only 12 bytes, and sem_t is always
+   at least 16 bytes, so moving the start of new_sem to an offset
+   of 4 bytes within sem_t is okay.  We must not read or write the
+   "pad" field within new_sem to maintain correctness.  */
+#define to_new_sem(s)                                                \
+  ((__alignof__ (sem_t) == 4 && __alignof__ (struct new_sem) == 8) ? \
+   (struct new_sem *) (((uintptr_t) (s) + 4) & -8) :                 \
+   (struct new_sem *) (s))
+
 struct old_sem
 {
   unsigned int value;

--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]