Clean up semaphore EINTR handling after Linux futex docs clarification.
authorTorvald Riegel <triegel@redhat.com>
Mon, 8 Jun 2015 21:14:20 +0000 (23:14 +0200)
committerTorvald Riegel <triegel@redhat.com>
Fri, 10 Jul 2015 11:47:45 +0000 (13:47 +0200)
The Linux kernel futex documentation now states that since Linux 2.6.22,
FUTEX_WAIT does return EINTR only when interrupted by a signal, and not
spuriously anymore.  We only support more recent kernels, so clean up
EINTR handling in the semaphore and update the comments.

ChangeLog
nptl/sem_waitcommon.c

index a8bab48..f4e4be6 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2015-07-10  Torvald Riegel  <triegel@redhat.com>
 
+       * nptl/sem_waitcommon.c (__new_sem_wait_slow): Update comments.
+       (sem_assume_only_signals_cause_futex_EINTR): Remove.
+
+2015-07-10  Torvald Riegel  <triegel@redhat.com>
+
        * sysdeps/nptl/futex-internal.h: New file.
        * sysdeps/nacl/futex-internal.h: New file.
        * sysdeps/unix/sysv/linux/futex-internal.h: New file.
index d3702c7..f222e71 100644 (file)
    requirement because the semaphore must not be destructed while any sem_wait
    is still executing.  */
 
-/* Set this to true if you assume that, in contrast to current Linux futex
-   documentation, lll_futex_wake can return -EINTR only if interrupted by a
-   signal, not spuriously due to some other reason.
-   TODO Discuss EINTR conditions with the Linux kernel community.  For
-   now, we set this to true to not change behavior of semaphores compared
-   to previous glibc builds.  */
-static const int sem_assume_only_signals_cause_futex_EINTR = 1;
-
 #if !__HAVE_64B_ATOMICS
 static void
 __sem_wait_32_finish (struct new_sem *sem);
@@ -191,26 +183,12 @@ __new_sem_wait_slow (struct new_sem *sem, const struct timespec *abstime)
             wake-up, or due to a change in the number of tokens.  We retry in
             these cases.
             If we timed out, forward this to the caller.
-            EINTR could be either due to being interrupted by a signal, or
-            due to a spurious wake-up.  Thus, we cannot distinguish between
-            both, and are not allowed to return EINTR to the caller but have
-            to retry; this is because we may not have been interrupted by a
-            signal.  However, if we assume that only signals cause a futex
-            return of EINTR, we forward EINTR to the caller.
-
-            Retrying on EINTR is technically always allowed because to
-            reliably interrupt sem_wait with a signal, the signal handler
-            must call sem_post (which is AS-Safe).  In executions where the
-            signal handler does not do that, the implementation can correctly
-            claim that sem_wait hadn't actually started to execute yet, and
-            thus the signal never actually interrupted sem_wait.  We make no
-            timing guarantees, so the program can never observe that sem_wait
-            actually did start to execute.  Thus, in a correct program, we
-            can expect a signal that wanted to interrupt the sem_wait to have
-            provided a token, and can just try to grab this token if
-            futex_wait returns EINTR.  */
-         if (err == ETIMEDOUT ||
-             (err == EINTR && sem_assume_only_signals_cause_futex_EINTR))
+            EINTR is returned if we are interrupted by a signal; we
+            forward this to the caller.  (See futex_wait and related
+            documentation.  Before Linux 2.6.22, EINTR was also returned on
+            spurious wake-ups; we only support more recent Linux versions,
+            so do not need to consider this here.)  */
+         if (err == ETIMEDOUT || err == EINTR)
            {
              __set_errno (err);
              err = -1;
@@ -302,8 +280,7 @@ __new_sem_wait_slow (struct new_sem *sem, const struct timespec *abstime)
            {
              /* See __HAVE_64B_ATOMICS variant.  */
              err = do_futex_wait(sem, abstime);
-             if (err == ETIMEDOUT ||
-                 (err == EINTR && sem_assume_only_signals_cause_futex_EINTR))
+             if (err == ETIMEDOUT || err == EINTR)
                {
                  __set_errno (err);
                  err = -1;