From patchwork Mon Jun 8 21:24:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Torvald Riegel X-Patchwork-Id: 7080 Received: (qmail 118294 invoked by alias); 8 Jun 2015 21:24:58 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 118282 invoked by uid 89); 8 Jun 2015 21:24:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL, BAYES_50, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Subject: [PATCH] Clean up semaphore EINTR handling after Linux futex docs clarification. From: Torvald Riegel To: GLIBC Devel Date: Mon, 08 Jun 2015 23:24:53 +0200 Message-ID: <1433798693.21461.451.camel@triegel.csb> Mime-Version: 1.0 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. This is code and mostly comments clean-up, no behavioral change. I've put this on top of the futex API patch I just sent because that one documents the conditions under which futex_wait can return EINTR. I intend to commit this unless I hear objections. 2015-06-08 Torvald Riegel * nptl/sem_waitcommon.c (__new_sem_wait_slow): Update comments. (sem_assume_only_signals_cause_futex_EINTR): Remove. commit 16c60b687a61c3d2b1a89766d9132bce78d05719 Author: Torvald Riegel Date: Mon Jun 8 23:14:20 2015 +0200 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. diff --git a/nptl/sem_waitcommon.c b/nptl/sem_waitcommon.c index d3702c7..77421cd 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 and 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;