summary refs log tree commit diff
diff options
context:
space:
mode:
authorTorvald Riegel <triegel@redhat.com>2015-06-08 23:14:20 +0200
committerTorvald Riegel <triegel@redhat.com>2015-07-10 13:47:45 +0200
commit1aa8d144f971a907950014177d7e150fe82b83b0 (patch)
treeaa2a67d1e42091da50df990556d10dda870af219
parenta2f0363f817a58c4d3f439aa515a3b1d73efde36 (diff)
downloadglibc-1aa8d144f971a907950014177d7e150fe82b83b0.tar.gz
glibc-1aa8d144f971a907950014177d7e150fe82b83b0.tar.xz
glibc-1aa8d144f971a907950014177d7e150fe82b83b0.zip
Clean up semaphore EINTR handling after Linux futex docs clarification.
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.
-rw-r--r--ChangeLog5
-rw-r--r--nptl/sem_waitcommon.c37
2 files changed, 12 insertions, 30 deletions
diff --git a/ChangeLog b/ChangeLog
index a8bab48b86..f4e4be6be5 100644
--- 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.
diff --git a/nptl/sem_waitcommon.c b/nptl/sem_waitcommon.c
index d3702c7b4f..f222e71895 100644
--- a/nptl/sem_waitcommon.c
+++ b/nptl/sem_waitcommon.c
@@ -78,14 +78,6 @@
    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;